Bug 121553 - Editing: Firebird: Table editor erroneously prompts to change CLOB field to a BLOB field during alter table
Summary: Editing: Firebird: Table editor erroneously prompts to change CLOB field to a...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.2.0.0.beta1+
Hardware: All All
: high major
Assignee: Not Assigned
URL:
Whiteboard: target:7.1.0 target:7.0.4
Keywords:
Depends on:
Blocks: Database-Firebird-Default
  Show dependency treegraph
 
Reported: 2018-11-20 14:27 UTC by Drew Jensen
Modified: 2020-11-21 15:07 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Test file (3.00 KB, application/vnd.oasis.opendocument.database)
2018-11-20 14:27 UTC, Drew Jensen
Details
Screen shot of erroneous error message (65.37 KB, image/png)
2018-11-20 14:28 UTC, Drew Jensen
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Drew Jensen 2018-11-20 14:27:15 UTC
Created attachment 146837 [details]
Test file

The Firbird support in Base added CLOB to the list recently.
Using the table editor to create a CLOB field in a table works initially.
However if you attempt to alter the table structure subsequently by adding a new column then during the ALTER TABLE process the CLOB field is first erroneously reported as needing to be dropped and added to the back of the table columns, but will change the type of field from CLOB to BLOB.

Steps to reproduce:
1 - download and open the attached test file.
2 - Open the table TABLE2 with the Table Editor
3 - Add a new column after Notes of any name and type
4 - Click on save changes.

Result:
User is prompted (see second attachment for screen shot) that the Notes field can not be changed (OK) and instead should it be dropped and appended with new format.

This is because what the application really wants to do is change the column type to BLOB (a type that was supported with older versions of LO/Firebird when it did not support CLOB), so if you the user answers yes it does just that (which looses any data currently in the column for all records) and the user is now unable to edit, insert, etc any data in the changed field. If the user is sharp and answers no there is no harm.

Expected result:
The application adds the new column silently.

Setting this to priority 'High' because of the data loss potential.
Comment 1 Drew Jensen 2018-11-20 14:28:13 UTC
Created attachment 146838 [details]
Screen shot of erroneous error message

screen shot of error box message.
Comment 2 Robert Großkopf 2018-11-20 15:14:33 UTC
I have tested it with both tables:
The "notes"-field, defined as BLOB[BLOB], needs an <OBJECT> for input.
The "notes"-field, defined as CLOB[BLOB], could contain text.

Both fields show the same irritating and wrong errormessage. You have to switch to "No" to prevent dataloss.

By the way: What is the real type of the fields? With HSQLDB the real type is described in square brackets, and this is the same here ([BLOB]).

Tested with LO 6.1.3.2, OpenSUSE 15, 64bit rpm Linux
Comment 3 Drew Jensen 2018-11-20 15:44:49 UTC
Real type.

BLOB and CLOB are two types of LOB, to be AR about it.

From Firebird perspective a CLOB is a specialized sub type of BLOB with the C in CLOB standing for Character and the BLOB being a generic binary encoded large object. CLOB signifies to client software that the binary data may be retrieved as character data. 

Same is true of the Image field type in Base, which is another BLOB sub type.

So, it is accurate in the table editor to, as it does, display the types as:
BLOB  [BLOB]
CLOB  [BLOB]
IMAGE [BLOB]

The actual equivalent to the CLOB [BLOB] field in the HSQLdb SDBC was the field type MEMO [BLOB].
Comment 4 RolandVL 2019-05-13 16:52:34 UTC
This bug is also in LibreOffice Version: 6.1.5.2 ("Stable Version")
Comment 5 Julien Nabet 2020-10-29 22:35:35 UTC
Lionel: I tried 2 ways to fix this:
1) In ODatabaseMetaData::getTypeInfo(), replace:
aRow[1] = new ORowSetValueDecorator(OUString("BLOB")); // BLOB, with subtype 1
by
aRow[1] = new ORowSetValueDecorator(OUString("CLOB")); // BLOB, with subtype 1
See 
https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/firebird/DatabaseMetaData.cxx?r=1daeea3a#885

2) In firebird::ColumnTypeInfo::getColumnTypeName()
Replace:
         case DataType::CLOB:
            return "CLOB";
by
         case DataType::CLOB:
            return "BLOB";
