fix(sqlalchemy-spanner): resolve compliance and migration failures with Alembic 1.20 and SQLAlchemy 2.1

packages/sqlalchemy-spanner/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py

@@ -15,14 +15,6 @@
 import re
 
 import sqlalchemy
-from alembic.ddl.base import (
-    ColumnNullable,
-    ColumnType,
-    alter_column,
-    alter_table,
-    format_server_default,
-    format_type,
-)
 from google.api_core.client_options import ClientOptions
 from google.auth.credentials import AnonymousCredentials
 from google.cloud.spanner_v1 import Client, TransactionOptions
@@ -51,6 +43,26 @@
 from google.cloud.sqlalchemy_spanner import version as sqlalchemy_spanner_version
 from google.cloud.sqlalchemy_spanner._opentelemetry_tracing import trace_call
 
+# Defensively decouple the Alembic import so the Spanner dialect does not
+# hard-depend on Alembic at runtime. A database dialect does not inherently
+# require a schema migration tool, and consumers using only SQLAlchemy Core
+# or ORM (or managing DDL outside of Alembic) can still import and use the
+# dialect even if Alembic is omitted or unavailable in the environment
+# (see #18584).
+try:
+    from alembic.ddl.base import (
+        ColumnNullable,
+        ColumnType,
+        alter_column,
+        alter_table,
+        format_server_default,
+        format_type,
+    )
+
+    HAS_ALEMBIC_INSTALLED = True
+except ImportError:
+    HAS_ALEMBIC_INSTALLED = False
+
 USING_SQLACLCHEMY_20 = False
 if sqlalchemy.__version__.split(".")[0] == "2":
     USING_SQLACLCHEMY_20 = True
@@ -1882,55 +1894,60 @@ def do_execute_no_params(self, cursor, statement, context=None):
             cursor.execute(statement)
 
 
-# Alembic ALTER operation override
-@compiles(ColumnNullable, "spanner+spanner")
-def visit_column_nullable(
-    element: "ColumnNullable", compiler: "SpannerDDLCompiler", **kw
-) -> str:
-    return _format_alter_column(
-        compiler,
-        element.table_name,
-        element.schema,
-        element.column_name,
-        element.existing_type,
-        element.nullable,
-        element.existing_server_default,
-    )
-
-
-# Alembic ALTER operation override
-@compiles(ColumnType, "spanner+spanner")
-def visit_column_type(
-    element: "ColumnType", compiler: "SpannerDDLCompiler", **kw
-) -> str:
-    return _format_alter_column(
-        compiler,
-        element.table_name,
-        element.schema,
-        element.column_name,
-        element.type_,
-        element.existing_nullable,
-        element.existing_server_default,
-    )
+# Cloud Spanner requires ALTER TABLE ... ALTER COLUMN statements to specify the
+# complete column definition (type, nullability, and default expression), whereas
+# Alembic's default DDL compiler emits partial clauses (e.g., only SET NOT NULL
+# or TYPE). Because the @compiles decorators reference Alembic's ColumnNullable
+# and ColumnType classes at module import time, we only register these overrides
+# when Alembic is available in the environment.
+if HAS_ALEMBIC_INSTALLED:
+    # Alembic ALTER operation override
+    @compiles(ColumnNullable, "spanner+spanner")
+    def visit_column_nullable(
+        element: "ColumnNullable", compiler: "SpannerDDLCompiler", **kw
+    ) -> str:
+        return _format_alter_column(
+            compiler,
+            element.table_name,
+            element.schema,
+            element.column_name,
+            element.existing_type,
+            element.nullable,
+            element.existing_server_default,
+        )
 
+    # Alembic ALTER operation override
+    @compiles(ColumnType, "spanner+spanner")
+    def visit_column_type(
+        element: "ColumnType", compiler: "SpannerDDLCompiler", **kw
+    ) -> str:
+        return _format_alter_column(
+            compiler,
+            element.table_name,
+            element.schema,
+            element.column_name,
+            element.type_,
+            element.existing_nullable,
+            element.existing_server_default,
+        )
 
-def _format_alter_column(
-    compiler, table_name, schema, column_name, type_, nullable, server_default
-):
-    # Older versions of SQLAlchemy pass in a boolean to indicate whether there
-    # is an existing DEFAULT constraint, instead of the actual DEFAULT constraint
-    # expression. In those cases, we do not want to explicitly include the DEFAULT
-    # constraint in the expression that is generated here.
-    if isinstance(server_default, bool):
-        server_default = None
-    return "%s %s %s%s%s" % (
-        alter_table(compiler, table_name, schema),
-        alter_column(compiler, column_name),
-        format_type(compiler, type_),
-        "" if nullable else " NOT NULL",
-        (
-            ""
-            if server_default is None
-            else f" DEFAULT {format_server_default(compiler, server_default)}"
-        ),
-    )
+    def _format_alter_column(
+        compiler, table_name, schema, column_name, type_, nullable, server_default
+    ):
+        # Older versions of SQLAlchemy pass in a boolean to indicate whether there
+        # is an existing DEFAULT constraint, instead of the actual DEFAULT constraint
+        # expression. In those cases, we do not want to explicitly include the DEFAULT
+        # constraint in the expression that is generated here.
+        if isinstance(server_default, bool):
+            server_default = None
+        return "%s %s %s%s%s" % (
+            alter_table(compiler, table_name, schema),
+            alter_column(compiler, column_name),
+            format_type(compiler, type_),
+            "" if nullable else " NOT NULL",
+            (
+                ""
+                if server_default is None
+                else f" DEFAULT {format_server_default(compiler, server_default)}"
+            ),
+        )

