Bug 126468 - Unable to deselect 'Visible' flags in Base query, if field is set for "sorting"
Summary: Unable to deselect 'Visible' flags in Base query, if field is set for "sorting"
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
6.1.3.2 release
Hardware: All Linux (All)
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:7.0.0 target:6.4.5
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-18 20:19 UTC by harvey
Modified: 2020-05-11 10:51 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Open the query for editing in the GUI. Try to remove visibility of field "test". (86.20 KB, application/vnd.oasis.opendocument.database)
2019-07-19 06:05 UTC, Robert Großkopf
Details
Open the HSQLDB-query for editing. The field for sorting could be set "unvisible". (3.84 KB, application/vnd.oasis.opendocument.database)
2019-07-19 06:13 UTC, Robert Großkopf
Details

Note You need to log in before you can comment on or make changes to this bug.
Description harvey 2019-07-18 20:19:38 UTC
Problem was discovered with LibreOffice (version 6.1.2.3) Base as client to a backend Mariadb10 via MySQL(JDBC) connector. 

Using the GUI query editor, an entire table as <tablename>.* was entered into
into the query fields. Two of the table fields were added to the query and selected 'ascending' for sorting for each of these 2
fields. It was however, not possible to deselect the 'Visible' flags
for these 2 sort fields. The behaviour means that the 2 fields will each now appear twice when running the query. A Base Form based on this query did not transfer the data from those fields back to the table, presumably because of the above ambiguity.   

On inspecting the SQL text of the query, a query of the
kind: SELECT * FROM <tablename> ORDER BY <Field1> ASC, <Field2> ASC was expected.

The actual text in the SQL editor was, however:
SELECT * <tablename>.*, <tablename>.<Field1>, <tablename>.<Field2> FROM
<schema>.<tablename> <tablename> ORDER BY <Field1> ASC, <Field2> ASC

After deleting the text ', <tablename>.<Field1>, <tablename>.<Field2>'
from the SQL query, the query ran successfully, i.e. the extra fields were not displayed in the query result. But on saving the query from the GUI again, the deleted text was added again.

The source of the problem seems to be that the 'visible' flags in the extra  fields (i.e. duplicates of fields from <tablename>.*) could not be deselected.
Comment 1 Robert Großkopf 2019-07-19 06:05:49 UTC
Created attachment 152867 [details]
Open the query for editing in the GUI. Try to remove visibility of field "test".

Con reproduce the behaviour with MariaDB. It is like the behaviour in Firebird when adding a field in the query-GUI for sorting a query. You couldn't set this field to "unvisible". There must be an alias: bug 126178.

All tested with OpenSUSE 15 64bit rpm Linux and LO 6.2.5.2
Comment 2 Robert Großkopf 2019-07-19 06:13:57 UTC
Created attachment 152868 [details]
Open the HSQLDB-query for editing. The field for sorting could be set "unvisible".

This buggy behaviour doesn't with the internal HSQLDB. You could set the field for the column, which is needed to sort the content, to unvisible in the GUI and you could save such a query in the GUI.
Comment 3 Julien Nabet 2020-05-10 11:12:50 UTC
On pc Debian x86-64 with mariadbJDBC, I don't reproduce this.
Did I miss something?
Comment 4 Robert Großkopf 2020-05-10 13:02:15 UTC
(In reply to Julien Nabet from comment #3)
> On pc Debian x86-64 with mariadbJDBC, I don't reproduce this.
> Did I miss something?

Could be you have set the visibility to "No" and then changed the sorting order without setting the cursor to next field.

I see the title doesn't reflect sorting is set for the field. I have changed this.
Comment 5 Julien Nabet 2020-05-10 14:57:22 UTC
Ok I had missed the <table>.* for first field.
I could reproduce this.
If I put no sort to be able to uncheck visibility then put back a sort, the visibility is automatically checked again and the field appears twice.
Comment 6 Julien Nabet 2020-05-10 18:30:54 UTC
Code pointer:
#0  dbaui::OSelectionBrowseBox::AddOrder(rtl::Reference<dbaui::OTableFieldDesc> const&, dbaui::EOrderDir, unsigned int) (this=0x3a994e0, rInfo=rtl::Reference to 0x3b116a0, eDir=dbaui::ORDER_ASC, _nCurrentPos=0)
    at dbaccess/source/ui/querydesign/SelectionBrowseBox.cxx:1818
#1  0x00007fffdcb48a1c in (anonymous namespace)::GetOrderCriteria(dbaui::OQueryDesignView*, dbaui::OSelectionBrowseBox*, connectivity::OSQLParseNode const*) (_pView=
    0x3a584d0, _pSelectionBrw=0x3a994e0, pParseRoot=0x3b0d9a0) at dbaccess/source/ui/querydesign/QueryDesignView.cxx:2254
