Bug Hunting Session
Bug 118809 - Firebird: XDatabaseMetaData implementation returns NULL strings for DatabaseProductName and DatabaseProductVersion
Summary: Firebird: XDatabaseMetaData implementation returns NULL strings for DatabaseP...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.2.0.0.alpha0+
Hardware: All All
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:6.4.0 target:6.3.2
Keywords:
Depends on:
Blocks: Database-Firebird-Default
  Show dependency treegraph
 
Reported: 2018-07-17 17:48 UTC by Drew Jensen
Modified: 2019-09-05 09:08 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
newFBDoc odb (3.52 KB, application/vnd.oasis.opendocument.database)
2018-07-17 17:49 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-07-17 17:48:21 UTC
Description:
The XDatabaseMetaData imlpementation should return the properties defined in the SDK.



Steps to Reproduce:
1. Ensure LibreOffice will run basic macro in ODB
2. Load attached Firebird ODB file
3. Click on the Tables icon to activate the default connection
4. Run the procedure Main in the newFBdoc.odb Standard basic library
5. close the first message box

Actual Results:
Both basic instructions
ThisDatabaseDocument.CurrentController.ActiveConnection.MetaData.getDatabaseProductName
and
...MetaData.getDatabaseProductVersion
return null strings

Expected Results:
A product name of Firebird
A product version of, I guess, 3.0 perhaps there is more.


Reproducible: Always


User Profile Reset: No



Additional Info:
Comment 1 Drew Jensen 2018-07-17 17:49:10 UTC
Created attachment 143602 [details]
newFBDoc odb
Comment 2 Julien Nabet 2018-07-23 16:25:43 UTC
Drew: we currently use Firebird 3.0.0
Now version retrieved must work with embedded Firebird and not embedded Firebird.
For last one, obviously, we can't use a fixed string to display the version.
For first case, should we display the built Firebird version in LO or the Firebird version corresponding to the moment we created the file?

Code pointers:
- https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/firebird/DatabaseMetaData.cxx#599
- https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/firebird/DatabaseMetaData.cxx#604

