[Upstream] field length inherited

When I define a table field in LbreOfficeBase I notice that the following field types NUMERIC, DECIMAL, FLOAT, VARCHAR, VARCHAR_IGNORECASE, inherit the latest set length, that is: when I add a field it is set to VARCHAR (100), if I change it to FLOAT its length is automatically set to 17, if I come back to VARCHAR, the field length is not set back to 100, but still remains 17.

[Reproducible] with "LibreOffice 3.5.2.2 - GNU/Linux Ubuntu 11.10,English
UI"

When I define a table field in LbreOfficeBase I notice that the following field types NUMERIC, DECIMAL, FLOAT, VARCHAR, VARCHAR_IGNORECASE, inherit the latest set length, that is: when I add a field it is set to VARCHAR (100), if I change it to FLOAT its length is automatically set to 17, if I come back to VARCHAR, the field length is not set back to 100, but still remains 17.

[Reproducible] with "LibreOffice 3.5.2.2 - GNU/Linux Ubuntu 11.10,English
UI"

*** Bug 53591 has been marked as a duplicate of this bug. ***

*** Bug 53591 has been marked as a duplicate of this bug. ***

Changing version to 3.4.4 as this is the earliest release in which I can reproduce the behaviour.

Alex

Changing version to 3.4.4 as this is the earliest release in which I can reproduce the behaviour.

Alex

Changed title to better reflect situation

Changed title to better reflect situation

On pc Debian x86-64 with master sources updated today, I could reproduce this.

After a gdb session, it seems the problem is here:
    224 if ( _pType->aCreateParams.isEmpty() )
    225 {
    226 SetPrecision(_pType->nPrecision);
    227 SetScale(_pType->nMinimumScale);
    228 }

http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/tabledesign/FieldDescriptions.cxx#224

Indeed, the types which have the problem have _pType->aCreateParams not empty and so don't enter the if block.

NUMBER => "PRECISION,SCALE"
VARCHAR => "LENGTH"
etc.

Lionel: What's the purpose of this if block? Could it be removed?

On pc Debian x86-64 with master sources updated today, I could reproduce this.

After a gdb session, it seems the problem is here:
    224 if ( _pType->aCreateParams.isEmpty() )
    225 {
    226 SetPrecision(_pType->nPrecision);
    227 SetScale(_pType->nMinimumScale);
    228 }

http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/tabledesign/FieldDescriptions.cxx#224

Indeed, the types which have the problem have _pType->aCreateParams not empty and so don't enter the if block.

NUMBER => "PRECISION,SCALE"
VARCHAR => "LENGTH"
etc.

Lionel: What's the purpose of this if block? Could it be removed?

Forgot to say I was looking "precision" because of this line of the same file:
   1422 if( pLength )
   1423 pLength->SetText( OUString::number(pFieldDescr->GetPrecision()) );
See http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/control/FieldDescControl.cxx#1422

Forgot to say I was looking "precision" because of this line of the same file:
   1422 if( pLength )
   1423 pLength->SetText( OUString::number(pFieldDescr->GetPrecision()) );
See http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/control/FieldDescControl.cxx#1422

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);

since _bReset seems to deal with SetFormatKey and SetControlDefault, see lines 176..180 of this same file.

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);

since _bReset seems to deal with SetFormatKey and SetControlDefault, see lines 176..180 of this same file.

(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.

(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.

Created attachment 103650
bts

After having put a break on SetPrecision, I retrieved differents bts showing 2 tests:
1) Varchar => Numeric (where precision isn't changed) which call 2 times SetPrecision()
2) Numeric => Real (where precision changed) which call 3 times SetPrecision() (because _pType->aCreateParams.isEmpty() - see my previous comment - is true)

If it can help...

Created attachment 103650
bts

After having put a break on SetPrecision, I retrieved differents bts showing 2 tests:
1) Varchar => Numeric (where precision isn't changed) which call 2 times SetPrecision()
2) Numeric => Real (where precision changed) which call 3 times SetPrecision() (because _pType->aCreateParams.isEmpty() - see my previous comment - is true)

If it can help...

Adding self to CC if not already on

Adding self to CC if not already on

** Please read this message in its entirety before responding **

To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year.

There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present.

If you have time, please do the following:

Test to see if the bug is still present on a currently supported version of LibreOffice
(5.1.5 or 5.2.1 https://www.libreoffice.org/download/

If the bug is present, please leave a comment that includes the version of LibreOffice and
your operating system, and any changes you see in the bug behavior

If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave
a short comment that includes your version of LibreOffice and Operating System

Please DO NOT

Update the version field
Reply via email (please reply directly on the bug tracker)
Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not
appropriate in this case)

If you want to do more to help you can test to see if your issue is a REGRESSION. To do so:
1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3)

http://downloadarchive.documentfoundation.org/libreoffice/old/

2. Test your bug
3. Leave a comment with your results.
4a. If the bug was present with 3.3 - set version to "inherited from OOo";
4b. If the bug was not present in 3.3 - add "regression" to keyword

Feel free to come ask questions or to say hello in our QA chat: http://webchat.freenode.net/?channels=libreoffice-qa

Thank you for helping us make LibreOffice even better for everyone!

Warm Regards,
QA Team

MassPing-UntouchedBug-20160920

Dear Sandra Farnedi,

To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year.

There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present.

If you have time, please do the following:

Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/

If the bug is present, please leave a comment that includes the information from Help - About LibreOffice.

If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice.

Please DO NOT

Update the version field
Reply via email (please reply directly on the bug tracker)
Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not
appropriate in this case)

If you want to do more to help you can test to see if your issue is a REGRESSION. To do so:
1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from http://downloadarchive.documentfoundation.org/libreoffice/old/

2. Test your bug
3. Leave a comment with your results.
4a. If the bug was present with 3.3 - set version to 'inherited from OOo';
4b. If the bug was not present in 3.3 - add 'regression' to keyword

Feel free to come ask questions or to say hello in our QA chat: https://kiwiirc.com/nextclient/irc.freenode.net/#libreoffice-qa

Thank you for helping us make LibreOffice even better for everyone!

Warm Regards,
QA Team

MassPing-UntouchedBug

Dear Sandra Farnedi,

To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year.

There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present.

If you have time, please do the following:

Test to see if the bug is still present with the latest version of LibreOffice from https://www.libreoffice.org/download/

If the bug is present, please leave a comment that includes the information from Help - About LibreOffice.

If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a comment that includes the information from Help - About LibreOffice.

Please DO NOT

Update the version field
Reply via email (please reply directly on the bug tracker)
Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not
appropriate in this case)

If you want to do more to help you can test to see if your issue is a REGRESSION. To do so:
1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) from https://downloadarchive.documentfoundation.org/libreoffice/old/

2. Test your bug
3. Leave a comment with your results.
4a. If the bug was present with 3.3 - set version to 'inherited from OOo';
4b. If the bug was not present in 3.3 - add 'regression' to keyword

Feel free to come ask questions or to say hello in our QA chat: https://web.libera.chat/?settings=#libreoffice-qa

Thank you for helping us make LibreOffice even better for everyone!

Warm Regards,
QA Team

MassPing-UntouchedBug