#2  0x00007fffdcb42b15 in (anonymous namespace)::InitFromParseNodeImpl(dbaui::OQueryDesignView*, dbaui::OSelectionBrowseBox*) (_pView=0x3a584d0, _pSelectionBrw=0x3a994e0)
    at dbaccess/source/ui/querydesign/QueryDesignView.cxx:2012
#3  0x00007fffdcb416c3 in dbaui::OQueryDesignView::initByParseIterator(dbtools::SQLExceptionInfo*) (this=0x3a584d0, _pErrorInfo=0x7ffffffebbc8) at dbaccess/source/ui/querydesign/QueryDesignView.cxx:3045
#4  0x00007fffdcb77f79 in dbaui::OQueryViewSwitch::switchView(dbtools::SQLExceptionInfo*) (this=0x3a64720, _pErrorInfo=0x7ffffffebbc8) at dbaccess/source/ui/querydesign/QueryViewSwitch.cxx:210
#5  0x00007fffdcb1d264 in dbaui::OQueryContainerWindow::switchView(dbtools::SQLExceptionInfo*) (this=0x3a47ef0, _pErrorInfo=0x7ffffffebbc8) at dbaccess/source/ui/querydesign/querycontainerwindow.cxx:82
#6  0x00007fffdcb25e86 in dbaui::OQueryController::impl_setViewMode(dbtools::SQLExceptionInfo*) (this=0x3a843c0, _pErrorInfo=0x7ffffffebed8) at dbaccess/source/ui/querydesign/querycontroller.cxx:659
#7  0x00007fffdcb28ad1 in dbaui::OQueryController::impl_initialize() (this=0x3a843c0) at dbaccess/source/ui/querydesign/querycontroller.cxx:853
#8  0x00007fffdc830bab in dbaui::OGenericUnoController::initialize(com::sun::star::uno::Sequence<com::sun::star::uno::Any> const&) (this=0x3a843c0, aArguments=uno::Sequence of length 13 = {...})
    at dbaccess/source/ui/browser/genericcontroller.cxx:264
#9  0x00007fffdc7fa99d in (anonymous namespace)::DBContentLoader::load(com::sun::star::uno::Reference<com::sun::star::frame::XFrame> const&, rtl::OUString const&, com::sun::star::uno::Sequence<com::sun::star::beans::PropertyValue> const&, com::sun::star::uno::Reference<com::sun::star::frame::XLoadEventListener> const&) (this=0x3a6acf0, rFrame=
    uno::Reference to ((anonymous namespace)::XFrameImpl *) 0x36cd520, rURL=".component:DB/QueryDesign", rArgs=uno::Sequence of length 12 = {...}, rListener=uno::Reference to (framework::(anonymous namespace)::LoadEnvListener *) 0x3a69c38) at dbaccess/source/ui/browser/dbloader.cxx:260
Comment 7 Julien Nabet 2020-05-10 18:44:17 UTC
It works for hsqldb and not for Mariadb because for last one, "supportsOrderByUnrelated" method returns false.

Here are details about this method:
    420     /** Can an "ORDER BY" clause use columns not in the SELECT statement?
    421         @returns
    422             `TRUE` if so
    423         @throws SQLException
    424             if a database access error occurs.
    425      */
    426     boolean supportsOrderByUnrelated() raises (SQLException);

So if you take the code pointer from my previous comment:
   1816                 if ( !m_bOrderByUnRelated )
   1817                     pEntry->SetVisible();
it'll put the field visible because "m_bOrderByUnRelated" = false