packages/sqlalchemy-spanner/noxfile.py

@@ -120,6 +120,7 @@ class = StreamHandler
 
 SQLALCHEMY_14_DEPENDENCIES = [
     "sqlalchemy>=1.4,<2.0",
+    "alembic<1.20.0",
 ]
 
 SQLALCHEMY_20_DEPENDENCIES = [
@@ -139,7 +140,6 @@ class = StreamHandler
     "compliance_test_14",
     "compliance_test_20",
     "migration_test",
-    "_migration_test",
     "mockserver",
 ]
 
@@ -206,13 +206,7 @@ def compliance_test_14(session):
 
     try:
         session.install(*SYSTEM_TEST_STANDARD_DEPENDENCIES)
-        session.install(".[tracing]")
-        session.run(
-            "pip",
-            "install",
-            *SQLALCHEMY_14_DEPENDENCIES,
-            "--force-reinstall",
-        )
+        session.install(".[tracing]", *SQLALCHEMY_14_DEPENDENCIES)
         session.run(
             "python",
             "create_test_database.py",
@@ -335,29 +329,35 @@ def mockserver(session):
     )
 
 
-@nox.session(python=SYSTEM_COMPLIANCE_MIGRATION_TEST_PYTHON_VERSIONS[0])
-def migration_test(session):
-    """Test migrations with SQLAlchemy v1.4 and Alembic"""
-    session.run(
-        "pip",
-        "install",
-        *SQLALCHEMY_14_DEPENDENCIES,
-        "--force-reinstall",
-    )
-    _migration_test(session)
+@nox.session
+@nox.parametrize(
+    ("python", "extra_dependencies"),
+    [
+        (
+            SYSTEM_COMPLIANCE_MIGRATION_TEST_PYTHON_VERSIONS[0],
+            SQLALCHEMY_14_DEPENDENCIES,
+        ),
+        (
+            DEFAULT_PYTHON_VERSION_FOR_SQLALCHEMY_20,
+            SQLALCHEMY_20_DEPENDENCIES,
+        ),
+    ],
+    ids=["14", "20"],
+)
+def migration_test(session, extra_dependencies):
+    """Test migrations with SQLAlchemy and Alembic."""
+    _run_migration_test(session, extra_dependencies=extra_dependencies)
 
 
-@nox.session(python=SYSTEM_COMPLIANCE_MIGRATION_TEST_PYTHON_VERSIONS[-1])
-def _migration_test(session):
+def _run_migration_test(session, extra_dependencies=()):
     """Migrate with SQLAlchemy and Alembic and check the result."""
     import glob
     import os
     import shutil
 
     config_file = f"test_migration_{session.python}_{uuid.uuid4().hex[:6]}.cfg"
 
