Bug 131295 - Error while saving the table design when changing structure
Summary: Error while saving the table design when changing structure
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.2.8.2 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-11 20:54 UTC by Damian Hofmann
Modified: 2023-04-30 06:43 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Damian Hofmann 2020-03-11 20:54:59 UTC
================================================================
Error code: 1

firebird_sdbc error:
*unsuccessful metadata update
*ALTER TABLE test failed
*SQL error code = -607
*Invalid command
*Specified domain or source column SQL_INT64 does not exist
caused by
'ALTER TABLE "test" ADD "name" SQL_INT64'
================================================================


Steps to reproduce:

1. in LO Base
2. create a table 'test' with an INTEGER column 'id' (primary key) and a NUMERIC (sic) column 'name'
3. Save and exit the table designer
4. Open table again in the table designer
5. Change "Field Type" for 'name' from NUMERIC to VARCHAR
6. "Save"
7. Confirm "The column "name" could not be changed. Should the column instead be deleted and the new format appended?"

Result:

The error above occurs. Note how the error massage says, that it tried to add a column of type SQL_INT64, but VARCHAR was selected by the user for the new column type.

Secondary finding: After the error, the column 'name' doesn't exist anymore. It apparently has been dropped. AFAIK, Firebird supports transactional DDL. Therefore, if an error occurs, I'd expect the database to be rolled back to it's original state. 



Tested with firebird. Not tested with HSQLDB.


About LibreOffice
Version: 6.2.8.2 (x64)
Build ID: f82ddfca21ebc1e222a662a32b25c0c9d20169ee
CPU threads: 8; OS: Windows 10.0; UI render: GL; VCL: win; 
Locale: de-CH (de_CH); UI-Language: en-US
Comment 1 Julien Nabet 2020-03-12 08:07:34 UTC
On pc Debian x86-64 with master sources updated yesterday, I could reproduce this.

As a workaround, when changing to VARCHAR, you should change length to 20, you may be able to save afterwards.
Could you give it a try?
Comment 2 Julien Nabet 2020-03-12 10:59:06 UTC
About the workaroud, please first install a more recent LO version, either last stable one 6.3.5 or very last one 6.4.1
Comment 3 Julien Nabet 2020-03-12 14:41:02 UTC
Here's the Windbg stack when changing type of the field:
dbulo!dbaui::OFieldDescription::FillFromTypeInfo [C:\BLP\core\dbaccess\source\ui\tabledesign\FieldDescriptions.cxx @ 169] 
dbulo!dbaui::OTableRow::SetFieldType+0xb0 [C:\BLP\core\dbaccess\source\ui\tabledesign\TableRow.cxx @ 90] 
dbulo!dbaui::OTableEditorCtrl::SwitchType+0x101 [C:\BLP\core\dbaccess\source\ui\tabledesign\TEditControl.cxx @ 1571] 
dbulo!dbaui::OTableEditorCtrl::resetType+0x80 [C:\BLP\core\dbaccess\source\ui\tabledesign\TEditControl.cxx @ 693] 
dbulo!dbaui::OTableEditorCtrl::CellModified+0x826 [C:\BLP\core\dbaccess\source\ui\tabledesign\TEditControl.cxx @ 676] 
dbulo!dbaui::OTableEditorCtrl::CellModified+0x45 [C:\BLP\core\dbaccess\source\ui\tabledesign\TEditControl.cxx @ 701] 
svtlo!svt::EditBrowseBox::CellModifiedHdl+0x3a [C:\BLP\core\svtools\source\brwbox\editbrowsebox.cxx @ 1052] 
svtlo!svt::EditBrowseBox::LinkStubCellModifiedHdl+0x26 [C:\BLP\core\svtools\source\brwbox\editbrowsebox.cxx @ 1048] 
vcllo!Link<void *,void>::Call+0x43 [C:\BLP\core\include\tools\link.hxx @ 111] 
vcllo!ImplHandleUserEvent+0x47 [C:\BLP\core\vcl\source\window\winproc.cxx @ 2012]
Comment 4 Julien Nabet 2020-03-12 15:23:01 UTC
The problem isn't Firebird only, depending of the old/new field, length updates or not.

Following bt retrieved previously, I noticed this commit:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=465cf674e8118922029e7e117fdae91e887bcee0
INTEGRATION: CWS apps61beta2 (1.11.2); FILE MERGED
2003/04/01 10:56:20 oj 1.11.2.1: #108512# correct precision handling

