Bug 130595 - FIREBIRD (internal): Parameter query with :parameter IS NULL doesn't work
Summary: FIREBIRD (internal): Parameter query with :parameter IS NULL doesn't work
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.1.5.2 release
Hardware: x86-64 (AMD64) All
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:7.4.0 target:7.3.0.2 target:7.2.6
Keywords:
Depends on:
Blocks: Database-Firebird-Default
  Show dependency treegraph
 
Reported: 2020-02-11 16:30 UTC by Robert Großkopf
Modified: 2021-12-27 19:40 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Open the query "Query_parameter_IS_NULL" and input a character for parameter (3.66 KB, application/vnd.oasis.opendocument.database)
2020-02-11 16:30 UTC, Robert Großkopf
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Großkopf 2020-02-11 16:30:17 UTC
Created attachment 157803 [details]
Open the query "Query_parameter_IS_NULL" and input a character for parameter

Open the attached database.
Open the query "Query_parameter_IS_NULL".
If you won't add any content to the parameter the query will work. It shows all rows of the table.
If you write down 'Rob' (or something else, must not exist in the database) there appears

SQL Status: HY004
Incorrect type for setString

... and no content will be shown.
This behavior only changes a little bit if the field isn't a VARCHAR-field. When I change to INTEGER the error is

Incorrect type for setValue

Bug appears in LO 6.4.0.3, also LO 6.1.5.2 on OpenSUSE 15.1 64bit rpm Linux.
Comment 1 Robert Großkopf 2020-02-11 16:32:37 UTC
Haven't written down the query:

SELECT * FROM "Table1" WHERE "Name" = :name OR :name IS NULL

