feat(Data::AbstractSessionImpl): add autoCommit property and tests #4261 (#4262)

* fix(Data::AbstracSessionImpl): protect autocommit feature handlers #4261

* chore(CI): re-enable mysql

* MySQL SessionImpl: make sure autocommit mode is on when session is openend or reset.

* PostgreSQL SessionImpl: reuse autocommit flag of AbstractSessionImpl.

* Github workflow: re-activated linux-gcc-make-postgres

* Fixed indentation in ci.yml

* Fix for DataTest SQLExecutor: use connector

* Data::Session: when parser is not used and autocommit mode is off, assume any SQL statement begins a transaction.

* PostgreSQL: don't use SQL parser (it currently cannot handle placeholders).

* PostgreSQL: added test sessionTransactionNoAutoCommit

* PostgreSQL test suite: removed reference to generic SQLExecutor

* PostgreSQL: fixes for sessionTransactionNoAutoCommit.

* MySQL: added test sessionPoolAndUnicode (from #2801)

* Fixed #define in sql-parser

* Data generic testsuite: support numbered placeholders

* PostgreSQL test suite: added missing include directory to Makefile.

* Attempt to fix PostgreSQL Makefiles

* PostgreSQL testsuite: added include path to Makefile

* PostgreSQL testsuite: added PocoDataTest library to Makefile

* DataTest SQLExecutor::formatSQL: don't use string_view

* PostgreSQL test suite: delegated most tests to Poco::Data::Test

* Makefile: added dependencies on Data-Tests

* Weaken assumptions about async in generic transaction tests

* Makefile: added dependency for Prometheus samples

* Fix deadlock in DataTest SQLExecutor

* PostgreSQL tests SQLExecutor: cleanup

* feat(Data::AbstractSessionImpl): add autoCommit property and tests #4261

* Brought MySQL backend in line with _autoCommit flag of AbstractSessionImpl.

---------

Co-authored-by: Friedrich Wilckens <[email protected]>
Co-authored-by: Friedrich Wilckens <[email protected]>

Author: aleks-f

.github/workflows/ci.yml

@@ -420,55 +420,55 @@ jobs:
 #          cd cmake-build;
 #          ctest --output-on-failure -E "(DataMySQL)|(DataODBC)|(Redis)|(MongoDB)" -C Debug
 
-#  linux-gcc-make-mysql:
-#    runs-on: ubuntu-22.04
-#    services:
-#      mysql:
-#        image: mysql:8.1.0
-#        env:
-#          MYSQL_ALLOW_EMPTY_PASSWORD: yes
-#          MYSQL_USER: pocotest
-#          MYSQL_PASSWORD: pocotest
-#          MYSQL_DATABASE: pocotest
-#        ports:
-#          - 3306:3306
-#    steps:
-#      - uses: actions/checkout@v3
-#      - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev libmysqlclient-dev  mysql-client
-#      - run: ./configure --everything --no-samples --omit=ActiveRecord,ApacheConnector,CppParser,Crypto,Data/PostgreSQL,Data/SQLite,Data/ODBC,Encodings,JSON,JWT,MongoDB,Net,NetSSL_OpenSSL,NetSSL_Win,PDF,PageCompiler,PocoDoc,ProGen,Prometheus,Redis,SevenZip,Util,XML,Zip && make all -s -j4 && sudo make install
-#      - uses: ./.github/actions/retry-action
-#        with:
-#          timeout_minutes: 90
-#          max_attempts: 3
-#          retry_on: any
-#          command: >-
-#            sudo -s
-#            EXCLUDE_TESTS="ActiveRecord ApacheConnector CppParser CppUnit Crypto Data Data/PostgreSQL Data/ODBC Data/SQLite Encodings Foundation JSON JWT MongoDB Net NetSSL_OpenSSL NetSSL_Win PDF PageCompiler PocoDoc ProGen Prometheus Redis SevenZip Util XML Zip"
-#            ./ci/runtests.sh
+  linux-gcc-make-mysql:
+    runs-on: ubuntu-22.04
+    services:
+      mysql:
+        image: mysql:8.1.0
+        env:
+          MYSQL_ALLOW_EMPTY_PASSWORD: yes
+          MYSQL_USER: pocotest
+          MYSQL_PASSWORD: pocotest
+          MYSQL_DATABASE: pocotest
+        ports:
+          - 3306:3306
+    steps:
+      - uses: actions/checkout@v3
+      - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev libmysqlclient-dev  mysql-client
+      - run: ./configure --everything --no-samples --omit=ActiveRecord,ApacheConnector,CppParser,Crypto,Data/PostgreSQL,Data/SQLite,Data/ODBC,Encodings,JSON,JWT,MongoDB,Net,NetSSL_OpenSSL,NetSSL_Win,PDF,PageCompiler,PocoDoc,ProGen,Prometheus,Redis,SevenZip,Util,XML,Zip && make all -s -j4 && sudo make install
+      - uses: ./.github/actions/retry-action
+        with:
+          timeout_minutes: 90
+          max_attempts: 3
+          retry_on: any
+          command: >-
+            sudo -s
+            EXCLUDE_TESTS="ActiveRecord ApacheConnector CppParser CppUnit Crypto Data Data/PostgreSQL Data/ODBC Data/SQLite Encodings Foundation JSON JWT MongoDB Net NetSSL_OpenSSL NetSSL_Win PDF PageCompiler PocoDoc ProGen Prometheus Redis SevenZip Util XML Zip"
+            ./ci/runtests.sh
 
-# TODO tests sometimes failling on testTransaction and testReconnect
-#   linux-gcc-make-postgres:
-#     runs-on: ubuntu-22.04
-#     services:
-#       postgres:
-#         image: postgres:16.0
-#         env:
-#           POSTGRES_PASSWORD: postgres
-#         ports:
-#           - 5432:5432
-#     steps:
-#       - uses: actions/checkout@v3
-#       - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev libmysqlclient-dev odbc-postgresql
-#       - run: ./configure --everything --no-samples --omit=ActiveRecord,ApacheConnector,CppParser,Crypto,Data/MySQL,Data/ODBC,Data/SQLite,Encodings,JSON,JWT,MongoDB,Net,NetSSL_OpenSSL,NetSSL_Win,PDF,PageCompiler,PocoDoc,ProGen,Prometheus,Redis,SevenZip,Util,XML,Zip && make all -s -j4 && sudo make install
-#       - uses: ./.github/actions/retry-action
-#         with:
-#           timeout_minutes: 90
-#           max_attempts: 3
-#           retry_on: any
-#           command: >-
-#             sudo -s
-#             EXCLUDE_TESTS="ActiveRecord ApacheConnector CppParser CppUnit Crypto Data Data/ODBC Data/MySQL Data/SQLite Encodings Foundation JSON JWT MongoDB Net NetSSL_OpenSSL NetSSL_Win PDF PageCompiler PocoDoc ProGen Prometheus Redis SevenZip Util XML Zip"
-#             ./ci/runtests.sh
+# TODO tests sometimes failing on testTransaction and testReconnect
+  linux-gcc-make-postgres:
+    runs-on: ubuntu-22.04
+    services:
+      postgres:
+        image: postgres:16.0
+        env:
+          POSTGRES_PASSWORD: postgres
+        ports:
+          - 5432:5432
+    steps:
+      - uses: actions/checkout@v3
+      - run: sudo apt -y update && sudo apt -y install libssl-dev unixodbc-dev libmysqlclient-dev odbc-postgresql
+      - run: ./configure --everything --no-samples --omit=ActiveRecord,ApacheConnector,CppParser,Crypto,Data/MySQL,Data/ODBC,Data/SQLite,Encodings,JSON,JWT,MongoDB,Net,NetSSL_OpenSSL,NetSSL_Win,PDF,PageCompiler,PocoDoc,ProGen,Prometheus,Redis,SevenZip,Util,XML,Zip && make all -s -j4 && sudo make install
+      - uses: ./.github/actions/retry-action
+        with:
+          timeout_minutes: 90
+          max_attempts: 3
+          retry_on: any
+          command: >-
+            sudo -s
+            EXCLUDE_TESTS="ActiveRecord ApacheConnector CppParser CppUnit Crypto Data Data/ODBC Data/MySQL Data/SQLite Encodings Foundation JSON JWT MongoDB Net NetSSL_OpenSSL NetSSL_Win PDF PageCompiler PocoDoc ProGen Prometheus Redis SevenZip Util XML Zip"
+            ./ci/runtests.sh
 
   linux-gcc-make-redis:
     runs-on: ubuntu-22.04

Data/MySQL/include/Poco/Data/MySQL/SessionHandle.h

@@ -67,6 +67,9 @@ class SessionHandle
 	void rollback();
 		/// Rollback transaction
 
+	void autoCommit(bool val);
+		/// Set autocommit mode
+
 	void reset();
 		/// Reset connection with dababase and clears session state, but without disconnecting
 

Data/MySQL/src/SessionHandle.cpp

@@ -181,6 +181,13 @@ void SessionHandle::rollback()
 }
 
 