See connectivity/source/drivers/firebird/Util.cxx (not present in Opengrok yet since the patch is recent).

But considering http://www.firebirdfaq.org/faq48/, if we want to stick to Firebird naming, I think 2) should be chosen.
Now I must recognize it's not easy to think the impacts of each way.

Any thoughts?
Comment 6 Julien Nabet 2020-10-29 22:36:04 UTC
Of course, related with tdf#137801
Comment 7 Lionel Elie Mamane 2020-10-30 06:04:50 UTC
Try:

        // Clob (SQL_BLOB SUBTYPE TEXT)
-       aRow[1] = new ORowSetValueDecorator(OUString("BLOB")); // BLOB, with subtype 1
+       aRow[1] = new ORowSetValueDecorator(OUString("BLOB SUBTYPE TEXT")); // BLOB, with subtype 1
        aRow[2] = new ORowSetValueDecorator(DataType::CLOB);
        aRow[3] = new ORowSetValueDecorator(sal_Int32(2147483647)); // Precision = max length
        aRow[6] = new ORowSetValueDecorator(); // Create Params
        aRow[9] = new ORowSetValueDecorator(
                sal_Int16(ColumnSearch::FULL)); // Searchable
        aRow[12] = new ORowSetValueDecorator(false); // Autoincrement
        aRow[14] = ODatabaseMetaDataResultSet::get0Value(); // Minimum scale
        aRow[15] = ODatabaseMetaDataResultSet::get0Value(); // Max scale
        tmp.push_back(aRow);

        // Longvarbinary (SQL_BLOB SUBTYPE BINARY)
        // Distinguished from simple blob with a user-defined subtype.
+       aRow[1] =         aRow[2] = new ORowSetValueDecorator(DataType::LONGVARBINARY);
        aRow[2] = new ORowSetValueDecorator(DataType::LONGVARBINARY);
        tmp.push_back(aRow);

and

In firebird::ColumnTypeInfo::getColumnTypeName()
Replace:
         case DataType::CLOB:
            return "CLOB";
by
         case DataType::CLOB:
            return "BLOB SUBTYPE TEXT";
and replace
         case DataType::BLOB:
            return "BLOB";
by
         case DataType::CLOB:
            return "BLOB SUBTYPE BINARY";
