When connecting to a MySQL database (served by a Mariadb 10.1 server) via the LibreOffice "direct" connector method (i.e. a "Native" connector), the Execute SQL Statement dialog can get confused and get stuck issuing this incorrect Status: "Commands out of sync; you can't run this command now". This occurs only when both of the following are true: * the 'Show output of "select" statements' box is not checked, and * when a query returns results. Connecting via JDBC does not exhibit this bug. In other words, JDBC works where MySQL native is broken. I suspect that what needs to be fixed is to get any results and throw them away when 'Show output of "select" statements' is not checked. This would keep MySQL happy. === How to setup the bug ======================================= --- STEP 1 - CONNECTING ------------- File | New | Database. ----------- Database Wizard [x] Connect to an existing database [MySQL] Next>> ------------ [x] Connect directly Next>> ------------ Database name: a_database [*] Server/port Server: localhost Port: 3306 Next>> ------------ User name root [v] Password required Test connection Ok Next>> ----------- ..register the database in LibreOffice? No [x] Open the database for editing Finish ----------- Save as: bigbase mysql direct.odb --- STEP 2 - TESTING SQL... ---------- Select: Tools | SQL... ----------- In the "Execute SQL Statement" dialog box: SQL Command Command to execute: select * from `table1`; [x] Show output of "select" statements Execute Status: Command successfully executed. (This is ok) --- select * from `table1`; [x] Show output of "select" statements Execute Status: Command successfully executed. (This is ok) ------- select * from `table1`; [ ] Show output of "select" statements Execute Status: Command successfully executed. (This is ok) --- select * from `table1`; [x] Show output of "select" statements Execute Status: Commands out of sync; you can't run this command now (This FAILS) ---- This is the full accumulated output in the status box: 1: Command successfully executed. 2: Command successfully executed. 3: Command successfully executed. 4: Commands out of sync; you can't run this command now
While you're at it, this dialog could use a little better status to indicate that it is still working, but slowly. I have a 800 record database with a bunch of fields and a "select * from table;" took so long that I thought LibreOffice had crashed, as it appeared to just hang forever. Later I discovered that it just took a very long time. Then I added this, ""select * from table LIMIT 100;" And/OR you might suggest to add a LIMIT to clue people into the fact that it might be slow on big tables. Thanks!
Could confirm this buggy behavior. Every time you change from unchecked 'Show output ...' in Tools > SQL to checked 'Show output ...' "Commands out of sync; you can't run this command now" appears. Have tested with direct driver. LO 5.2.3.3 on OpenSUSE 42.1 64bit rpm Linux.
Lionel: When in SQL direct, Select is used + show results is checked, "executeQuery" is used and it seems there's no pb. In the other cases, "execute" is used. I tried to debug and think the pb is in mariadb-connector-c since "Commands out of sync" comes from this part but not easy to follow. The execute method should return with status = MYSQL_STATUS_READY but it's not the case. In mysql-connector-cpp/driver/mysql_statement.cpp we go line 87: 80 MySQL_Statement::do_query(const ::sql::SQLString &q) 81 { 82 CPP_ENTER("MySQL_Statement::do_query"); 83 CPP_INFO_FMT("this=%p", this); 84 checkClosed(); 85 86 if (proxy->query(q) && proxy->errNo()) { 87 CPP_ERR_FMT("Error during proxy->query : %d:(%s) %s", proxy->errNo(), proxy->sqlstate().c_str(), proxy->error().c_str()); 88 sql::mysql::util::throwSQLException(*proxy.get()); 89 } 90 91 warningsCount= proxy->warning_count(); 92 93 warningsHaveBeenLoaded= false; 94 } The pb in tdf#112423 must be similar
Lionel: Taking a look to MariaDB website, 3.0.2 version of the C connector is out (see https://downloads.mariadb.org/connector-c/3.0.2/), compared to 2.0.0. But there are also 2.1.0 + 2.2 and 2.3 branches if we don't want to jump to 3.X version About mysql connector c++, there's now 1.1.10 (see https://dev.mysql.com/doc/relnotes/connector-cpp/en/news-1-1.html), compared to 1.1.4 Of course, upgrading all this + updating the associated patches (THIS is really a PITA, I don't know a good and simple procedure for this) from LO is quite a big work and not for beginners I suppose. I don't know on what you're working right now but i'm afraid nobody would give it a try if you don't have time or are on other things.
Lionel: I forced to update status by calling cppStatement->getResultSet() patch: diff --git a/mysqlc/source/mysqlc_statement.cxx b/mysqlc/source/mysqlc_statement.cxx index 3a082004831a..6e425c2a28cd 100644 --- a/mysqlc/source/mysqlc_statement.cxx +++ b/mysqlc/source/mysqlc_statement.cxx @@ -141,6 +141,7 @@ sal_Bool SAL_CALL OCommonStatement::execute(const rtl::OUString& sql) bool success = false; try { success = cppStatement->execute(rtl::OUStringToOString(sSqlStatement, m_pConnection->getConnectionSettings().encoding).getStr()); + cppStatement->getResultSet(); } catch (const sql::SQLException &e) { mysqlc_sdbc_driver::translateAndThrow(e, *this, m_pConnection->getConnectionEncoding()); } I suppose it's a bit quick and dirty but at least, I don't reproduce the problem with it. I also tried tdf#112423 but no better. Do you think I should push the patch?
(In reply to Julien Nabet from comment #5) > Lionel: > I forced to update status by calling cppStatement->getResultSet() > patch: > --- a/mysqlc/source/mysqlc_statement.cxx > +++ b/mysqlc/source/mysqlc_statement.cxx > @@ -141,6 +141,7 @@ sal_Bool SAL_CALL OCommonStatement::execute(const > try { > success = > cppStatement->execute(rtl::OUStringToOString(sSqlStatement, > m_pConnection->getConnectionSettings().encoding).getStr()); > + cppStatement->getResultSet(); > } catch (const sql::SQLException &e) { This looks like it fetches the first result set and then throws it away. Always. Even when the caller wants the result. I'm rather sure this will cause other problems. E.g. when the query actually returns multiple result sets, the caller will not get the first one, only the second, third, etc. The solution is probably in code that _calls_ that. E.g. the code that implements the "Execute SQL Statement" dialog.
(In reply to Lionel Elie Mamane from comment #6) > (In reply to Julien Nabet from comment #5) > > Lionel: >... > This looks like it fetches the first result set and then throws it away. > Always. Even when the caller wants the result. I'm rather sure this will > cause other problems. E.g. when the query actually returns multiple result > sets, the caller will not get the first one, only the second, third, etc. > > The solution is probably in code that _calls_ that. E.g. the code that > implements the "Execute SQL Statement" dialog. Also, in case of a Select like here, I wonder if we shouldn't simply call executeQuery (as when checkbox is checked). The only diff would be not displaying the result. Anyway, I'll give a try to your suggestion.
The call to getResultSet() is really just to update the status, not for using a rs. Some code pointer: DirectSQLDialog::implExecuteStatement from https://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/dlg/directsql.cxx#179 197 if (_rStatement.toAsciiUpperCase().startsWith("SELECT") && m_pShowOutput->IsChecked()) 198 { 199 // execute it as a query 200 xResultSet = xStatement->executeQuery(_rStatement); ... 226 } else { 227 // execute it 228 xStatement->execute(_rStatement); 229 } Location where mysql c++ connector is called for execute https://opengrok.libreoffice.org/xref/core/mysqlc/source/mysqlc_statement.cxx#135 135 sal_Bool SAL_CALL OCommonStatement::execute(const rtl::OUString& sql) 136 { 137 MutexGuard aGuard(m_aMutex); 138 checkDisposed(rBHelper.bDisposed); 139 const rtl::OUString sSqlStatement = m_pConnection->transFormPreparedStatement( sql ); 140 141 bool success = false; 142 try { 143 success = cppStatement->execute(rtl::OUStringToOString(sSqlStatement, m_pConnection->getConnectionSettings().encoding).getStr()); 144 } catch (const sql::SQLException &e) { 145 mysqlc_sdbc_driver::translateAndThrow(e, *this, m_pConnection->getConnectionEncoding()); 146 } 147 return success; 148 } and in case of executeQuery 150 Reference< XResultSet > SAL_CALL OCommonStatement::executeQuery(const rtl::OUString& sql) 151 { 152 MutexGuard aGuard(m_aMutex); 153 checkDisposed(rBHelper.bDisposed); 154 const rtl::OUString sSqlStatement = m_pConnection->transFormPreparedStatement(sql); 155 156 Reference< XResultSet > xResultSet; 157 try { 158 std::unique_ptr< sql::ResultSet > rset(cppStatement->executeQuery(rtl::OUStringToOString(sSqlStatement, m_pConnection->getConnectionEncoding()).getStr())); 159 xResultSet = new OResultSet(this, rset.get(), m_pConnection->getConnectionEncoding()); 160 rset.release(); 161 } catch (const sql::SQLException &e) { 162 mysqlc_sdbc_driver::translateAndThrow(e, *this, m_pConnection->getConnectionEncoding()); 163 } 164 return xResultSet; 165 } In executeQuery, we retrieve a rs that we can release, it's not the case for execute. I'll check if we could call something else than getResultSet() (a method to close/reset the statement so the status is set to MYSQL_STATUS_READY) because it's the only thing lacking here ; there's no error in the whole mysql c++ connector and mariadb c connector (unless I missed it)
(In reply to Lionel Elie Mamane from comment #6) ... > This looks like it fetches the first result set and then throws it away. > Always. Even when the caller wants the result. I'm rather sure this will > cause other problems. E.g. when the query actually returns multiple result > sets, the caller will not get the first one, only the second, third, etc. > > The solution is probably in code that _calls_ that. E.g. the code that > implements the "Execute SQL Statement" dialog. I'm not sure to understand, execute and executeUpdate never use resultsets or did I miss something? If yes, I don't achieve to use getMoreResults/getResultSet/disposeResultSet to throw away rs correctly. Here's a small bt to see the link between execute methods: #0 0x00007f0022c89549 in connectivity::mysqlc::OCommonStatement::execute(rtl::OUString const&) (this=0x555557f4ccf0, sql="CALL `AllNames`;") at /home/julien/lo/libreoffice/mysqlc/source/mysqlc_statement.cxx:138 #1 0x00007f0029afefcb in OStatement::execute(rtl::OUString const&) (this=0x555557f4e640, _rSQL="CALL `AllNames`;") at /home/julien/lo/libreoffice/dbaccess/source/core/api/statement.cxx:489 #2 0x00007f0026e6b2b6 in dbaui::DirectSQLDialog::implExecuteStatement(rtl::OUString const&) (this=0x555557ec9890, _rStatement="CALL `AllNames`;") at /home/julien/lo/libreoffice/dbaccess/source/ui/dlg/directsql.cxx:228
(In reply to Julien Nabet from comment #9) > > I'm not sure to understand, execute and executeUpdate never use resultsets > or did I miss something? > If yes, I don't achieve to use getMoreResults/getResultSet/disposeResultSet > to throw away rs correctly. The procedure you are using for the test won't give a resultset in Tools > SQL or in Query-GUI. But it has an resultset you can fetch through macrocode or if you call the procedure in MySQL-console (or for example PHPMyAdmin). ExecuteUpdate shouldn't give a resultset. ExecuteQuery (with the same code for calling a procedure) should give a resultset.
(In reply to robert from comment #10) > (In reply to Julien Nabet from comment #9) > > > > I'm not sure to understand, execute and executeUpdate never use resultsets > > or did I miss something? > > If yes, I don't achieve to use getMoreResults/getResultSet/disposeResultSet > > to throw away rs correctly. > > The procedure you are using for the test won't give a resultset in Tools > > SQL or in Query-GUI. But it has an resultset you can fetch through macrocode > or if you call the procedure in MySQL-console (or for example PHPMyAdmin). Ok > > ExecuteUpdate shouldn't give a resultset. ExecuteQuery (with the same code > for calling a procedure) should give a resultset. Depending where we're looking at we can have until 3 functions : - executeQuery - execute - executeUpdate for Tools/SQL, it's just executeQuery or execute. But the levels below have the 3 ones. Anyway, I'll try to find a way to obtain the right result.
Lionel: Since it seems the cleaning must be in dbaccess/source/ui/dlg/directsql.cxx so resultsets can be available in dbaccess/source/core/api/statement.cxx for macros or other things, I'm stuck. Indeed, I began with: diff --git a/mysqlc/source/mysqlc_statement.cxx b/mysqlc/source/mysqlc_statement.cxx index 3a082004831a..9ff3ce163fb6 100644 --- a/mysqlc/source/mysqlc_statement.cxx +++ b/mysqlc/source/mysqlc_statement.cxx @@ -240,7 +240,7 @@ sal_Bool SAL_CALL OCommonStatement::getMoreResults() // if your driver supports more than only one resultset // and has one more at this moment return true - return false; + return cppStatement->getMoreResults(); } but then, I don't know how to call getMoreResults() from directsql.cxx. I tried to declare OStatement::getMoreResults (because OStatementBase::getMoreResults already existts) in statement.hxx + statement.cxx but I've got an error when building. I think it's perhaps linked to XStatement.idl but don't want to touch this since it's a public interface and I don't know the consequences. I also tried to use XMultipleResults but since execute method doesn't return anything, I can't use XMultipleResults
(In reply to Julien Nabet from comment #8) > The call to getResultSet() is really just to update the status, not for > using a rs. I thought from memory that calling getResultSet twice in a row would implicitly retrieve the second result of the query. That is indeed wrong. To advance to the next result, one must call getMoreResults(). However, MySQL C++ connector implements (part of) the JDBC API in C++. So looking at the JDBC documentation https://docs.oracle.com/javase/7/docs/api/java/sql/Statement.html#getResultSet() I see: This method should be called only once per result. So unless MySQL C++ connector declares it made that safe in its documentation, we should avoid calling it and throwing away the result; we will not necessarily get it back correctly when calling getResultSet() the next time. > In executeQuery, we retrieve a rs that we can release, it's not the case for > execute. Yes, because in executeQuery, there is necessarily a ResultSet to be retrieved. It is an error to call executeQuery on a query that does not return results (such as a basic UPDATE / DELETE query). The query can be a query that returns a result containing zero rows, but it must return a result. It is allowed to call execute() on any kind of query: returning a single result (of zero, one or more rows) or not returning any result, or returning several results. It is then the caller's job to call getResultSet() and getMoreResults() to retrieve the results. There is example code on how to use execute() correctly there: https://blogs.msdn.microsoft.com/jdbcteam/2008/08/01/use-execute-and-getmoreresults-methods-for-those-pesky-complex-sql-queries/
(In reply to Julien Nabet from comment #9) > I'm not sure to understand, execute and executeUpdate never use resultsets > or did I miss something? In complement to my previous comment: executeUpdate() can only be used for a query that does not return a ResultSet, but returns exactly one (no more, no less) update counts. E.g. a basic UPDATE or DELETE. executeQuery() can only be used for a query that returns exactly *one* ResultSet and (I think) *no* update count. execute() can be be used for a query that returns zero or more ResultSets and zero or more update counts (but a query will always return at least one or the other). Since we don't know that the user types in the direct sql window, I think we should use execute() and use the algorithm linked to in my previous comment to process the result correctly. If the "show result" checkbox is checked, then use the resultset, else just close it immediately and go to the next result.
(In reply to Julien Nabet from comment #12) > Indeed, I began with: > diff --git a/mysqlc/source/mysqlc_statement.cxx > b/mysqlc/source/mysqlc_statement.cxx > index 3a082004831a..9ff3ce163fb6 100644 > --- a/mysqlc/source/mysqlc_statement.cxx > +++ b/mysqlc/source/mysqlc_statement.cxx > @@ -240,7 +240,7 @@ sal_Bool SAL_CALL OCommonStatement::getMoreResults() > > // if your driver supports more than only one resultset > // and has one more at this moment return true > - return false; > + return cppStatement->getMoreResults(); > } This looks good. > but then, I don't know how to call getMoreResults() from directsql.cxx. See the algorithm linked to in comment 13. > I tried to declare OStatement::getMoreResults No need, it inherits it from OCommonStatement. > (because OStatementBase::getMoreResults already exists) in statement.hxx + > statement.cxx but I've got an error when building. While it is not necessary, it is definitely allowed. What error did you get? > I think it's perhaps linked to XStatement.idl but don't want to touch this > since it's a public interface and I don't know the consequences. Yes, don't change that. > I also tried to use XMultipleResults but since execute method doesn't return > anything, I can't use XMultipleResults The XMultipleResults interface is exactly what you have to use. But I see that mysqlc doesn't implement it, which is... "dommage" because it is pretty easy since MySQL Connector C++ implements it! Something like: sal_Int32 SAL_CALL OCommonStatement::getUpdateCount() { - return 0; + return cppStatement->getUpdateCount(); } in addition to your above patch should be all that is needed.
Julien Nabet committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=444730a67dbd2ad6cebe666b2cd23c67d5c668f2 tdf#103685: "Commands out of sync" when connecting to MySQL using direct It will be available in 6.0.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
For 5.4 branch, we just need a review for https://gerrit.libreoffice.org/#/c/42701/1 Let's put this one to FIXED then.
Julien Nabet committed a patch related to this issue. It has been pushed to "libreoffice-5-4": http://cgit.freedesktop.org/libreoffice/core/commit/?id=6d01cf0cabb1168159c3b013bc64b0a33a96f0c7&h=libreoffice-5-4 tdf#103685: "Commands out of sync" when connecting to MySQL using direct It will be available in 5.4.3. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.