+void SessionHandle::autoCommit(bool val)
+{
+	if (mysql_autocommit(_pHandle, val) != 0)
+		throw TransactionException("Setting autocommit mode failed.", _pHandle);
+}
+
+
 void SessionHandle::reset()
 {
 #if ((defined (MYSQL_VERSION_ID)) && (MYSQL_VERSION_ID >= 50700)) || ((defined (MARIADB_PACKAGE_VERSION_ID)) && (MARIADB_PACKAGE_VERSION_ID >= 30000))

Data/MySQL/src/SessionImpl.cpp

@@ -173,6 +173,9 @@ void SessionImpl::open(const std::string& connect)
 		&SessionImpl::autoCommit,
 		&SessionImpl::isAutoCommit);
 
+	// autocommit is initially on when a session is opened
+	AbstractSessionImpl::setAutoCommit("", true);
+
 	_connected = true;
 }
 
@@ -215,18 +218,18 @@ void SessionImpl::rollback()
 }
 
 
-void SessionImpl::autoCommit(const std::string&, bool val)
+void SessionImpl::autoCommit(const std::string& s, bool val)
 {
-	StatementExecutor ex(_handle);
-	ex.prepare(Poco::format("SET autocommit=%d", val ? 1 : 0));
-	ex.execute();
+	if (val != getAutoCommit(s)) {
+		_handle.autoCommit(val);
+		AbstractSessionImpl::setAutoCommit(s, val);
+	}
 }
 
 
