Bug 121092 - Libreoffice 6.1.3.2 does not allow saving Advanced Properties options with SQLITE 3 over ODBC
Summary: Libreoffice 6.1.3.2 does not allow saving Advanced Properties options with SQ...
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.1.3.2 release
Hardware: All Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.3.0
Keywords: bibisectRequest, regression
Depends on:
Blocks:
 
Reported: 2018-11-01 11:27 UTC by jorojmaqui
Modified: 2019-03-13 06:42 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
sqlite 3 odbc not saved in libreoffice 6.1.3.2 (184.50 KB, application/pdf)
2018-11-06 20:45 UTC, jorojmaqui
Details
Attached file in pdf to improve the explanation (163.14 KB, application/pdf)
2018-12-04 05:59 UTC, jorojmaqui
Details
sqlite 3 odbc disable (164.33 KB, application/pdf)
2019-01-22 10:34 UTC, jorojmaqui
Details
files libreofficebase test (6.51 MB, application/vnd.sun.xml.base)
2019-01-23 13:23 UTC, jorojmaqui
Details
files libreoffice base test (2.34 MB, application/x-sqlite3)
2019-01-23 13:25 UTC, jorojmaqui
Details
error01 (91.79 KB, application/pdf)
2019-02-28 08:35 UTC, jorojmaqui
Details

Note You need to log in before you can comment on or make changes to this bug.
Description jorojmaqui 2018-11-01 11:27:56 UTC
Description:
Libreoffice 6.1.3.2 does not allow saving the special configuration of SQLITE 3 ODBC, but version 6.1.2 if allowed

Greetings for the effort

Actual Results:
Libreoffice 6.1.3.2 does not allow saving the special configuration of SQLITE 3 ODBC, but version 6.1.2 if allowed

Expected Results:
Libreoffice 6.1.3.2 does not allow saving the special configuration of SQLITE 3 ODBC, but version 6.1.2 if allowed


Reproducible: Always


User Profile Reset: No



Additional Info:
Comment 1 jorojmaqui 2018-11-06 20:45:34 UTC
Created attachment 146375 [details]
sqlite 3 odbc not saved in libreoffice 6.1.3.2
Comment 2 Alex Thurgood 2018-11-07 08:59:43 UTC
Possibly DUP of bug 120695
Comment 3 jorojmaqui 2018-12-04 05:59:43 UTC
Created attachment 147257 [details]
Attached file in pdf to improve the explanation
Comment 4 jorojmaqui 2018-12-04 06:05:15 UTC
It is possible that the error 120695 is very similar to this one that I expose but in Windows 10

Greetings for the effort
Comment 5 jorojmaqui 2019-01-18 08:31:09 UTC
continues with the error in libreoffice 6.1.3.2 y libreoffice 6.2.0.2 on windows

I modified it to critical because if he can not work on the database with sqlite odbc
Comment 6 jorojmaqui 2019-01-18 08:31:56 UTC
windows 10
Comment 7 Xisco Faulí 2019-01-21 18:48:38 UTC
You can't confirm your own bugs. Moving it back to UNCONFIRMED until someone
else confirms it.
Comment 8 jorojmaqui 2019-01-22 10:34:04 UTC
Created attachment 148507 [details]
sqlite 3 odbc disable

The problem happens to me when we disable the box that is marked in red in the drawing, it does not allow the data that is in SQLITE 3 ODBC to appear in the form

drawing that I show you in the last pdf file that I put
Comment 9 jorojmaqui 2019-01-22 11:03:18 UTC
The base form does not allow me to see the fields that I have entered in the SQLITE 3 ODBC database and in the older libreoffices I did not have that problem.

Thanks for the effort
Comment 10 jorojmaqui 2019-01-23 13:23:47 UTC
Created attachment 148554 [details]
files libreofficebase test

Attached files libreoffice base for test
Comment 11 jorojmaqui 2019-01-23 13:25:41 UTC
Created attachment 148555 [details]
files libreoffice base test

Attached files libreoffice base for test
Comment 12 jorojmaqui 2019-02-01 12:07:10 UTC
Libreoffice should take into account the SQLITE ODBC database since it is a very interesting database as a second option after Firebird

here are many people who use SQLITE, a standardized database.


Thanks for the effort
Comment 13 Xisco Faulí 2019-02-05 17:38:56 UTC
Please, stop confirming your own bugs. Someone else needs to confirm them...
Comment 14 Alex Thurgood 2019-02-06 10:23:51 UTC
It may be that these options are no longer taken into account because LO currently seems incapable of reading the metadata of a sqlite3 db over ODBC, see bug 120695.

