Fix unexpected auto commits (#336)

* Add regression test nox target
* Add regression test for invalid auto commit behavior (Issue-335)
* Add regression tests to integration test target
* Fix invalid auto commit behavior
* Update change-log
* Move odbc helpers to exasol driver(s) package

Co-authored-by: Sebastian Bär <redcatbear@ursus-minor.de>

CHANGELOG.rst

@@ -12,6 +12,10 @@ Unreleased
 
     Note: It is also very likely that turbodbc support will be dropped in future versions.
 
+🐞 Fixed
+--------
+- Fixed invalid implicit autocommit behaviour, for details see `<Issue-https://github.com/exasol/sqlalchemy-exasol/issues/335>`_
+
 ✨ Added
 --------
 

scripts/odbc.py → exasol/driver/odbc.py

@@ -13,6 +13,10 @@
 
 from pyodbc import Connection
 
+PROJECT_ROOT = Path(__file__).parent / ".." / ".."
+
+ODBC_DRIVER = PROJECT_ROOT / "driver" / "libexaodbc-uo2214lv2.so"
+
 ODBCINST_INI_TEMPLATE = dedent(
     """
     [ODBC]

noxfile.py

@@ -24,23 +24,22 @@
 from links import urls as _urls
 from nox import Session
 from nox.sessions import SessionRunner
-from odbc import (
-    odbcconfig,
-    transaction,
-)
-from pyodbc import connect
 from version_check import (
     version_from_poetry,
     version_from_python_module,
     version_from_string,
 )
 
+from exasol.driver.odbc import (
+    ODBC_DRIVER,
+    odbcconfig,
+)
+
 # default actions to be run if nothing is explicitly specified with the -s option
 nox.options.sessions = ["fix"]
 
 
 class Settings:
-    ODBC_DRIVER = PROJECT_ROOT / "driver" / "libexaodbc-uo2214lv2.so"
     CONNECTORS = ("pyodbc", "turbodbc")
     ENVIRONMENT_NAME = "test"
     DB_PORT = 8888
@@ -186,7 +185,7 @@ def parser() -> ArgumentParser:
         )
         return p
 
-    with odbcconfig(Settings.ODBC_DRIVER) as (config, env):
+    with odbcconfig(ODBC_DRIVER) as (config, env):
         args = parser().parse_args(session.posargs)
         connector = args.connector
         session.run(
@@ -227,7 +226,7 @@ def parser() -> ArgumentParser:
         )
         return p
 
-    with odbcconfig(Settings.ODBC_DRIVER) as (config, env):
+    with odbcconfig(ODBC_DRIVER) as (config, env):
         args = parser().parse_args(session.posargs)
         connector = args.connector
         session.run(
@@ -243,6 +242,12 @@ def parser() -> ArgumentParser:
     session.run("pytest", f"{PROJECT_ROOT / 'test' / 'integration' / 'dbapi'}")
 
 
+@nox.session(name="regression-tests", python=False)
+def regression_tests(session: Session) -> None:
+    """Run regression tests"""
+    session.run("pytest", f"{PROJECT_ROOT / 'test' / 'integration' / 'regression'}")
+
+
 @nox.session(name="integration-tests", python=False)
 def integration_tests(session: Session) -> None:
     """Run integration tests with a specific configuration. For more details append '-- -h'"""
@@ -279,6 +284,9 @@ def parser() -> ArgumentParser:
         find_session_runner(session, f"exasol-tests"),
         posargs=["--connector", f"{args.connector}"],
     )
+    session.notify(
+        find_session_runner(session, f"regression-tests"),
+    )
     session.notify(find_session_runner(session, "db-stop"))
 
 
@@ -431,7 +439,7 @@ def report_skipped(session: Session) -> None:
     with TemporaryDirectory() as tmp_dir:
         for connector in Settings.CONNECTORS:
             report = Path(tmp_dir) / f"test-report{connector}.json"
-            with odbcconfig(Settings.ODBC_DRIVER) as (config, env):
+            with odbcconfig(ODBC_DRIVER) as (config, env):
                 session.run(
                     "pytest",
                     "--dropfirst",

sqlalchemy_exasol/base.py

@@ -47,6 +47,7 @@
 
 import logging
 import re
+from contextlib import closing
 from datetime import (
     date,
     datetime,
@@ -750,7 +751,7 @@ def _get_schema(sql_compiler, dialect):
         sql_stmnt += " AND column_schema = ?" if schema else ""
         args = (table, id_col, schema) if schema else (table, id_col)
 
-        with self.create_cursor() as cursor:
+        with closing(self.create_cursor()) as cursor:
             cursor.execute(sql_stmnt, args)
             result = cursor.fetchone()
             return int(result[0]) - 1

test/integration/regression/test_regression_bug335.py

@@ -0,0 +1,81 @@
+import pytest
+from sqlalchemy import (
+    Column,
+    Integer,
+    MetaData,
+    String,
+    Table,
+    create_engine,
+    insert,
+)
+
+from exasol.driver.odbc import (
+    ODBC_DRIVER,
+    odbcconfig,
+)
+
+
+@pytest.fixture
+def pyodbc_connection_string(exasol_config):
+    config = exasol_config
+    return (
+        f"exa+pyodbc://{config.username}:{config.password}@{config.host}:{config.port}/"
+        f"?DEFAULTPARAMSIZE=200&INTTYPESINRESULTSIFPOSSIBLE=y"
+        "&FINGERPRINT=NOCERTCHECK&CONNECTIONLCALL=en_US.UTF-8&driver=EXAODBC"
+    )
+
+
+@pytest.fixture()
+def test_schema(control_connection):
+    connection = control_connection
+    schema = "REGRESSION_335"
+    connection.execute(f"CREATE SCHEMA {schema}")
+    connection.commit()
+    yield schema
+    connection.execute(f"DROP SCHEMA IF EXISTS {schema} CASCADE")
+    connection.commit()
+
+
+@pytest.fixture()
+def users_table(control_connection, test_schema):
+    connection = control_connection
+    table_name = "users"
+    connection.execute(
+        f"create table {test_schema}.{table_name} (id DECIMAL(18) identity primary key, name VARCHAR(2000) UTF8)"
+    )
+    connection.commit()
+    yield test_schema, table_name
+
+
+def test_lastrowid_does_not_create_extra_commit(
+    exasol_config, users_table, pyodbc_connection_string
+):
+    """
+    For further details on this regression see `Issue-335 <https://github.com/exasol/sqlalchemy-exasol/issues/335>`_.
+    """
+    schema_name, table_name = users_table
+    metadata = MetaData()
+    engine = create_engine(pyodbc_connection_string)
+
+    table = Table(
+        table_name,
+        metadata,
+        Column("id", Integer, primary_key=True),
+        Column("name", String(2000)),
+        schema=schema_name,
+    )
+
+    with odbcconfig(ODBC_DRIVER):
+        conn = engine.connect()
+        trans = conn.begin()
+
+        # Insert without an explicit ID will trigger a call to `get_lastrowid`
+        # which in turn cause the unintended autocommit
+        insert_statement = insert(table).values(name="Gandalf")
+        conn.execute(insert_statement)
+        trans.rollback()
+
+        result = conn.execute(f"SELECT * FROM {schema_name}.{table_name};").fetchall()
+        conn.close()
+
+        assert len(result) == 0