-bool SessionImpl::isAutoCommit(const std::string&) const
+bool SessionImpl::isAutoCommit(const std::string& s) const
 {
-	int ac = 0;
-	return 1 == getSetting("autocommit", ac);
+	return AbstractSessionImpl::getAutoCommit(s);
 }
 
 
@@ -306,6 +309,7 @@ void SessionImpl::reset()
 	if (_connected && _reset)
 	{
 		_handle.reset();
+		AbstractSessionImpl::setAutoCommit("", true);
 	}
 }
 

Data/MySQL/testsuite/src/MySQLTest.cpp

@@ -727,6 +727,15 @@ void MySQLTest::testTupleWithNullable()
 }
 
 
+void MySQLTest::testSessionPoolAndUnicode()
+{
+	if (!_pSession) fail ("Test not available.");
+
+	recreateStringsTable();
+	_pExecutor->sessionPoolAndUnicode(_dbConnString);
+}
+
+
 void MySQLTest::dropTable(const std::string& tableName)
 {
 	try { *_pSession << format("DROP TABLE IF EXISTS %s", tableName), now; }
@@ -993,6 +1002,6 @@ CppUnit::Test* MySQLTest::suite()
 	CppUnit_addTest(pSuite, MySQLTest, testSessionTransaction);
 	CppUnit_addTest(pSuite, MySQLTest, testTransaction);
 	CppUnit_addTest(pSuite, MySQLTest, testReconnect);
-
+	CppUnit_addTest(pSuite, MySQLTest, testSessionPoolAndUnicode);
 	return pSuite;
 }

Data/MySQL/testsuite/src/MySQLTest.h

@@ -107,6 +107,7 @@ class MySQLTest: public CppUnit::TestCase
 	void testTransaction();
 
 	void testReconnect();
+	void testSessionPoolAndUnicode();
 
 	void setUp();
 	void tearDown();

Data/MySQL/testsuite/src/SQLExecutor.cpp

@@ -21,6 +21,7 @@
 #include "Poco/Data/Time.h"
 #include "Poco/Data/StatementImpl.h"
 #include "Poco/Data/RecordSet.h"
+#include "Poco/Data/SessionPool.h"
 #include "Poco/Data/Transaction.h"
 #include "Poco/Data/MySQL/Connector.h"
 #include "Poco/Data/MySQL/MySQLException.h"
@@ -1372,7 +1373,7 @@ void SQLExecutor::timestamp()
 	std::string firstName("Simpson");
 	std::string address("Springfield");
 	DateTime birthday(1980, 4, 1, 5, 45, 12, 354, 879);
-	
+
 	int count = 0;
 	try { *_pSession << "INSERT INTO Person VALUES (?,?,?,?)", use(lastName), use(firstName), use(address), use(birthday), now; }
 	catch(ConnectionException& ce){ std::cout << ce.displayText() << std::endl; fail (funct); }
@@ -1381,14 +1382,14 @@ void SQLExecutor::timestamp()
 	catch(ConnectionException& ce){ std::cout << ce.displayText() << std::endl; fail (funct); }
 	catch(StatementException& se){ std::cout << se.displayText() << std::endl; fail (funct); }
 	assertTrue (count == 1);
