From 21e985387674f4bc8d7ff129eb6789a3d4ed08be Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Fri, 9 Oct 2026 17:27:40 -0400 Subject: [PATCH 1/3] fix(sqlalchemy-spanner): isolate literal round trip test tables with unique naming and teardown Prevent schema collisions ('400 Duplicate name in schema') across test invocations by generating unique table names per test execution and guaranteeing table teardown with committed DDL and MetaData eviction. --- packages/sqlalchemy-spanner/tests/conftest.py | 111 +++++++++++------- 1 file changed, 71 insertions(+), 40 deletions(-) diff --git a/packages/sqlalchemy-spanner/tests/conftest.py b/packages/sqlalchemy-spanner/tests/conftest.py index ec16ad532dde..1fa433ef1439 100644 --- a/packages/sqlalchemy-spanner/tests/conftest.py +++ b/packages/sqlalchemy-spanner/tests/conftest.py @@ -15,6 +15,7 @@ # limitations under the License. import importlib +import uuid from contextlib import contextmanager from unittest.mock import MagicMock @@ -57,11 +58,33 @@ def no_op_trace_call(*args, **kwargs): from sqlalchemy.testing.plugin.pytestplugin import * # noqa: E402, F401, F403 +def _commit_ddl(connection): + """Flush and commit buffered DDL in Spanner DBAPI or underlying connections.""" + dbapi_conn = getattr(connection, "connection", connection) + commit_fn = getattr(dbapi_conn, "commit", None) + if callable(commit_fn): + commit_fn() + + @pytest.fixture def literal_round_trip_spanner(metadata, connection): - # for literal, we test the literal render in an INSERT - # into a typed column. we can then SELECT it back as its - # official type; + """Test literal SQL rendering in INSERT and SELECT with full table isolation. + + In SQLAlchemy, literal rendering tests verify that Python data types (such as + Strings with special escape characters, Decimals, or Booleans) are correctly + rendered directly into raw SQL text (e.g., 'SELECT ... WHERE x = 15.7563') + without syntax corruption or parameter escaping failures. + + To ensure test isolation in Cloud Spanner, this fixture: + 1. Generates a unique table name per invocation using the column type and + UUID entropy. + 2. Explicitly commits table creation DDL so Spanner creates the table + immediately. + 3. Executes the insert and select assertions. + 4. Guarantees in a finally block that the table is dropped, DDL is committed, + and the table reference is evicted from metadata so subsequent tests and + class teardowns do not collide. + """ def run( type_, @@ -71,44 +94,52 @@ def run( compare=None, support_whereclause=True, ): - t = Table("t_literal_round_trip_spanner", metadata, Column("x", type_)) + table_name = f"t_lit_{type_.__class__.__name__.lower()}_{uuid.uuid4().hex[:6]}" + t = Table(table_name, metadata, Column("x", type_)) t.create(connection) - - for value in input_: - ins = t.insert().values(x=literal(value, type_)) - connection.execute(ins) - - if support_whereclause: - if compare: - stmt = t.select().where( - t.c.x - == literal( - compare, - type_, - ), - t.c.x - == literal( - input_[0], - type_, - ), - ) - else: - stmt = t.select().where( - t.c.x - == literal( - compare if compare is not None else input_[0], - type_, + _commit_ddl(connection) + + try: + for value in input_: + ins = t.insert().values(x=literal(value, type_)) + connection.execute(ins) + _commit_ddl(connection) + + if support_whereclause: + if compare: + stmt = t.select().where( + t.c.x + == literal( + compare, + type_, + ), + t.c.x + == literal( + input_[0], + type_, + ), + ) + else: + stmt = t.select().where( + t.c.x + == literal( + compare if compare is not None else input_[0], + type_, + ) ) - ) - else: - stmt = t.select() - - rows = connection.execute(stmt).all() - assert rows, "No rows returned" - for row in rows: - value = row[0] - if filter_ is not None: - value = filter_(value) - assert value in output + else: + stmt = t.select() + + rows = connection.execute(stmt).all() + assert rows, "No rows returned" + for row in rows: + value = row[0] + if filter_ is not None: + value = filter_(value) + assert value in output + finally: + t.drop(connection, checkfirst=True) + _commit_ddl(connection) + metadata.remove(t) return run From f30a0b17ebdbd18763c30d6499b721d60585c15d Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Mon, 12 Oct 2026 07:20:47 -0400 Subject: [PATCH 2/3] fix(sqlalchemy-spanner): resolve DDL concurrency and cleanup in literal test fixture --- packages/sqlalchemy-spanner/tests/conftest.py | 117 +++++++++++------- 1 file changed, 71 insertions(+), 46 deletions(-) diff --git a/packages/sqlalchemy-spanner/tests/conftest.py b/packages/sqlalchemy-spanner/tests/conftest.py index 1fa433ef1439..cacc75412f6e 100644 --- a/packages/sqlalchemy-spanner/tests/conftest.py +++ b/packages/sqlalchemy-spanner/tests/conftest.py @@ -60,12 +60,28 @@ def no_op_trace_call(*args, **kwargs): def _commit_ddl(connection): """Flush and commit buffered DDL in Spanner DBAPI or underlying connections.""" - dbapi_conn = getattr(connection, "connection", connection) + # TODO: modern sqlalchemy uses dbapi_connection, remove the getattr check + # for "connection" once we drop support for sqlalchemy < 2.0 + dbapi_conn = getattr( + connection, "dbapi_connection", getattr(connection, "connection", connection) + ) commit_fn = getattr(dbapi_conn, "commit", None) if callable(commit_fn): commit_fn() +def _rollback_txn(connection): + """Roll back any active Spanner transaction to release schema and row locks.""" + # TODO: modern sqlalchemy uses dbapi_connection, remove the getattr check + # for "connection" once we drop support for sqlalchemy < 2.0 + dbapi_conn = getattr( + connection, "dbapi_connection", getattr(connection, "connection", connection) + ) + rollback_fn = getattr(dbapi_conn, "rollback", None) + if callable(rollback_fn): + rollback_fn() + + @pytest.fixture def literal_round_trip_spanner(metadata, connection): """Test literal SQL rendering in INSERT and SELECT with full table isolation. @@ -78,12 +94,14 @@ def literal_round_trip_spanner(metadata, connection): To ensure test isolation in Cloud Spanner, this fixture: 1. Generates a unique table name per invocation using the column type and UUID entropy. - 2. Explicitly commits table creation DDL so Spanner creates the table - immediately. - 3. Executes the insert and select assertions. - 4. Guarantees in a finally block that the table is dropped, DDL is committed, - and the table reference is evicted from metadata so subsequent tests and - class teardowns do not collide. + 2. Wraps table creation in the primary try block so table reference cleanup + in metadata is guaranteed even if creation fails. + 3. Flushes and commits table creation DDL immediately. + 4. Executes the insert and select assertions. + 5. Cleans up any active read-write transactions before attempting DDL drops, + preventing Spanner's concurrent schema change rejection. + 6. Drops the table and uses a nested finally block to guarantee table eviction + from metadata even if the drop operation fails. """ def run( @@ -96,50 +114,57 @@ def run( ): table_name = f"t_lit_{type_.__class__.__name__.lower()}_{uuid.uuid4().hex[:6]}" t = Table(table_name, metadata, Column("x", type_)) - t.create(connection) - _commit_ddl(connection) try: - for value in input_: - ins = t.insert().values(x=literal(value, type_)) - connection.execute(ins) + t.create(connection) _commit_ddl(connection) - if support_whereclause: - if compare: - stmt = t.select().where( - t.c.x - == literal( - compare, - type_, - ), - t.c.x - == literal( - input_[0], - type_, - ), - ) - else: - stmt = t.select().where( - t.c.x - == literal( - compare if compare is not None else input_[0], - type_, + try: + for value in input_: + ins = t.insert().values(x=literal(value, type_)) + connection.execute(ins) + + if support_whereclause: + if compare: + stmt = t.select().where( + t.c.x + == literal( + compare, + type_, + ), + t.c.x + == literal( + input_[0], + type_, + ), ) - ) - else: - stmt = t.select() - - rows = connection.execute(stmt).all() - assert rows, "No rows returned" - for row in rows: - value = row[0] - if filter_ is not None: - value = filter_(value) - assert value in output + else: + stmt = t.select().where( + t.c.x + == literal( + compare if compare is not None else input_[0], + type_, + ) + ) + else: + stmt = t.select() + + rows = connection.execute(stmt).all() + assert rows, "No rows returned" + for row in rows: + value = row[0] + if filter_ is not None: + value = filter_(value) + assert value in output + finally: + _rollback_txn(connection) finally: - t.drop(connection, checkfirst=True) - _commit_ddl(connection) - metadata.remove(t) + try: + t.drop(connection, checkfirst=True) + _commit_ddl(connection) + except Exception: + pass + finally: + metadata.remove(t) return run From eca5f8f543521af583ba4fad5f11e97514d091a6 Mon Sep 17 00:00:00 2001 From: chalmer lowe Date: Tue, 13 Oct 2026 06:54:04 -0400 Subject: [PATCH 3/3] refactor(sqlalchemy-spanner): encapsulate table lifecycle in context manager --- packages/sqlalchemy-spanner/tests/conftest.py | 121 +++++++++--------- 1 file changed, 61 insertions(+), 60 deletions(-) diff --git a/packages/sqlalchemy-spanner/tests/conftest.py b/packages/sqlalchemy-spanner/tests/conftest.py index cacc75412f6e..0aebf344b275 100644 --- a/packages/sqlalchemy-spanner/tests/conftest.py +++ b/packages/sqlalchemy-spanner/tests/conftest.py @@ -82,6 +82,47 @@ def _rollback_txn(connection): rollback_fn() +@contextmanager +def _temporary_spanner_table(metadata, connection, table_name, *columns): + """Context manager managing an isolated Spanner table lifecycle. + + Guarantees that: + 1. Table creation DDL is committed immediately. + 2. Any active read-write transactions are rolled back prior to DDL drop. + 3. The table is dropped and its DDL committed. + 4. The table reference is evicted from SQLAlchemy metadata even if drop fails. + """ + t = Table(table_name, metadata, *columns) + try: + t.create(connection) + _commit_ddl(connection) + yield t + finally: + _rollback_txn(connection) + try: + t.drop(connection, checkfirst=True) + _commit_ddl(connection) + except Exception: + pass + finally: + metadata.remove(t) + + +def _build_literal_select(table, type_, input_val, compare, support_whereclause): + """Build a SELECT statement for literal testing with optional WHERE clause.""" + if not support_whereclause: + return table.select() + + if compare: + return table.select().where( + table.c.x == literal(compare, type_), + table.c.x == literal(input_val, type_), + ) + + target_val = compare if compare is not None else input_val + return table.select().where(table.c.x == literal(target_val, type_)) + + @pytest.fixture def literal_round_trip_spanner(metadata, connection): """Test literal SQL rendering in INSERT and SELECT with full table isolation. @@ -94,14 +135,9 @@ def literal_round_trip_spanner(metadata, connection): To ensure test isolation in Cloud Spanner, this fixture: 1. Generates a unique table name per invocation using the column type and UUID entropy. - 2. Wraps table creation in the primary try block so table reference cleanup - in metadata is guaranteed even if creation fails. - 3. Flushes and commits table creation DDL immediately. - 4. Executes the insert and select assertions. - 5. Cleans up any active read-write transactions before attempting DDL drops, - preventing Spanner's concurrent schema change rejection. - 6. Drops the table and uses a nested finally block to guarantee table eviction - from metadata even if the drop operation fails. + 2. Uses _temporary_spanner_table to manage table creation, transaction + rollback, drop, and metadata eviction cleanly. + 3. Executes the insert and select assertions with _build_literal_select. """ def run( @@ -113,58 +149,23 @@ def run( support_whereclause=True, ): table_name = f"t_lit_{type_.__class__.__name__.lower()}_{uuid.uuid4().hex[:6]}" - t = Table(table_name, metadata, Column("x", type_)) - try: - t.create(connection) - _commit_ddl(connection) - - try: - for value in input_: - ins = t.insert().values(x=literal(value, type_)) - connection.execute(ins) - - if support_whereclause: - if compare: - stmt = t.select().where( - t.c.x - == literal( - compare, - type_, - ), - t.c.x - == literal( - input_[0], - type_, - ), - ) - else: - stmt = t.select().where( - t.c.x - == literal( - compare if compare is not None else input_[0], - type_, - ) - ) - else: - stmt = t.select() - - rows = connection.execute(stmt).all() - assert rows, "No rows returned" - for row in rows: - value = row[0] - if filter_ is not None: - value = filter_(value) - assert value in output - finally: - _rollback_txn(connection) - finally: - try: - t.drop(connection, checkfirst=True) - _commit_ddl(connection) - except Exception: - pass - finally: - metadata.remove(t) + with _temporary_spanner_table( + metadata, connection, table_name, Column("x", type_) + ) as t: + for value in input_: + ins = t.insert().values(x=literal(value, type_)) + connection.execute(ins) + + stmt = _build_literal_select( + t, type_, input_[0], compare, support_whereclause + ) + rows = connection.execute(stmt).all() + assert rows, "No rows returned" + for row in rows: + value = row[0] + if filter_ is not None: + value = filter_(value) + assert value in output return run