As it was, previously, there was never any guarantee that the options selected in the Advanced Properties dialogue would be respected, even if they did just happen to work.
Comment 15 jorojmaqui 2019-02-28 08:35:29 UTC
Created attachment 149646 [details]
error01
Comment 16 jorojmaqui 2019-02-28 08:38:16 UTC
Comment on attachment 149646 [details]
error01

windows home 10
libreoffice 6.2.1
sqlite3 odbc on libreoffice Base

error:

Hay un controlador no registrado para el URL ~sdbc:odbc:FINCAxSQLITE3xODBC.

what can be done?
Comment 17 Mike Kaganski 2019-03-05 18:37:18 UTC
It is confirmed by independent research on https://forumooo.ru/index.php/topic,7532.0.html, where another server (Firebird) was connected using ODBC. Newer versions of LO (reportedly 6.0+) could not perform specific tasks on the server (namely, add records), while creation and editing of tables was possible. It was reportedly OK with 5.4.7.2.

The problem was nailed down to the "Respect the result set type from the database driver" setting on the Advanced Settings dialog, where it is reset to enabled state upon reloading the odb, along with some other settings. Being unable to unset and save state drove the reporter to write a macro attached to "File Open" event:

> ThisDatabaseDocument = ThisComponent
> ThisDatabaseDocument.DataSource.Settings.RespectDriverResultSetType = false

Setting to NEW.
Comment 18 Mike Kaganski 2019-03-05 18:40:24 UTC
The research described in comment 17 was performed using Windows hosts and server.
Comment 19 Mike Kaganski 2019-03-05 18:45:20 UTC
I cannot myself bibisect this; so I ask original poster: could you please follow the steps described on [1], and perform a bibisection, which would give us the specific commit which is responsible for the regression?

[1] https://wiki.documentfoundation.org/QA/Bibisect/Windows
Comment 20 Alex Thurgood 2019-03-06 11:03:40 UTC
Any chance it is linked to :

commit d6fce54c82868b82bd6fa190db6047d69bbb3ecf
Comment 21 Alex Thurgood 2019-03-06 11:08:21 UTC
That commit, added a default value of True to the ODBC Drivers.xcu file in:

connectivity/registry/odbc/org/openoffice/Office/DataAccess/Drivers.xcu

@Julien : any thoughts ?
Comment 22 Alex Thurgood 2019-03-06 11:09:08 UTC
+        <node oor:name="RespectDriverResultSetType" oor:op="replace">
+          <prop oor:name="Value" oor:type="xs:boolean">
+            <value>true</value>
+          </prop>
+        </node>
Comment 23 Alex Thurgood 2019-03-06 11:13:31 UTC
However, it may be worthwhile bearing in mind why this change came about, cf. bug 119743
Comment 24 Julien Nabet 2019-03-06 13:43:42 UTC
Alex: yes it may be related with https://cgit.freedesktop.org/libreoffice/core/commit/?id=d6fce54c82868b82bd6fa190db6047d69bbb3ecf and https://cgit.freedesktop.org/libreoffice/core/commit/?id=3208fcb3a36d75d6290d9c548430682f153b09db

Now I don't know if it's the root cause or if it revealed a pb more than it's the rootcause.

Indeed, looking at where "RespectDriverResultSetType" is used, I noticed:
https://opengrok.libreoffice.org/xref/core/dbaccess/source/core/api/RowSet.cxx?r=435b64ec#1579

Since it's now considered to true, perhaps the if has some impact.

I think it needs some debugging
Comment 25 Lionel Elie Mamane 2019-03-06 13:51:32 UTC
Can someone try to change the "true" default added in https://cgit.freedesktop.org/libreoffice/core/commit/?id=3208fcb3a36d75d6290d9c548430682f153b09db to a "false" default? That might solve this.
Comment 26 Julien Nabet 2019-03-06 14:37:36 UTC
On Win10 + Cywin 3.01 + master sources updated today and enable-dbgutil + odbc sqlite driver Devart (trial), I could reproduce the pb.
When opening file, the option "Respect the resultset..." is checked.
If I uncheck this, save the file, then reopen the file, this option is checked again.