-                    if ( _pType->nPrecision && _pType->nMaximumScale )
-                    {
-                        SetPrecision(nPrec ? nPrec : DEFAULT_NUMERIC_PRECSION);
+                    if ( _pType->nPrecision )
+                        SetPrecision(::std::min<sal_Int32>(nPrec ? nPrec : DEFAULT_NUMERIC_PRECSION,_pType->nPrecision));
+                    if ( _pType->nMaximumScale )
                         SetScale(::std::min<sal_Int32>(GetScale() ? GetScale() : DEFAULT_NUMERIC_SCALE,_pType->nMaximumScale));
-                    }
-                    else if ( _pType->nPrecision )
-                        SetPrecision(::std::min<sal_Int32>(nPrec,_pType->nPrecision));

Lionel: the whole "switch" in OFieldDescription::FillFromTypeInfo seems a bit strange
See https://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/tabledesign/FieldDescriptions.cxx?r=a41f9b3a#181

Why we would take into account these elements(in addition to the precision of new type):
- the precision from previous type
- a default value
- a min of a combination of these?
- new type (do we really need all this switch?)

I mean, in case of bForce is true, why couldn't we just use precision of new element and only take the previous one (or a by default one, DEFAULT_VARCHAR_PRECISION/DEFAULT_OTHER_PRECISION) if there's no precision defined for new element?

Remark as a side part:
About DEFAULT_VARCHAR_PRECISION/DEFAULT_OTHER_PRECISION, we could check a precision has been defined with an assert. I mean perhaps we should force to define precision for all types of all DB drivers?
Comment 5 Julien Nabet 2020-03-12 15:42:35 UTC
(related: I got the same thoughts with scale part)
Comment 6 Damian Hofmann 2020-03-12 18:46:47 UTC
(In reply to Julien Nabet from comment #2)
> About the workaroud, please first install a more recent LO version, either
> last stable one 6.3.5 or very last one 6.4.1

Thanks for the quick response.

Tested proposed workaround, but on LO 6.2.8.2. Doesn't seem to work there. I'm not updating to a more recent version of LO because of unrelated bugs in the newer versions (broken UI on HiDPI screens)

I have found another workaround:

1. Drop the column
2. Save
3. Add column with new type
4. Save
Comment 7 Julien Nabet 2020-03-12 19:33:26 UTC
(In reply to Damian Hofmann from comment #6)
> (In reply to Julien Nabet from comment #2)
> > About the workaroud, please first install a more recent LO version, either
> > last stable one 6.3.5 or very last one 6.4.1
> 
> Thanks for the quick response.
> 
> Tested proposed workaround, but on LO 6.2.8.2. Doesn't seem to work there.
> I'm not updating to a more recent version of LO because of unrelated bugs in
> the newer versions (broken UI on HiDPI screens)
> 
> I have found another workaround:
> 
> 1. Drop the column
> 2. Save
> 3. Add column with new type
> 4. Save

Ok then.
Just for information, if a fix is found, it'll be on master sources (future LO release 7.0), 6.4 branch and 6.3 branch. 6.2 branch is EOL so won't be fixed (at least not from official TDF source)
Comment 8 Damian Hofmann 2020-08-18 16:04:58 UTC
Retested with:

Version: 7.0.0.3 (x64)
Build ID: 8061b3e9204bef6b321a21033174034a5e2ea88e
CPU threads: 8; OS: Windows 10.0 Build 19041; UI render: Skia/Raster; VCL: win
Locale: de-CH (de_CH); UI: en-US
Calc: threaded

It kind of works now. No longer getting the error and the change is applied correctly. But the Table Designer seems to have a refresh issue:

1. in LO Base
2. create a table 'test' with an INTEGER column 'id' (primary key) and a NUMERIC (sic) column 'name'
3. Save and exit the table designer
4. Open table again in the table designer
5. Change "Field Type" for 'name' from NUMERIC to VARCHAR
6. "Save"
7. Confirm "The column "name" could not be changed. Should the column instead be deleted and the new format appended?"

No error message this time. But the table designer still reports the column type as NUMERIC. 

8. Exit the table designer
9. Open table again in table designer

Column type still reported as NUMERIC

10. Don't change anything and exit table designer
11. Save and close database document
12. Open database document agai
13. Open table again in table designer

Column type is correctly reported as VARCHAR
Comment 9 Julien Nabet 2020-08-18 18:02:11 UTC
Can't help here => uncc myself.
Comment 10 QA Administrators 2022-08-19 03:43:06 UTC Comment hidden (obsolete)
Comment 11 Damian Hofmann 2022-10-19 22:49:14 UTC
Retested with LO 7.3.5.2 and still reproducible as described in comment #8

Version: 7.3.5.2 (x64) / LibreOffice Community
Build ID: 184fe81b8c8c30d8b5082578aee2fed2ea847c01
CPU threads: 8; OS: Windows 10.0 Build 19044; UI render: Skia/Raster; VCL: win
Locale: de-CH (de_CH); UI: en-US
Calc: threaded