-    session.install(*MIGRATION_TEST_DEPENDENCIES)
-    session.install(".")
+    session.install(*MIGRATION_TEST_DEPENDENCIES, *extra_dependencies, ".")
 
     try:
         session.run(
@@ -482,9 +482,13 @@ def system(session, test_type):
     elif test_type == "compliance_20":
         return compliance_test_20(session)
     elif test_type == "migration_14":
-        return migration_test(session)
+        return _run_migration_test(
+            session, extra_dependencies=SQLALCHEMY_14_DEPENDENCIES
+        )
     elif test_type == "migration_20":
-        return _migration_test(session)
+        return _run_migration_test(
+            session, extra_dependencies=SQLALCHEMY_20_DEPENDENCIES
+        )
 
     config_file = f"test_{test_type}_{session.python}_{uuid.uuid4().hex[:6]}.cfg"
 

packages/sqlalchemy-spanner/setup.py

@@ -22,9 +22,9 @@
 name = "sqlalchemy-spanner"
 description = "SQLAlchemy dialect integrated into Cloud Spanner database"
 dependencies = [
-    "sqlalchemy>=1.1.13",
+    "sqlalchemy>=1.1.13,<3.0.0",
     "google-cloud-spanner>=3.55.0",
-    "alembic",
+    "alembic>=1.0.0,<2.0.0",
 ]
 extras = {
     "tracing": [

packages/sqlalchemy-spanner/tests/test_suite_20.py

@@ -78,7 +78,6 @@
     LongNameBlowoutTest as _LongNameBlowoutTest,
 )
 from sqlalchemy.testing.suite.test_ddl import TableDDLTest as _TableDDLTest
-from sqlalchemy.testing.suite.test_deprecations import *  # noqa: F401, F403
 from sqlalchemy.testing.suite.test_dialect import *  # noqa: F401, F403
 from sqlalchemy.testing.suite.test_dialect import (
     DifficultParametersTest as _DifficultParametersTest,
@@ -225,6 +224,15 @@
     get_project,
 )
 
+try:
+    # SQLAlchemy 2.1+ removed test_deprecations from sqlalchemy.testing.suite.
+    # Guard this import so the test suite remains compatible with both 2.0.x and 2.1+.
+    # Tell flake8 to ignore F401 (unused import) and F403 (wildcard import) since
+    # pytest discovers the imported SQLAlchemy compliance test classes at module scope.
+    from sqlalchemy.testing.suite.test_deprecations import *  # noqa: F401, F403
+except ModuleNotFoundError:
+    pass
+
 config.test_schema = ""
 
 
@@ -721,6 +729,7 @@ def test_get_columns(self, connection, use_views, use_schema):
                             [
                                 types.Integer,
                                 types.Numeric,
+                                types.Float,
                                 types.DateTime,
                                 types.Date,
                                 types.Time,
@@ -2916,6 +2925,10 @@ def test_has_table_nonexistent_schema(self):
     def test_has_table_schema(self):
         pass
 
+    @pytest.mark.skip("Not supported by Cloud Spanner")
+    def test_has_multi_table_schema(self):
+        pass
+
     @pytest.mark.skip("Not supported by Cloud Spanner")
     def test_has_table_cache(self):
         pass
@@ -3066,19 +3079,26 @@ def test_eval_none_flag_orm(self):
         pass
 
     @pytest.mark.skip(
-        "Spanner JSON_VALUE() always returns STRING,"
+        "Spanner JSON_VALUE() always returns STRING, "
         "thus, this test case can't be executed."
     )
     def test_index_typed_comparison(self):
         pass
 
     @pytest.mark.skip(
-        "Spanner JSON_VALUE() always returns STRING,"
+        "Spanner JSON_VALUE() always returns STRING, "
         "thus, this test case can't be executed."
     )
     def test_path_typed_comparison(self):
         pass
 
+    @pytest.mark.skip(
+        "Spanner JSON_VALUE() always returns STRING, "
+        "thus, this test case can't be executed."
+    )
+    def test_index_cross_casts(self):
+        pass
+
     @pytest.mark.skip("Custom JSON de-/serializers are not supported.")
     def test_round_trip_custom_json(self):
         pass

packages/sqlalchemy-spanner/tests/unit/test_alembic.py

@@ -12,8 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import importlib.util
+import sys
+from unittest import mock
+
 from alembic.ddl import base as ddl_base
-from sqlalchemy import String, TextClause
+from sqlalchemy import String, TextClause, event
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing.plugin.plugin_base import fixtures
 
@@ -96,3 +100,33 @@ def test_visit_column_type_with_default(self):
             "ALTER COLUMN col "
             "STRING(256) NOT NULL DEFAULT (GENERATE_UUID())",
         )
+
+    def test_dialect_import_without_alembic(self):
+        """Verify that sqlalchemy_spanner imports cleanly when Alembic is unavailable.
+
+        Why we test this way instead of calling importlib.reload(sqlalchemy_spanner):
+        1. Setting sys.modules["alembic.ddl.base"] = None via mock.patch.dict causes
+           Python to raise ModuleNotFoundError (a subclass of ImportError) when
+           sqlalchemy_spanner attempts to import from alembic.ddl.base.
+        2. Calling importlib.reload(sqlalchemy_spanner) would re-execute the module
+           in-place inside the existing sqlalchemy_spanner.__dict__. That has two
+           undesirable side effects:
+           - Functions defined during the initial import (like visit_column_nullable)
+             remain in sqlalchemy_spanner.__dict__ even if skipped on reload.
+           - Classes (like SpannerIdentifierPreparer) are recreated with new class
+             identities in sqlalchemy_spanner.__dict__, which breaks other test
+             modules (such as test_dialect.py) that already imported SpannerDialect
+             before the reload and rely on super(SpannerIdentifierPreparer, self).
+        3. Creating a fresh module object via importlib.util.module_from_spec and
+           executing the loader into that isolated namespace tests a clean import
+           without mutating the shared sqlalchemy_spanner module in sys.modules.
+        """
+        with mock.patch.dict(sys.modules, {"alembic.ddl.base": None}):
+            module = importlib.util.module_from_spec(sqlalchemy_spanner.__spec__)
+            sqlalchemy_spanner.__spec__.loader.exec_module(module)
+            # Executing the module registers module.reset_connection on the shared
+            # SQLAlchemy Pool class via @listens_for(Pool, "reset"); remove that
+            # temporary listener so global Pool state stays clean for other tests.
+            event.remove(sqlalchemy_spanner.Pool, "reset", module.reset_connection)
+            assert not module.HAS_ALEMBIC_INSTALLED
+            assert not hasattr(module, "visit_column_nullable")