Then I applied what Lionel indicated and put false odbc\org\openoffice\Office\DataAccess for RespectDriverResultSetType values both in Features and Properties part, I don't reproduce this anymore.
Comment 27 Alex Thurgood 2019-03-06 17:23:27 UTC
(In reply to Julien Nabet from comment #26)


> Then I applied what Lionel indicated and put false
> odbc\org\openoffice\Office\DataAccess for RespectDriverResultSetType values
> both in Features and Properties part, I don't reproduce this anymore.

I suppose the question then is, what effect, if any, would changing these back to false have on other ODBC driver connections ? I assume (perhaps wrongly) that this current issue is specific to the properties of the sqlite ODBC driver, as opposed to some more general functionality ?

See, for example, my comment in bug122306#c4
Comment 28 Mike Kaganski 2019-03-06 18:44:29 UTC
Setting default to true or false looks like a workaround. The real problem is that the setting(s) apparently are not saved to or read from ODB, so a default is used after the reload. And that is despite the value(s) clearly have effect on the behavior of the connection.
Comment 29 Lionel Elie Mamane 2019-03-06 19:55:20 UTC
I think the values that the XML saving/parsing code considers are the default (which I'm pretty sure is a global hard-coded thing, not taken from the per-driver xcu config file!) are not saved explicitly in the XML stream; the attribute is just omitted.

That's the source of my suggestion in comment 25. My theory is that this interacts somewhat with "per driver" defaults in the xcu files in a way that does not quite work as intended.
Comment 30 Mike Kaganski 2019-03-09 09:46:39 UTC
A bibisect by user mentioned in comment 17 gives this:

https://gerrit.libreoffice.org/plugins/gitiles/core/+/6de679cca6978694bacf5322c9ab8559740f0443
Comment 31 Mike Kaganski 2019-03-09 09:58:30 UTC
And it was backported to 6-1 in https://gerrit.libreoffice.org/plugins/gitiles/core/+/8d0f2c5192d2c6bc18e406906825c0889e1edc22, which was included exactly in 6.1.3.
Comment 32 Julien Nabet 2019-03-09 10:31:40 UTC
Lionel: should I submit a patch with false following comments 25 and 26 even if it's just a workaround?
Comment 33 Mike Kaganski 2019-03-09 10:59:29 UTC
(In reply to Julien Nabet from comment #32)
> Lionel: should I submit a patch with false following comments 25 and 26 even
> if it's just a workaround?

Then those connections that rely on that value to be true would break :-)
Comment 34 Julien Nabet 2019-03-09 12:40:23 UTC
(In reply to Mike Kaganski from comment #33)
> (In reply to Julien Nabet from comment #32)
> > Lionel: should I submit a patch with false following comments 25 and 26 even
> > if it's just a workaround?
> 
> Then those connections that rely on that value to be true would break :-)
So what should we do then?
Comment 35 Mike Kaganski 2019-03-09 12:50:12 UTC
(In reply to Julien Nabet from comment #34)

Could you please check if reverting your commit referenced in comment 30 also has an impact (checking if the options are actually saved and persist over reopen); if so, could you check why that commit breaks saving/reloading the values?
Comment 36 Julien Nabet 2019-03-09 13:15:52 UTC
(In reply to Mike Kaganski from comment #35)
> ...

Taking a look at https://cgit.freedesktop.org/libreoffice/core/commit/?id=6de679cca6978694bacf5322c9ab8559740f0443 from comment 30, I see 4 files modified, they concern:
- ado
- jdbc
- mysql
- odbc
Since we're talking about odbc, I suppose we can consider ado, jdbc, mysql changes not related and only consider odbc one and so we got:
         <node oor:name="BooleanComparisonMode" oor:op="replace">
-          <prop oor:name="Value" oor:type="xs:boolean">
-            <value>true</value>
+          <prop oor:name="Value" oor:type="xs:int">
+            <value>0</value>
           </prop>
         </node>
         <node oor:name="FormsCheckRequiredFields" oor:op="replace">
@@ -165,7 +165,7 @@
         </node>
         <node oor:name="UseKeywordAsBeforeAlias" oor:op="replace">
           <prop oor:name="Value" oor:type="xs:boolean">
-            <value>false</value>
+            <value>true</value>
           </prop>
         </node>
Both don't concern "RespectDriverResultSetType" quoted in previous comments.

I'm a bit lost here.

Don't hesitate to revert what I did if you think it'll improve the situation.
I'm not deeply attached to my patches :-)
Comment 37 Mike Kaganski 2019-03-09 13:23:26 UTC
(In reply to Julien Nabet from comment #36)

I don't think that that patch should be reverted; only that something is missing (possibly some code checks types strictly, and fails now not getting expected type somewhere?). I can't test the bug myself - so I can't fix it; but you said that you can - that's why I suppose you are in the right position to check that and fix ;-)
Comment 38 Mike Kaganski 2019-03-09 13:28:56 UTC
(In reply to Julien Nabet from comment #36)
>          <node oor:name="BooleanComparisonMode" oor:op="replace">
> -          <prop oor:name="Value" oor:type="xs:boolean">
> -            <value>true</value>
> +          <prop oor:name="Value" oor:type="xs:int">
> +            <value>0</value>
>            </prop>
>          </node>
>          <node oor:name="FormsCheckRequiredFields" oor:op="replace">
> @@ -165,7 +165,7 @@
>          </node>
>          <node oor:name="UseKeywordAsBeforeAlias" oor:op="replace">
>            <prop oor:name="Value" oor:type="xs:boolean">
> -            <value>false</value>
> +            <value>true</value>
>            </prop>
>          </node>
> Both don't concern "RespectDriverResultSetType" quoted in previous comments.
> 
> I'm a bit lost here.

E.g., some code (that reads multiple settings, including RespectDriverResultSetType) tries to read boolean from BooleanComparisonMode (when now it's int), and that throws; so the rest is not read at all?
Comment 39 Mike Kaganski 2019-03-09 13:46:53 UTC
For reference: testing with setting all checkboxes in Advanced Settings dialog then reloading; and unsetting all checkboxes in the dialog then reloading, shows that the following settings are automatically reset to CHECKED:

* Use SQL92 naming constraints
* Use keyword AS before table alias names
* End text lines with CR+LF
* Respect the result set type from the database driver

These are automatically reset to UNCHECKED:

* Append the table alias name on SELECT statements
* Display version columns (when available)
Comment 40 Mike Kaganski 2019-03-09 16:14:32 UTC
I have setup a testing data connection, and tested myself.

I need to note that bibisect from comment 30 is indeed incorrect (though looks amazingly relevant wrt affected files ;-D).

And the suggestion from comment 25 seems to be not a workaround after all, but a proper solution:

ODatabaseModelImpl::getDefaultDataSourceSettings [1] sets the default for the setting to false; that default is most probably taken to consideration at *some* level when deciding if a value is different from default (i.e., it might be simply dropped from some property set if it's equal to this default). On the other hand, the change mentioned in comment 25 provides the opposite value for default on a different level; that might also be considered likewise in the process of writing the setting - so having them both makes the scenario likely that neither value gets it to the content.xml in ODB.

Since the defaults in [1] are there at least since 2006 [2], they must be relevant for all ODBs out there - so it's correct to change newly introduced defaults accordingly.

I have checked that making the proposed change to that default of RespectDriverResultSetType in connectivity/registry/odbc/org/openoffice/Office/DataAccess/Drivers.xcu indeed produces the "true" value into the ODB; while when "false", the value is not present there.

I'm not sure if that is relevant to other connection types.

[1] https://opengrok.libreoffice.org/xref/core/dbaccess/source/core/dataaccess/ModelImpl.cxx?r=435b64ec#1003
[2] https://git.libreoffice.org/core/+/0eb026293c88d422d701d1a06735d9cce5457ca9
Comment 41 Mike Kaganski 2019-03-09 16:21:01 UTC
And in the change, it's likely a good thing to add a comment there that the settings must be aligned with each other - so that other devs would be aware of this situation in the future.
Comment 42 Mike Kaganski 2019-03-09 16:29:49 UTC
And indeed, *exchanging* the two defaults for RespectDriverResultSetType - in ODatabaseModelImpl::getDefaultDataSourceSettings and connectivity/registry/odbc/org/openoffice/Office/DataAccess/Drivers.xcu - make the setting to not be written to ODB again, but this time, unchecked state becomes the auto-restoring setting (just tested) :-)

(In reply to Julien Nabet from comment #32)
> Lionel: should I submit a patch with false following comments 25 and 26

Julien: please do!
Comment 43 Julien Nabet 2019-03-09 16:45:03 UTC
(In reply to Mike Kaganski from comment #42)
...
> (In reply to Julien Nabet from comment #32)
> > Lionel: should I submit a patch with false following comments 25 and 26
> 
> Julien: please do!

done here: https://gerrit.libreoffice.org/#/c/68979/

I also noticed that:
    487 void ODBExport::exportApplicationConnectionSettings(const TSettingsMap& _aSettings)
    488 {
    489     const ::xmloff::token::XMLTokenEnum pSettings[] = {
    490         XML_IS_TABLE_NAME_LENGTH_LIMITED
    491         ,XML_ENABLE_SQL92_CHECK
    492         ,XML_APPEND_TABLE_ALIAS_NAME
    493         ,XML_IGNORE_DRIVER_PRIVILEGES
    494         ,XML_BOOLEAN_COMPARISON_MODE
    495         ,XML_USE_CATALOG
    496         ,XML_MAX_ROW_COUNT
    497         ,XML_SUPPRESS_VERSION_COLUMNS
    498     };
See https://opengrok.libreoffice.org/xref/core/dbaccess/source/filter/xml/xmlExport.cxx?r=b47bca7f#487

It seems we can't export any settings, see:
schema/odf1.2/OpenDocument-v1.2-os-schema.rng from line 9158
schema/odf1.3/OpenDocument-schema-v1.3.rng from line from line 3036
Comment 44 Mike Kaganski 2019-03-09 17:16:16 UTC
(In reply to Julien Nabet from comment #43)
> I also noticed that:
>     487 void ODBExport::exportApplicationConnectionSettings(const
> TSettingsMap& _aSettings)
>     488 {
>     489     const ::xmloff::token::XMLTokenEnum pSettings[] = {
>     490         XML_IS_TABLE_NAME_LENGTH_LIMITED
>     491         ,XML_ENABLE_SQL92_CHECK
>     492         ,XML_APPEND_TABLE_ALIAS_NAME
>     493         ,XML_IGNORE_DRIVER_PRIVILEGES
>     494         ,XML_BOOLEAN_COMPARISON_MODE
>     495         ,XML_USE_CATALOG
>     496         ,XML_MAX_ROW_COUNT
>     497         ,XML_SUPPRESS_VERSION_COLUMNS
>     498     };
> See
> https://opengrok.libreoffice.org/xref/core/dbaccess/source/filter/xml/
> xmlExport.cxx?r=b47bca7f#487
> 
> It seems we can't export any settings, see:
> schema/odf1.2/OpenDocument-v1.2-os-schema.rng from line 9158
> schema/odf1.3/OpenDocument-schema-v1.3.rng from line from line 3036

I'm not sure I follow you here; but please note that that list corresponds with what is written to ODB's content.xml as db:driver-settings element's attributes, as opposed to sub-elements of db:application-connection-settings, which is how other settings are written here.

All the settings might be written as far as I see.
Comment 45 Mike Kaganski 2019-03-09 17:19:49 UTC
(In reply to Mike Kaganski from comment #44)
> but please note that that list corresponds
> with what is written to ODB's content.xml as db:driver-settings element's
> attributes, as opposed to sub-elements of
> db:application-connection-settings, which is how other settings are written
> here.

A thinko: not "as db:driver-settings element's attributes", but "as db:application-connection-settings element's attributes"
Comment 46 Commit Notification 2019-03-10 05:26:10 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/f33eb08a2b74e5de033af9b5f5283b220ac58ded%5E%21

tdf#121092: synchronize defaults for ODBC settings

It will be available in 6.3.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 47 Mike Kaganski 2019-03-10 05:28:27 UTC
Settings other than

* Use SQL92 naming constraints
* End text lines with CR+LF
* Respect the result set type from the database driver

were not handled in the above patch, and still need investigation.

Also note tdf#123975.
Comment 48 Commit Notification 2019-03-10 12:00:07 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/f8eef913a33b24ce6d9c6e37fdcc2614ab823659%5E%21

tdf#121092: save GenerateASBeforeCorrelationName to ODB

It will be available in 6.3.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 49 Commit Notification 2019-03-11 06:01:26 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/87b44efa2a21279d22a3350b3a3d497284621d52%5E%21

tdf#121092: save AppendTableAliasName to ODB

It will be available in 6.3.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 50 Commit Notification 2019-03-12 09:22:52 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/fde00c42b169d80c9fe850d4876a0d0e5d6744a6%5E%21

tdf#121092: save db:suppress-version-columns to ODB

It will be available in 6.3.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 51 Commit Notification 2019-03-13 06:42:11 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/8402cd6974bf4dc6318bda39298ac1b56f1923f7%5E%21

tdf#121092: synchronize defaults for drivers settings

It will be available in 6.3.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.