Bug 116987 - FIREBIRD : Migration : error on importing tables or queries with names longer than 30 characters
Summary: FIREBIRD : Migration : error on importing tables or queries with names longer...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.1.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.1.0
Keywords: bibisected, bisected
Depends on:
Blocks: Database-Firebird-Migration
  Show dependency treegraph
 
Reported: 2018-04-13 09:18 UTC by Alex Thurgood
Modified: 2019-02-04 21:16 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Test hsqldb ODB file for migration (331.44 KB, application/vnd.oasis.opendocument.database)
2018-04-13 09:19 UTC, Alex Thurgood
Details
Test File (4.27 KB, application/vnd.oasis.opendocument.database)
2018-05-17 13:22 UTC, Drew Jensen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Thurgood 2018-04-13 09:18:52 UTC
Description:
1. Open the attached test hsqldb-ODB file in master. FWIW, this file is/was one of the test files used with the Getting Started with Base Guide (at the time created by Dan Lewis).

2. Notice how the import fails with the following error:

The connection to the data source "Budget" could not be established.
Error code: 1

firebird_sdbc error:
*Dynamic SQL Error
*SQL error code = -104
*Name longer than database column size
caused by
'isc_dsql_prepare'

Steps to Reproduce:
See above

Actual Results:  
The tables fail to be created - import fails.

Expected Results:
The import should succeed and all data should be accessible.
The file is opened correctly in LO6032.


Reproducible: Always


User Profile Reset: No



Additional Info:


User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:59.0) Gecko/20100101 Firefox/59.0
Comment 1 Alex Thurgood 2018-04-13 09:19:43 UTC
Created attachment 141327 [details]
Test hsqldb ODB file for migration
Comment 2 Alex Thurgood 2018-04-13 09:23:08 UTC
Apparently, this is due to a limitation in Firebird, where column names are limited to 30 characters...

https://stackoverflow.com/questions/20155517/is-it-possible-to-extend-firebird-table-name-length
Comment 3 Alex Thurgood 2018-04-13 09:25:04 UTC
And apparently, from that same thread, only to be overcome in FB4, which is quite a while away.

In the meantime, any tables from hsqldb ODB files with names of more than 31 characters will silently fail to be imported...
Comment 4 Alex Thurgood 2018-04-13 09:32:22 UTC
In the sample file, none of the tables have names or columns with more than 30 characters.

The queries and reports have names with more than 30 characters.
Comment 5 Alex Thurgood 2018-04-13 09:38:15 UTC
Hmm, even if I rename all of the entries to less than 30 characters in LO6032, save under a different name, and re-import to master, I still get the same error.
Comment 6 Xisco Faulí 2018-04-13 10:13:17 UTC
Regression introduced by:

author	Tamas Bunth <tamas.bunth@collabora.co.uk>	2018-03-25 13:26:57 +0200
committer	Tamás Bunth <btomi96@gmail.com>	2018-04-07 17:08:26 +0200
commit	159dd28651788a19848eae56693ad06ed947414d (patch)
tree	b69013db1c0fa45677b59d58999ce0d8ebebd76c
parent	1a9bfdd8976d28fa3a56726bdcae9f2b294d6c6d (diff)
dbaccess: Enable hsql migration by default
Also make Firebird driver not experimental anymore.

Bisected with: bibisect-linux64-6.1

Adding Cc: to Tamas Bunth
Comment 7 Tamas Bunth 2018-04-26 12:09:51 UTC
A workaround would be to truncate table names with more than 30 characters.

If there are tables with the same name after the truncation, then put an
increasing number at the end. (e.g. truncate to 29 characters and the 30th
character would be a number to distinguish tables with the same name)