For getDatabaseProductName(), could we indicate "Firebird" or should we distinguish if embedded version or not?
Comment 3 Drew Jensen 2018-07-23 20:09:09 UTC
(In reply to Julien Nabet from comment #2)
> Drew: we currently use Firebird 3.0.0
> Now version retrieved must work with embedded Firebird and not embedded
> Firebird.

My .02 worth:

(Hopefully if I have any details wrong someone will correct me.)

Both the Base embedded Firebird and Base file Firebird use the Firebird embedded engine, which in the Firebird server configuration is referred to as an 'engine12' connection when configuring access to managed files. Server managed .fdb files can be configured to accept 'engine12' connections or reject them.

It could make sense then to return a string of 'Firebird (engine12)' for the 
DatabaseProductName property as it would be the most precise and would be accurate for both scenarios of an embedded database file and an external database file using the firebird sdbc driver. 

If a user is connecting to a Firebird server with a jdbc or odbc driver the server would see that connection as 'remote' or 'loopback', using tcp/ip or or wnet or whathaveyou, for a file on the local machine or remote machine. 

Having 'engine12' in the sdbc designation could help if someone tries to create a Base odc and pointed to a managed .fdb file only to be refused connection attempts. 

> For last one, obviously, we can't use a fixed string to display the version.
> For first case, should we display the built Firebird version in LO or the
> Firebird version corresponding to the moment we created the file?

The version number could be retrieved at runtime by executing

SELECT rdb$get_context('SYSTEM', 'ENGINE_VERSION') 
             as version from rdb$database;

currently (6.0, 6.1, 6.2) Base sdbc connections return 3.0.0. If the binaries for the Firebird engine going forward are updated that should reflect any change.
Comment 4 Drew Jensen 2018-07-23 23:44:08 UTC
Could I tag on a few additional items for the MetaData:

NumericFunctions
return OUString("ABS,ACOS,ASIN,ATAN,ATAN2,BIN_AND,BIN_NOT,BIN_OR,BIN_SHL,BIN_SHR,BIN_XOR,CEIL,CEILING,COS,COSH,COT,EXP,FLOOR,LN,LOG,LOG10,MOD,PI,POWER,RAND,ROUND,SIGN,SIN,SINH,SQRT,TAN,TANH,TRUNC")

StringFunctions
return OUString("ASCII_CHAR,ASCII_VAL,BIT_LENGTH,CHAR_LENGTH,CHAR_TO_UUID,CHARACTER_LENGTH,GEN_UUID,HASH,LEFT,LOWER,LPAD,OCTET_LENGTH,OVERLAY,POSITION,REPLACE,REVERSE,RIGHT,RPAD,SUBSTRING,TRIM,UPPER,UUID_TO_CHAR")

TimeDateFunctions
retun OUString("CURRENT_DATE,CURRENT_TIME,CURRENT_TIMESTAMP,DATEADD, DATEDIFF,EXTRACT,'NOW','TODAY','TOMORROW','YESTERDAY'")
Comment 5 Alex Thurgood 2018-08-06 10:17:54 UTC
Confirming with:

Version: 6.1.1.0.0+
Build ID: 7a94069af971b9326e017d93b78118201291c48d
CPU threads: 4; OS: Mac OS X 10.13.6; UI render: default; 
TinderBox: MacOSX-x86_64@49-TDF, Branch:libreoffice-6-1, Time: 2018-08-04_20:34:42
Locale: fr-FR (fr_FR.UTF-8); Calc: group threaded
Comment 6 Alex Thurgood 2018-08-06 10:19:17 UTC
Also confirmed with:

Version: 6.2.0.0.alpha0+
Build ID: 36e1f6ebf0c74b4b90bbf1aab8d9ab69b8746f3a
CPU threads: 4; OS: Mac OS X 10.13.6; UI render: default; 
Locale: fr-FR (fr_FR.UTF-8); Calc: group threaded
Comment 7 QA Administrators 2019-08-19 06:56:02 UTC Comment hidden (obsolete)
Comment 8 Peter Nowee 2019-08-27 03:31:10 UTC
Still confirmed with:

Version: 6.3.0.4
Build ID: 1:6.3.0-2~bpo10+1
CPU threads: 2; OS: Linux 4.19; UI render: default; VCL: gtk3; 
Locale: en-US (en_US.UTF-8); UI-Language: en-US
Calc: threaded

To be exact, they return empty strings, not Null.

Same with getDriverName and getDriverVersion, by the way.
Comment 9 Peter Nowee 2019-08-27 03:32:38 UTC
Not IsNull in Basic, I mean.
Comment 10 Alex Thurgood 2019-08-27 10:46:08 UTC
Confirming indeed that there has been no change in this bug status with

Version: 6.3.0.4
Build ID: 057fc023c990d676a43019934386b85b21a9ee99
Threads CPU : 8; OS : Mac OS X 10.14.6; UI Render : par défaut; VCL: osx; 
Locale : fr-FR (fr_FR.UTF-8); Langue IHM : fr-FR
Calc: threaded

Bug is still present (but unsurprising, seeing as no commits have changed what DatabaseMetaData.cxx does here).
Comment 11 Julien Nabet 2019-08-30 09:18:42 UTC
After my day time job, I'll submit a main patch with 2 suggestions:
"Firebird (engine12)" for getDatabaseProductName

result of "SELECT rdb$get_context('SYSTEM', 'ENGINE_VERSION') as version from rdb$database" for getDatabaseProductVersion

In a second patch, I'll put 
- NumericFunctions
- StringFunctions
- TimeDateFunctions
as suggested

For driver version and name, I thought about lib used by LO.
In master we still use Firebird 3.0.0 (see https://opengrok.libreoffice.org/xref/core/download.lst?r=df2b4a9d#48)

So I'd put:
for getDriverMajorVersion
// Retrieve version from download.lst
return OUString("3.0.0");


for getDriverName:
// Retrieve name from download.lst
Firebird-3.0.0.32483-0.tar.bz2


Also I noticed this:
sal_Int32 SAL_CALL ODatabaseMetaData::getDriverMajorVersion(  )
{
    return 1;
}

sal_Int32 SAL_CALL ODatabaseMetaData::getDriverMinorVersion(  )
{
    return 0;
}
See https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/firebird/DatabaseMetaData.cxx?r=1729f554#614

Shouldn't we put:
sal_Int32 SAL_CALL ODatabaseMetaData::getDriverMajorVersion(  )
{
    return 3;
}

sal_Int32 SAL_CALL ODatabaseMetaData::getDriverMinorVersion(  )
{
    return 0;
}

Any thoughts?
Comment 12 Lionel Elie Mamane 2019-08-30 10:41:58 UTC
(In reply to Julien Nabet from comment #11)

> Shouldn't we put:
> sal_Int32 SAL_CALL ODatabaseMetaData::getDriverMajorVersion(  )
> {
>     return 3;
> }
> 
> sal_Int32 SAL_CALL ODatabaseMetaData::getDriverMinorVersion(  )
> {
>     return 0;
> }

In my understanding, this is the version of the _driver_, that is of the sdbc-firebird code, the code in connectivity/source/firebirc. We should increment it when we change the driver... Or we could just return the LibreOffice version, considering that the driver is just part of LibreOffice and has the same versioning.

The other drivers in connectivity/source don't really do this correctly either.
Comment 13 Julien Nabet 2019-08-30 12:30:26 UTC
(In reply to Lionel Elie Mamane from comment #12)
>...
> In my understanding, this is the version of the _driver_, that is of the
> sdbc-firebird code, the code in connectivity/source/firebirc. We should
> increment it when we change the driver... Or we could just return the
> LibreOffice version, considering that the driver is just part of LibreOffice
> and has the same versioning.
> 
> The other drivers in connectivity/source don't really do this correctly
> either.

I don't know about LO/Basic/Uno vars but LO version should be a global kind of global var.
About sdbc, it's just a wrapper of libs in most of cases (all?), I would have thought that lib used should correspond more to what we expect for driver version.
Since I'm not sure, I won't touch getDriverMajorVersion/getDriverMinorVersion for the moment.
Comment 14 Julien Nabet 2019-08-30 15:03:34 UTC
I submitted the main patch for master sources here:
https://gerrit.libreoffice.org/#/c/78302/

If Jenkins doesn't complain, I'll push it (unless someone requires some changes) and cherry-pick for 6.3 branch.
Comment 15 Commit Notification 2019-08-31 03:38:24 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

tdf#118809: Fill DatabaseProductName and DatabaseProductVersion for Firebird

It will be available in 6.4.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 16 Commit Notification 2019-08-31 06:36:41 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

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

tdf#118809: Fill DatabaseProductName and DatabaseProductVersion for Firebird

It will be available in 6.3.2.

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 17 Julien Nabet 2019-08-31 08:09:15 UTC
The second patch about https://bugs.documentfoundation.org/show_bug.cgi?id=118809#c4 is here:
https://gerrit.libreoffice.org/#/c/78337

waiting for review + Jenkins checks.
Comment 18 Julien Nabet 2019-08-31 08:13:32 UTC
Since the main part is fixed, let's put this one to FIXED.
Comment 19 Commit Notification 2019-08-31 09:02:59 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

Related tdf#118809: add some XDatabaseMetaData for Firebird

It will be available in 6.4.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 20 Commit Notification 2019-09-05 09:08:45 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

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

Related tdf#118809: add some XDatabaseMetaData for Firebird

It will be available in 6.3.2.

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.