Comment 8 Julien Nabet 2020-10-30 12:18:38 UTC
(In reply to Lionel Elie Mamane from comment #7)
> Try:
> 
>         // Clob (SQL_BLOB SUBTYPE TEXT)
> -       aRow[1] = new ORowSetValueDecorator(OUString("BLOB")); // BLOB, with
> subtype 1
> +       aRow[1] = new ORowSetValueDecorator(OUString("BLOB SUBTYPE TEXT"));
> // BLOB, with subtype 1
>         aRow[2] = new ORowSetValueDecorator(DataType::CLOB);
>         aRow[3] = new ORowSetValueDecorator(sal_Int32(2147483647)); //
> Precision = max length
>         aRow[6] = new ORowSetValueDecorator(); // Create Params
>         aRow[9] = new ORowSetValueDecorator(
>                 sal_Int16(ColumnSearch::FULL)); // Searchable
>         aRow[12] = new ORowSetValueDecorator(false); // Autoincrement
>         aRow[14] = ODatabaseMetaDataResultSet::get0Value(); // Minimum scale
>         aRow[15] = ODatabaseMetaDataResultSet::get0Value(); // Max scale
>         tmp.push_back(aRow);
> 
>         // Longvarbinary (SQL_BLOB SUBTYPE BINARY)
>         // Distinguished from simple blob with a user-defined subtype.
> +       aRow[1] =         aRow[2] = new
> ORowSetValueDecorator(DataType::LONGVARBINARY);
>         aRow[2] = new ORowSetValueDecorator(DataType::LONGVARBINARY);
>         tmp.push_back(aRow);
> 
> and
> 
> In firebird::ColumnTypeInfo::getColumnTypeName()
> Replace:
>          case DataType::CLOB:
>             return "CLOB";
> by
>          case DataType::CLOB:
>             return "BLOB SUBTYPE TEXT";
> and replace
>          case DataType::BLOB:
>             return "BLOB";
> by
>          case DataType::CLOB:
>             return "BLOB SUBTYPE BINARY";

Remark I suppose the last one is:
> and replace
>          case DataType::BLOB:
>             return "BLOB";
> by
>          case DataType::BLOB:
>             return "BLOB SUBTYPE BINARY";

Anyway, it'll work for this case but it'll regress for "tdf#137801"
since I got:
sTypeName=SMALLINT pField->GetTypeName()=BLOB SUBTYPE BINARY
In connectivity/source/drivers/firebird/Util.cxx, I used:
         case DataType::BLOB:
-            return "BLOB";
+            return "BLOB SUBTYPE BINARY";
*Dynamic SQL Error
*SQL error code = -104
*Token unknown - line 1, column 36
*SUBTYPE
caused by
'isc_dsql_prepare'

It seems we can modify table containing a CLOB (and I suppose a BLOB) but we can't create a brand new table when adding a BLOB or CLOB because of the "SUBTYPE ..."
Comment 9 Lionel Elie Mamane 2020-10-30 16:10:03 UTC
(In reply to Julien Nabet from comment #8)
> (In reply to Lionel Elie Mamane from comment #7)
> Remark I suppose the last one is:
> > and replace
> >          case DataType::BLOB:
> >             return "BLOB";
> > by
> >          case DataType::BLOB:
> >             return "BLOB SUBTYPE BINARY";

Yes.

> Anyway, it'll work for this case but it'll regress for "tdf#137801"
> since I got:
> sTypeName=SMALLINT pField->GetTypeName()=BLOB SUBTYPE BINARY

Sorry, the correct token is SUB_TYPE, not SUBTYPE. Replace everywhere. See https://firebirdsql.org/file/documentation/pdf/en/refdocs/fblangref25/firebird-25-language-reference.pdf section 3.6 page 38.
Comment 10 Julien Nabet 2020-10-30 17:50:41 UTC
(In reply to Lionel Elie Mamane from comment #9)
> ...
> Sorry, the correct token is SUB_TYPE, not SUBTYPE. Replace everywhere. See
> https://firebirdsql.org/file/documentation/pdf/en/refdocs/fblangref25/
> firebird-25-language-reference.pdf section 3.6 page 38.
I can create a BLOB field but not a CLOB field.

warn:connectivity.firebird:3720:3720:connectivity/source/drivers/firebird/Util.cxx:57: firebird_sdbc error:
*Dynamic SQL Error
*SQL error code = -104
*Token unknown - line 1, column 70
*SUB_TYPE
caused by
'isc_dsql_prepare'
Comment 11 Commit Notification 2020-10-31 21:17:14 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/aba06f2c3a39f33007a8f4e6e234254f42e01f0d

tdf#121553: Firebird, fix datatypes management

It will be available in 7.1.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 12 Julien Nabet 2020-10-31 21:18:07 UTC
Backport for 7.0 waiting for review here:
https://gerrit.libreoffice.org/c/core/+/105050
Comment 13 Commit Notification 2020-11-05 19:00:27 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/101647617f38a40dabbe9fb66b71ee9b2db5f2ae

tdf#121553: Firebird, fix datatypes management

It will be available in 7.0.4.

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

Affected users are encouraged to test the fix and report feedback.
Comment 14 trowelandmattock 2020-11-21 14:32:39 UTC
Great to hear this issue is getting fixes:)

To add a little note:

CLOB fields which are larger than standard VARCHAR max do not display correctly in LO - they appear to be truncated to VARCHAR max, eg in form text-fields, and (unsurprisingly) in calc cells....
...since there is also an issue exporting .csv directly from Base+Firebird (oops-lost link) this becomes a potentially big issue>>

ie: CLOB fields larger than VARCHAR max may become truncated and irrecoverable by 'normal' means !!! :O

(perhaps a LO project could  be fully reconstructed in external DB...but not exactly for the standard user)
Comment 15 Julien Nabet 2020-11-21 15:07:39 UTC
(In reply to trowelandmattock from comment #14)
> ...
> CLOB fields which are larger than standard VARCHAR max do not display
> correctly in LO - they appear to be truncated to VARCHAR max, eg in form
> text-fields, and (unsurprisingly) in calc cells....
> ...since there is also an issue exporting .csv directly from Base+Firebird
> (oops-lost link) this becomes a potentially big issue>>
> ...
Don't hesitate to submit a new bugtracker about this if there's none which corresponds.