(see https://opengrok.libreoffice.org/xref/core/offapi/com/sun/star/sdbc/XDatabaseMetaData.idl?r=157420e4#426)

However, I tested on MariaDB via http://localhost/adminer.php, the following request is ok and works:
"select test1 from Table1 order by test2"
so fields in order part don't need to be also present in select part.
Comment 8 Julien Nabet 2020-05-10 18:50:22 UTC
I gave a try with this:
https://gerrit.libreoffice.org/c/core/+/93937

but it's more a "trick" which works for MariaDB/Mysql direct connection, since the real fix would be to test if "select" part contains "*".
Comment 9 Robert Großkopf 2020-05-11 07:31:27 UTC
I don't know any database, which needs a field shown in the query, which is set to order the query.
Have tested MariaDB, Firebird, Sqlite and PostgreSQL. All these databases don't need a field shown in the query for sorting the rows by this field.

So I have tested:

No problem to hide the field, which is set for order: 
MariaDB with ODBC, 
Firebird with JDBC, 
Sqlite with ODBC, 
PostgreSQL with direct connection, 
PostgreSQL with JDBC

Impossible to hide the field: 
MariaDB with JDBC, 
MariaDB with direct connection, 
Firebird with ODBC, 
PostgreSQL with ODBC

For me it seems
boolean supportsOrderByUnrelated() raises (SQLException);
doesn't work right.
You could switch from GUI to direct SQL, delete the field from the shown fields and the queries will work well - in all connections, which doesn't allow to hide the field.
Comment 10 Julien Nabet 2020-05-11 07:42:47 UTC
Lionel: following Robert's last comment, should we change all "supportsOrderByUnrelated" methods so they return true or do you have in mind some exceptions or perhaps disagree here?
Comment 11 Lionel Elie Mamane 2020-05-11 07:55:53 UTC
(In reply to Julien Nabet from comment #10)
> Lionel: following Robert's last comment, should we change all
> "supportsOrderByUnrelated" methods so they return true or do you have in
> mind some exceptions or perhaps disagree here?

The standards (ISO SQL, ODBC, ...) definitely allow a database to require an ORDER BY column to be an output column. But if any sdbc driver has the wrong value for supportsOrderByUnrelated(), then definitely correct it. That is, for the databases mentioned by Robert, correct it if it is wrong. In the JDBC, ODBC, ADO, etc case, it is up to the JDBC / ODBC / ADO / ... driver to give the correct information on whether the underlying database engine supports that.
Comment 12 Julien Nabet 2020-05-11 08:06:59 UTC
For Firebird I'm ok for the change, see
https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/firebird/DatabaseMetaData.cxx?r=65374780#515
515  sal_Bool SAL_CALL ODatabaseMetaData::supportsOrderByUnrelated(  )
516  {
517      return false;
518  }

But for ODBC and JDBC,we got:
1) ODBC:
sal_Bool SAL_CALL ODatabaseMetaData::supportsOrderByUnrelated(  )
1041  {
1042      OUString aValue;
1043      OTools::GetInfo(m_pConnection,m_aConnectionHandle,SQL_ORDER_BY_COLUMNS_IN_SELECT,aValue,*this,m_pConnection->getTextEncoding());
1044      return aValue.toChar() == 'N';
1045  }
See https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/odbc/ODatabaseMetaData.cxx?r=1dd9200b#1040

2) JDBC:
1049  sal_Bool SAL_CALL java_sql_DatabaseMetaData::supportsOrderByUnrelated(  )
1050  {
1051      static jmethodID mID(nullptr);
1052      return impl_callBooleanMethod( "supportsOrderByUnrelated", mID );
1053  }
See https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/jdbc/DatabaseMetaData.cxx?r=1dd9200b#1049

So if JDBC or ODBC driver responds false, perhaps it should be considered as a driver bug not LO bug.

Any thoughts?
Comment 13 Lionel Elie Mamane 2020-05-11 08:09:14 UTC
(In reply to Julien Nabet from comment #12)
> So if JDBC or ODBC driver responds false, perhaps it should be considered as
> a driver bug not LO bug.

That's exactly what I meant in comment 11.
Comment 14 Julien Nabet 2020-05-11 08:16:09 UTC
Indeed.
So I'll change the patch to add Firebird part.
Comment 15 Julien Nabet 2020-05-11 08:20:43 UTC
(In reply to Julien Nabet from comment #14)
> Indeed.
> So I'll change the patch to add Firebird part.

Done.
Comment 16 Commit Notification 2020-05-11 09:17:16 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/572c9db0440d9aba4945fb1761ec7f83e717c2f0

tdf#126468: MySQL/MariaDB and Firebird don't require order field in select part

It will be available in 7.0.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 17 Julien Nabet 2020-05-11 09:38:45 UTC
Backport for 6.4 here:
https://gerrit.libreoffice.org/c/core/+/93801
(just wait for Jenkins check before pushing it)
Comment 18 Commit Notification 2020-05-11 10:51:14 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

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

tdf#126468: MySQL/MariaDB and Firebird don't require order field in select part

It will be available in 6.4.5.

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.