... and this query works with the internal HSQLDB without any problems.
Comment 2 Julien Nabet 2020-02-11 21:55:50 UTC
Just to be sure, if we put "Rob", should we have only row with 'Rob' or all the lines?
Comment 3 Alex Thurgood 2020-02-12 09:49:26 UTC
Confirming with 
Version: 6.3.4.2
Build ID: 60da17e045e08f1793c57c00ba83cdfce946d0aa
Threads CPU : 4; OS : Mac OS X 10.15.3; UI Render : par défaut; VCL: osx; 
Locale : fr-FR (fr_FR.UTF-8); Langue IHM : fr-FR
Calc: threaded
Comment 4 Robert Großkopf 2020-02-12 15:03:02 UTC
(In reply to Julien Nabet from comment #2)
> Just to be sure, if we put "Rob", should we have only row with 'Rob' or all
> the lines?

Yes, there should be shown the row with "Name" = 'Rob' - but an error appears instead and nothing else ...
Comment 5 Robert Großkopf 2020-02-12 15:49:52 UTC
Have tested a little bit more:

SELECT * FROM "Table1" WHERE "Name" = :name OR :name IS NULL

must be changed to

SELECT * FROM "Table1" WHERE "Name" = :name OR CAST( :name AS VARCHAR ( 20 ) ) IS NULL

and it will work. You have to set a parameter to the fieldtype of the field "Name". So if it should work with an INTEGER-field you have to change to INTEGER here. But: The casting of the parameter isn't needed, because this part of the query only want to know, if the parameter is without value (NULL).
Comment 6 Julien Nabet 2020-02-13 22:07:48 UTC
Lionel: just for information, this patch makes it work:
diff --git a/connectivity/source/drivers/firebird/PreparedStatement.cxx b/connectivity/source/drivers/firebird/PreparedStatement.cxx
index 002723d9a697..7dd46226d028 100644
--- a/connectivity/source/drivers/firebird/PreparedStatement.cxx
+++ b/connectivity/source/drivers/firebird/PreparedStatement.cxx
@@ -240,6 +240,7 @@ void SAL_CALL OPreparedStatement::setString(sal_Int32 nParameterIndex,
         setShort(nParameterIndex, int32Value);
         break;
     }
+    case SQL_NULL: break;
     default:
         ::dbtools::throwSQLException(
             "Incorrect type for setString",
diff --git a/connectivity/source/drivers/firebird/Util.cxx b/connectivity/source/drivers/firebird/Util.cxx
index 020dffbf2076..6b67551975cc 100644
--- a/connectivity/source/drivers/firebird/Util.cxx
+++ b/connectivity/source/drivers/firebird/Util.cxx
@@ -325,7 +325,7 @@ void firebird::mallocSQLVAR(XSQLDA* pSqlda)
             pVar->sqldata = static_cast<char *>(malloc(sizeof(sal_Bool)));
             break;
         case SQL_NULL:
-            assert(false); // TODO: implement
+            //assert(false); // TODO: implement
             break;
         case SQL_QUAD:
             assert(false); // TODO: implement
@@ -370,7 +370,7 @@ void firebird::freeSQLVAR(XSQLDA* pSqlda)
             assert(false); // TODO: implement
             break;
         case SQL_NULL:
-            assert(false); // TODO: implement
+            //assert(false); // TODO: implement
             break;
         case SQL_QUAD:
             assert(false); // TODO: implement

but I don't think it's the right path.
Don't hesitate to comment.
Comment 7 Commit Notification 2021-12-27 13:20:21 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

Related tdf#130595: SQL_NULL means pVar->sqldata = nullptr

It will be available in 7.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 8 Commit Notification 2021-12-27 14:41:02 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/6d82b1f0c6deaaf55df762dbfda03eae21ff82c2

Related tdf#130595: SQL_NULL means pVar->sqldata = nullptr

It will be available in 7.3.0.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 9 Commit Notification 2021-12-27 14:50:57 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

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

Related tdf#130595: SQL_NULL means pVar->sqldata = nullptr

It will be available in 7.2.6.

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 10 Julien Nabet 2021-12-27 15:02:50 UTC
Just rethinking about all this.
I'm more accustomed to this kind of request:
SELECT * FROM "Table1" WHERE "Name" = :name OR "Name" IS NULL
putting "Rob", I got 2 lines (the one with "Rob" + the one with nothing)
putting nothing, I got only the line which contains nothing.
Seems ok to me.

If taking yours:
1) I noticed LO considers there 2 parameters (whereas there's only 1) as this part of bt shows:
#0  dbtools::setObjectWithInfo(com::sun::star::uno::Reference<com::sun::star::sdbc::XParameters> const&, int, connectivity::ORowSetValue const&, int, int)
    (_xParams=uno::Reference to (dbaccess::OPreparedStatement *) 0x5c91490, parameterIndex=2, _rValue=..., sqlType=12, scale=0) at connectivity/source/commontools/dbtools.cxx:1782
#1  0x00007ff09dafab50 in dbtools::setObjectWithInfo(com::sun::star::uno::Reference<com::sun::star::sdbc::XParameters> const&, int, com::sun::star::uno::Any const&, int, int)
    (_xParams=uno::Reference to (dbaccess::OPreparedStatement *) 0x5c91490, parameterIndex=2, x=uno::Any("string": "Rob"), sqlType=12, scale=0) at connectivity/source/commontools/dbtools.cxx:1773
#2  0x00007ff0932ea1b0 in dbaccess::ORowSet::impl_prepareAndExecute_throw() (this=0x5853ac0) at dbaccess/source/core/api/RowSet.cxx:1681
#3  0x00007ff0932e5a34 in dbaccess::ORowSet::execute_NoApprove_NoNewConn(osl::ResettableGuard<osl::Mutex>&) (this=0x5853ac0, _rClearForNotification=...) at dbaccess/source/core/api/RowSet.cxx:1806
#4  0x00007ff0932e4dbd in dbaccess::ORowSet::execute() (this=0x5853ac0) at dbaccess/source/core/api/RowSet.cxx:1565
#5  0x00007ff088730610 in frm::ODatabaseForm::executeRowSet(osl::ResettableGuard<osl::Mutex>&, bool, com::sun::star::uno::Reference<com::sun::star::task::XInteractionHandler> const&)
    (this=0x5fa8f00, _rClearForNotifies=..., bMoveToFirst=true, _rxCompletionHandler=empty uno::Reference) at forms/source/component/DatabaseForm.cxx:1141
#6  0x00007ff088738fcc in frm::ODatabaseForm::load_impl(bool, bool, com::sun::star::uno::Reference<com::sun::star::task::XInteractionHandler> const&)
    (this=0x5fa8f00, bCausedByParentForm=false, bMoveToFirst=true, _rxCompletionHandler=empty uno::Reference) at forms/source/component/DatabaseForm.cxx:2845
#7  0x00007ff08873a1c5 in frm::ODatabaseForm::load() (this=0x5fa8f00) at forms/source/component/DatabaseForm.cxx:2608
#8  0x00007ff08d1c1a4b in dbaui::SbaXDataBrowserController::reloadForm(com::sun::star::uno::Reference<com::sun::star::form::XLoadable> const&)
    (this=0x5e3b9f0, _rxLoadable=uno::Reference to (frm::ODatabaseForm *) 0x5fa9198) at dbaccess/source/ui/browser/brwctrlr.cxx:604
#9  0x00007ff08d31973c in dbaui::SbaTableQueryBrowser::implLoadAnything(rtl::OUString const&, rtl::OUString const&, int, bool, utl::SharedUNOComponent<com::sun::star::sdbc::XConnection, utl::DisposableComponent> const&) (this=0x5e3b9f0, _rDataSourceName="file:///tmp/Firebird_Parameter.odb", _rCommand="Query_parameter_IS_NULL", nCommandType=1, _bEscapeProcessing=true, _rxConnection=...)
    at dbaccess/source/ui/browser/unodatbr.cxx:2380
#10 0x00007ff08d315c0a in dbaui::SbaTableQueryBrowser::implSelect(weld::TreeIter const*) (this=0x5e3b9f0, pEntry=0x5cb8780) at dbaccess/source/ui/browser/unodatbr.cxx:2677
#11 0x00007ff08d318e18 in dbaui::SbaTableQueryBrowser::implSelect(rtl::OUString const&, rtl::OUString const&, int, bool, utl::SharedUNOComponent<com::sun::star::sdbc::XConnection, utl::DisposableComponent> const&, bool) (this=0x5e3b9f0, _rDataSourceName="file:///tmp/Firebird_Parameter.odb", _rCommand="Query_parameter_IS_NULL", nCommandType=1, _bEscapeProcessing=true, _rxConnection=..., _bSelectDirect=true)
    at dbaccess/source/ui/browser/unodatbr.cxx:2444
#12 0x00007ff08d31e4b7 in dbaui::SbaTableQueryBrowser::impl_initialize() (this=0x5e3b9f0) at dbaccess/source/ui/browser/unodatbr.cxx:3250
#13 0x00007ff08d285c42 in dbaui::OGenericUnoController::initialize(com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) (this=0x5e3b9f0, aArguments=uno::Sequence of length 14 = {...})
    at dbaccess/source/ui/browser/genericcontroller.cxx:259

2) is it expected that for the second parameter LO goes into OPreparedStatement::setString but retrieves dType (which is equal to pVar->sqltype & ~1) to SQL_NULL ?
Comment 11 Lionel Elie Mamane 2021-12-27 15:39:43 UTC
(In reply to Julien Nabet from comment #10)
> Just rethinking about all this.
> I'm more accustomed to this kind of request:
> SELECT * FROM "Table1" WHERE "Name" = :name OR "Name" IS NULL
> putting "Rob", I got 2 lines (the one with "Rob" + the one with nothing)
> putting nothing, I got only the line which contains nothing.
> Seems ok to me.

> 1) I noticed LO considers there 2 parameters (whereas there's only 1) as
> this part of bt shows:

LibreOffice "should" map the _one_ named parameter called :name to _two_ unnamed ? parameters and fill in the same thing for both unnamed parameters. See the documentation of FirebirdSQL that you gave a link to in your commit message (or was it a comment in the code or in gerrit)

https://www.firebirdsql.org/file/documentation/html/en/refdocs/fblangref25/firebird-25-language-reference.html#fblangref25-datatypes-special-sqlnull



User has supplied a value

    First parameter (value compare): set *sqldata to the supplied value and *sqlind to 0 (for NOT NULL)

    Second parameter (NULL test): set sqldata to null (null pointer, not SQL NULL) and *sqlind to 0 (for NOT NULL)
User has left the field blank

    Both parameters: set sqldata to null (null pointer, not SQL NULL) and *sqlind to -1 (indicating NULL)
Comment 12 Julien Nabet 2021-12-27 16:06:35 UTC
(In reply to Lionel Elie Mamane from comment #11)
> ...
> LibreOffice "should" map the _one_ named parameter called :name to _two_
> unnamed ? parameters and fill in the same thing for both unnamed parameters.
> See the documentation of FirebirdSQL that you gave a link to in your commit
> message (or was it a comment in the code or in gerrit)
> 
> ...

Indeed... I must have read this one more carefully. The named parameters are converted into "?" so become unnamed parameters, therefore 2 parameters in the SQL here.

As you may have seen, I submitted a patch here:
https://bugs.documentfoundation.org/show_bug.cgi?id=130595
Comment 13 Commit Notification 2021-12-27 17:22:42 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/8cf970d4b3dca4f2f93fcebb8910aaeeb7ccf5fe

tdf#130595: Parameter query with :parameter IS NULL doesn't work

It will be available in 7.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 14 Julien Nabet 2021-12-27 17:32:56 UTC
commits on Jenkins for:
7.3: https://gerrit.libreoffice.org/c/core/+/127571
7.2: https://gerrit.libreoffice.org/c/core/+/127572
Comment 15 Commit Notification 2021-12-27 18:27:04 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

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

tdf#130595: Parameter query with :parameter IS NULL doesn't work

It will be available in 7.3.0.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 16 Commit Notification 2021-12-27 19:40:35 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/30fb61f80bba3b119d2fbfb450e3cf1ae7e65d59

tdf#130595: Parameter query with :parameter IS NULL doesn't work

It will be available in 7.2.6.

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.