If there is more than 10 tables with the same name, then throw exception /
migration fails.
Comment 8 Alex Thurgood 2018-04-26 13:42:24 UTC
(In reply to Tamas Bunth from comment #7)
> A workaround would be to truncate table names with more than 30 characters.
> 


Not really a very satisfactory one though from a user perspective. The whole point of being able to uniquely identify a table in hsqldb with a "long" file name has clearly been used by DBAs and users alike. 

On the one hand, imposing a new form of table naming as you suggest isn't going to go down well with those users unless there is a valid reason, which there doesn't seem to be given that in FB4 that limitation will be lifted (but of course when that might happen is a different matter).

On the other hand, one could submit that having table names in excess of 30 characters really isn't the way one should be managing a DB in the first place, but that is a debate for another day.

Would it not be better to simply abort with a more meaningful message than is currently offered, something along the lines of :

"Firebird 3 doesn't currently support table/query/view names of more than 30 characters, please shorten your table names in the original file and try again."
Comment 9 Drew Jensen 2018-04-26 13:54:01 UTC
One note, querys or more acuratly queryDefinition names are not affected by the fb naming limits. Tables, Views, Triggers, Domains and Procedure names are.

In the current import function (drag drop of a table) a long table name is simply truncated at 30 characters.

As for a fully automated migration, after the name change the routine would need to check for other items using that identifier (querydefs, views, forms, reports and macros).
Comment 10 Commit Notification 2018-05-16 09:10:14 UTC
Tamas Bunth committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a1825b219a22be8b6cf0ca54bf0ee6c64f55dc6d

tdf#116987 dbahsql: check for table names..

It will be available in 6.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Drew Jensen 2018-05-16 14:46:36 UTC
(In reply to Commit Notification from comment #10)
> Tamas Bunth committed a patch related to this issue.
> It has been pushed to "master":
> 
> http://cgit.freedesktop.org/libreoffice/core/commit/
> ?id=a1825b219a22be8b6cf0ca54bf0ee6c64f55dc6d
> 
> tdf#116987 dbahsql: check for table names..
> 
> It will be available in 6.1.0.
> 
> The patch should be included in the daily builds available at
> http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
> information about daily builds can be found at:
> http://wiki.documentfoundation.org/Testing_Daily_Builds
> 
> Affected users are encouraged to test the fix and report feedback.

Howdy @Tamas

I looked over the code update and I have a concern regarding native language support. Specifically with this string addition in dbaccess/source/filter/hsqldb/utils.cxx:

+        constexpr char NAME_TOO_LONG[] = "Firebird 3 doesn't currently support table names of more "
+                                         "than 30 characters, please shorten your table names in "
+                                         "the original file and try again.";

I was concerned that this would create a hard coded English error message?
Comment 12 Drew Jensen 2018-05-17 13:22:26 UTC
Created attachment 142161 [details]
Test File

Single table test file with a column name longer than 30 characters.
Comment 13 Drew Jensen 2018-05-17 13:25:46 UTC
Good news bad news:
good news, using this mornings build and testing with the file I attached today this function works, the migration is halted. Picky little point the failure is due to a column name but the error message reports it as a long table name.

Bad News:
Using the test file from earlier (budget.odg) with Ubuntu (18.04) and  this build:
Version: 6.1.0.0.alpha1+
Build ID: 47dc3115f12ff16dc326b6edd12c46e6a6ef1843
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk2; 
TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time: 2018-05-17_00:32:17
Locale: en-US (en_US.UTF-8); Calc: group

Libo goes into a race condition and must be forced to shut down.
Comment 14 Xisco Faulí 2018-05-28 11:56:30 UTC
> Bad News:
> Using the test file from earlier (budget.odg) with Ubuntu (18.04) and  this
> build:
> Version: 6.1.0.0.alpha1+
> Build ID: 47dc3115f12ff16dc326b6edd12c46e6a6ef1843
> CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk2; 
> TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time:
> 2018-05-17_00:32:17
> Locale: en-US (en_US.UTF-8); Calc: group
> 
> Libo goes into a race condition and must be forced to shut down.

Could you please report it in a new bug ?
Comment 15 Drew Jensen 2018-05-28 12:12:24 UTC
(In reply to Xisco Faulí from comment #14)
> > Bad News:
> > Using the test file from earlier (budget.odg) with Ubuntu (18.04) and  this
> > build:
> > Version: 6.1.0.0.alpha1+
> > Build ID: 47dc3115f12ff16dc326b6edd12c46e6a6ef1843
> > CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk2; 
> > TinderBox: Linux-rpm_deb-x86_64@70-TDF, Branch:master, Time:
> > 2018-05-17_00:32:17
> > Locale: en-US (en_US.UTF-8); Calc: group
> > 
> > Libo goes into a race condition and must be forced to shut down.
> 
> Could you please report it in a new bug ?

Done, patched and closed already. Welcome back from vacation btw.

https://bugs.documentfoundation.org/show_bug.cgi?id=117670
Comment 16 Drew Jensen 2018-05-28 12:32:16 UTC
However regarding new issues from this one.

Right now there are a couple of things going on here.

1 - False positives for this 30 character limit are happening. Two test files which as far as I can tell have no identifiers over 30 characters (Table, Column, View, Relation, Trigger, Domain, Generator or Sequence, or Stored Procedure) yet trigger this error message anyhow, the first file attached to this issue is one and a different one on my disc.

2 - Another issue was recently resolved about the migration assistant continuing after an error. For this 30 character limit error (and it seems only this one) the migration assistant still bails at that point.

3 - The error message doesn't correctly state which type of identifier triggered it.

There has not been new issues opened for those.
Comment 17 Xisco Faulí 2018-06-06 21:15:12 UTC
(In reply to Drew Jensen from comment #16)
> However regarding new issues from this one.
> 
> Right now there are a couple of things going on here.
> 
> 1 - False positives for this 30 character limit are happening. Two test
> files which as far as I can tell have no identifiers over 30 characters
> (Table, Column, View, Relation, Trigger, Domain, Generator or Sequence, or
> Stored Procedure) yet trigger this error message anyhow, the first file
> attached to this issue is one and a different one on my disc.
> 
> 2 - Another issue was recently resolved about the migration assistant
> continuing after an error. For this 30 character limit error (and it seems
> only this one) the migration assistant still bails at that point.
> 
> 3 - The error message doesn't correctly state which type of identifier
> triggered it.
> 
> There has not been new issues opened for those.

Hello Drew,
Could you please create a new report for each point you mentioned?

@Alex, OTOH, since the original problem is no longer happening, should it be closed as RESOLVED FIXED ?
Comment 18 Xisco Faulí 2018-06-07 10:48:58 UTC
Lowering severity as the error reporting in the original report has been fixed...
Comment 19 Drew Jensen 2018-06-08 00:44:55 UTC
I'd agree. Just close this as fixed. 

I think an issues for the UI parts (table vs column vs view) is appropriate, even if minor, and I'll go ahead and do that.

I think is would be worth while to open a RFE for a simple automatic name change, the added task of sweeping through sql statements to replace the old name for new name is IMO a reasonably straight forward coding task. 

An RFE because it would be a 'like' over a 'must have' type of feature.
Comment 20 Drew Jensen 2018-06-08 00:51:14 UTC
Also, neither the attached budget.odb file nor the file on my local disc trigger the 30 character error message any longer. However, they do seem to have a common denominator, they both have table with more than 1 space in the name. But that is a different (currently closed) issue ;)