Comment 13 for bug 986880

Revision history for this message
In , Lionel Elie Mamane (lionel-mamane) wrote :

(In reply to comment #7)
> If we suppose_pType->aCreateParams.isEmpty() is useful, here's a naive patch:
> diff --git a/dbaccess/source/ui/tabledesign/FieldDescriptions.cxx
> b/dbaccess/source/ui/tabledesign/FieldDescriptions.cxx
> index 65fb772..d3495a8 100644
> --- a/dbaccess/source/ui/tabledesign/FieldDescriptions.cxx
> +++ b/dbaccess/source/ui/tabledesign/FieldDescriptions.cxx
> @@ -221,7 +221,7 @@ void OFieldDescription::FillFromTypeInfo(const
> TOTypeInfoSP& _pType,bool _bForce
> SetScale(::std::min<sal_Int32>(GetScale() ?
> GetScale() : DEFAULT_NUMERIC_SCALE,_pType->nMaximumScale));
> }
> }
> - if ( _pType->aCreateParams.isEmpty() )
> + if ( _pType->aCreateParams.isEmpty() || _bForce )
> {
> SetPrecision(_pType->nPrecision);
> SetScale(_pType->nMinimumScale);

To me, it looks like when changing type, the "switch" statement just before in the code is in charge (when the type changed, thus bForce is true, even if _bForce is not) of setting the default values. I'd recommend looking through it, what happens when the bug is hit?

But actually, it is not immediately obvious to me what the best behaviour should be here, actually. The bug reporter seems to suggest "always reset to the type default, for *any* type change". But keeping the same length (precision?) kinda makes sense, too. E.g. when going from FLOAT(17) to VARCHAR(n), well, a FLOAT(17) "fits" in a VARCHAR(17) (or is it VARCHAR(18) to account for the decimal separator? Need to look at definitions...), so one could argue that n=17 (or 18 / ...) is a rather good heuristic. A very quick glance at the code suggests that maybe that's what is being done, very much on purpose.

Similarly, when changing from NUMERIC to DECIMAL or vice-versa, keeping the same precision and scale again makes sense to me... Similarly, "in a perfect world" switching from INTEGER(10) to NUMERIC, I'd say that NUMERIC(10, 0) is as good a guess than any.

I'd also keep the previous length when switching between CHAR, VARCHAR, VARCHAR_IGNORECASE, etc, etc.