-	
+
 	DateTime bd;
 	assertTrue (bd != birthday);
 	try { *_pSession << "SELECT Birthday FROM Person", into(bd), now; }
 	catch(ConnectionException& ce){ std::cout << ce.displayText() << std::endl; fail (funct); }
 	catch(StatementException& se){ std::cout << se.displayText() << std::endl; fail (funct); }
 	assertTrue (bd == birthday);
-	
+
 	std::cout << std::endl << RecordSet(*_pSession, "SELECT * FROM Person");
 }
 
@@ -2066,3 +2067,31 @@ void SQLExecutor::reconnect()
 	assertTrue (count == age);
 	assertTrue (_pSession->isConnected());
 }
+
+
+void SQLExecutor::sessionPoolAndUnicode(const std::string& connString)
+{
+	std::string funct = "unicode()";
+	std::string text = "ěščřžťďůň";
+	std::string text2;
+
+	// Test uses session from SessionPool instead of _pSession to prove session
+	// obtained and returned into pool is valid.
+
+	// Min/Max 1 session - ensures that when get() is called, same session should be returned
+	Poco::SharedPtr<Poco::Data::SessionPool> sp = new Poco::Data::SessionPool(MySQL::Connector::KEY, connString, 1, 1);
+
+	{
+		Poco::Data::Session session = sp->get();
+		try { session << "INSERT INTO Strings VALUES (?)", use(text), now; }
+		catch(ConnectionException& ce){ std::cout << ce.displayText() << std::endl; fail (funct); }
+		catch(StatementException& se){ std::cout << se.displayText() << std::endl; fail (funct); }
+	} // parentheses to ensure session is returned into pool
+
+	Poco::Data::Session session2 = sp->get();
+	try { session2 << "SELECT str FROM Strings", into(text2), now; }
+	catch(ConnectionException& ce){ std::cout << ce.displayText() << std::endl; fail (funct); }
+	catch(StatementException& se){ std::cout << se.displayText() << std::endl; fail (funct); }
+
+	assertTrue (text == text2);
+}

Data/MySQL/testsuite/src/SQLExecutor.h

@@ -88,7 +88,7 @@ class SQLExecutor: public CppUnit::TestCase
 	void longText();
 #ifdef POCO_MYSQL_JSON
 	void json();
-#endif	
+#endif
 	void unsignedInts();
 	void floats();
 	void doubles();
@@ -103,6 +103,7 @@ class SQLExecutor: public CppUnit::TestCase
 	void transaction(const std::string& connect);
 
 	void reconnect();
+	void sessionPoolAndUnicode(const std::string& connString);
 
 private:
 	void setTransactionIsolation(Poco::Data::Session& session, Poco::UInt32 ti);

Data/PostgreSQL/Makefile

@@ -12,10 +12,13 @@ objects = Extractor BinaryExtractor Binder SessionImpl Connector \
 	PostgreSQLStatementImpl PostgreSQLException \
 	SessionHandle StatementExecutor PostgreSQLTypes Utility
 
+
+target_includes = $(POCO_BASE)/Data/testsuite/include
 ifndef POCO_DATA_NO_SQL_PARSER
-	target_includes = $(POCO_BASE)/Data/src
+	target_includes += $(POCO_BASE)/Data/src
 endif
 
+
 target         = PocoDataPostgreSQL
 target_version = $(LIBVERSION)
 target_libs    = PocoData PocoFoundation

Data/PostgreSQL/include/Poco/Data/PostgreSQL/SessionHandle.h

@@ -118,7 +118,7 @@ class SessionHandle
 	bool isAutoCommit() const;
 		/// is the connection in auto commit mode?
 
-	void setAutoCommit(bool aShouldAutoCommit = true);
+	void autoCommit(bool val);
 		/// is the connection in auto commit mode?
 
 	bool isAsynchronousCommit() const;
@@ -193,7 +193,6 @@ class SessionHandle
 	PGconn*                   _pConnection;
 	std::string               _connectionString;
 	bool                      _inTransaction;
-	bool                      _isAutoCommit;
 	bool                      _isAsynchronousCommit;
 	Poco::UInt32              _tranactionIsolationLevel;
 	std::vector <std::string> _preparedStatementsToBeDeallocated;
@@ -306,12 +305,6 @@ inline bool SessionHandle::isTransaction() const
 }
 
 
-inline bool SessionHandle::isAutoCommit() const
-{
-	return _isAutoCommit;
-}
-
-
 inline bool SessionHandle::isAsynchronousCommit() const
 {
 	return _isAsynchronousCommit;