Skip to content

feat(sqlalchemy-spanner): native UUID data type support in dialect - #17913

Open
sakthivelmanii wants to merge 3 commits into
mainfrom
feat-spanner-native-uuid
Open

feat(sqlalchemy-spanner): native UUID data type support in dialect#17913
sakthivelmanii wants to merge 3 commits into
mainfrom
feat-spanner-native-uuid

Conversation

@sakthivelmanii

@sakthivelmanii sakthivelmanii commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Adds native Spanner UUID type support to SQLAlchemy dialect:

  • Registers "UUID": types.UUID in _type_map for reflection and _type_map_inv.
  • types.UUID (all-caps) always emits native UUID DDL.
  • types.Uuid (CamelCase) defaults to emitting STRING(36) for backward compatibility with legacy string-backed UUID columns.
  • Native UUID Flag: To enable native UUID DDL for generic types.Uuid columns, set supports_native_uuid = True on the dialect:
    engine = create_engine(...)
    engine.dialect.supports_native_uuid = True

@sakthivelmanii
sakthivelmanii requested a review from a team as a code owner July 27, 2026 15:37

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces native UUID support to the Spanner SQLAlchemy dialect by registering the UUID type mapping, enabling supports_native_uuid on the dialect, and adding corresponding unit tests. The review feedback suggests two improvements: first, registering both types.UUID and types.Uuid in the inverse type map to prevent potential mapping failures in SQLAlchemy 2.0; second, removing the redundant visit_UUID method in SpannerTypeCompiler as the base class already handles UUID compilation correctly while respecting the supports_native_uuid flag.

Comment thread packages/sqlalchemy-spanner/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py Outdated
Comment thread packages/sqlalchemy-spanner/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py Outdated
@sakthivelmanii
sakthivelmanii force-pushed the feat-spanner-native-uuid branch 2 times, most recently from 3952c47 to 809b53f Compare July 27, 2026 15:53
@sakthivelmanii

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds native UUID support to the Spanner dialect, including type mapping, DDL compilation, and corresponding unit tests. However, a critical bug was identified in the visit_uuid compiler method, where calling self.visit_UUID will raise an AttributeError because that method is not defined. The reviewer provided a code suggestion to directly return the string "UUID" when native UUID is supported.

supports_identity_columns = True
supports_native_boolean = True
supports_native_decimal = True
supports_native_uuid = True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (probably) makes this a breaking change. I think that applications today can use UUID as a type in their application model. SQLAlchemy will then automatically assume that they are stored as strings and convert them to UUIDs in memory. By changing the (default) value of this property, that automatic conversion is no longer being done, even though the value is still stored and returned as a string from the database, as it was (probably) created as a string column.

I think that we need to add some tests for how this behaved before this change when someone had created a model like this:

import uuid
from sqlalchemy import Column, String, Uuid
from sqlalchemy.orm import declarative_base

Base = declarative_base()

class User(Base):
    __tablename__ = "users"

    # Declared using SQLAlchemy 2.0's standard Uuid type
    id = Column(Uuid, primary_key=True)
    name = Column(String(100))

The above model would probably have used a string column for the id property, and SQLAlchemy would automatically convert it to a UUID in memory. That (probably) does not happen anymore after this change.

@sakthivelmanii sakthivelmanii Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for raising this! I have evaluated the backward compatibility implications and decided to keep supports_native_uuid = True enabled by default for the following reasons:


1. Issue with Existing visit_uuid (CHAR(32) Syntax Error)

Previously, if a developer declared a model using SQLAlchemy 2.0's standard Uuid type:

class User(Base):
    __tablename__ = "users"

    id = Column(Uuid, primary_key=True)

SQLAlchemy Core's default fallback compiled Uuid to CHAR(32), generating DDL:

CREATE TABLE users (
    id CHAR(32) NOT NULL
) PRIMARY KEY (id)

This query failed in Spanner with a syntax error because Spanner GoogleSQL does not support the CHAR data type.


2. Workaround Required Monkey-Patching or Custom TypeDecorator

Because Column(Uuid) failed out of the box, applications had to resort to monkey-patching the type compiler at startup:

# Workaround: Monkey-patch SpannerTypeCompiler to output STRING(36)
from google.cloud.sqlalchemy_spanner.sqlalchemy_spanner import SpannerTypeCompiler

SpannerTypeCompiler.visit_uuid = lambda self, type_, **kw: "STRING(36)"

Or writing a custom TypeDecorator:

class UUIDString(TypeDecorator):
    impl = String(36)
    cache_ok = True

    def process_bind_param(self, val, dialect):
        return str(val) if val else None

    def process_result_value(self, val, dialect):
        return uuid.UUID(val) if val else None

3. Direct Native UUID Support Was Missing

Until now, applications could not use Spanner's native UUID data type directly in SQLAlchemy models or table reflection (table reflection crashed with KeyError: 'UUID').

Enabling supports_native_uuid = True by default brings first-class native UUID support to the dialect:

# Generates native Spanner DDL: CREATE TABLE users (id UUID NOT NULL) PRIMARY KEY (id)
class User(Base):
    __tablename__ = "users"

    id = Column(Uuid, primary_key=True)

If an existing application specifically requires legacy string-backed columns (STRING(36)), they can easily opt out:

  • Per Column: Column(types.Uuid(native_uuid=False), primary_key=True)
  • Globally on Engine/Dialect: create_engine("spanner:///...", supports_native_uuid=False)

We've added comprehensive unit tests covering both default native UUID DDL compilation and STRING(36) fallback behavior.

I suspect that no customer will be using UUID/Uuid in SQLAlchemy

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My worry is not with the DDL compiler or anything like that. My worry is with running applications that model a property as a UUID, even though it is stored as a string in the database. For those applications, the behavior will change after this pull request is merged. This test shows the difference:

import uuid
from google.cloud.spanner_dbapi import parse_utils
from google.cloud.spanner_v1 import (
    ExecuteSqlRequest,
    ResultSet,
    ResultSetMetadata,
    StructType,
    Type,
    TypeCode,
)
from sqlalchemy import String, Uuid, select
from sqlalchemy.orm import DeclarativeBase, Mapped, Session, mapped_column
from sqlalchemy.testing import eq_
from tests.mockserver_tests.mock_server_test_base import (
    MockServerTestBase,
    add_result,
)


class Base(DeclarativeBase):
    pass


class User(Base):
    __tablename__ = "users"
    id: Mapped[uuid.UUID] = mapped_column(Uuid, primary_key=True)
    name: Mapped[str] = mapped_column(String(100))


class TestUuidMockServer(MockServerTestBase):
    def test_query_existing_string_uuid_column(self):
        """Tests querying an existing table where the column in Spanner is STRING(36)."""
        raw_uuid_str = "550e8400-e29b-41d4-a716-446655440000"

        # Mock result set representing an existing table with STRING(36) column in Spanner
        res_metadata = ResultSetMetadata(
            row_type=StructType(
                fields=[
                    StructType.Field(name="id", type=Type(code=TypeCode.STRING)),
                    StructType.Field(name="name", type=Type(code=TypeCode.STRING)),
                ]
            )
        )
        res = ResultSet(
            metadata=res_metadata,
            rows=[[raw_uuid_str, "Alice"]],
        )

        sql_query = "SELECT users.id, users.name\nFROM users"
        add_result(sql_query, res)

        # -----------------------------------------------------------------
        # 1. Querying with supports_native_uuid = False (Legacy behavior)
        # -----------------------------------------------------------------
        engine_legacy = self.create_engine()
        engine_legacy.dialect.supports_native_uuid = False

        with Session(engine_legacy) as session:
            user = session.scalars(select(User)).first()
            # With supports_native_uuid=False: string from DB is automatically
            # converted into a Python uuid.UUID instance:
            eq_(type(user.id), uuid.UUID)
            eq_(user.id, uuid.UUID(raw_uuid_str))

        # -----------------------------------------------------------------
        # 2. Querying with supports_native_uuid = True (PR default behavior)
        # -----------------------------------------------------------------
        engine_native = self.create_engine()
        assert engine_native.dialect.supports_native_uuid is True

        with Session(engine_native) as session:
            user = session.scalars(select(User)).first()
            # BREAKING CHANGE: String from DB is NOT converted to uuid.UUID.
            # user.id is returned as raw str, breaking code that expects uuid.UUID:
            eq_(type(user.id), str)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense. We can have the flag disabled by default.

  • If anyone needs to use STRING(36), they can use types.Uuid
  • if anyone needs to use native UUID, they can use types.UUID

@sakthivelmanii
sakthivelmanii force-pushed the feat-spanner-native-uuid branch from 809b53f to c755982 Compare July 27, 2026 17:29
@sakthivelmanii
sakthivelmanii force-pushed the feat-spanner-native-uuid branch from c755982 to 681f3de Compare July 27, 2026 17:48
@parthea

parthea commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

@sakthivelmanii , Please could you review the failing system test?

@sakthivelmanii

sakthivelmanii commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@parthea Yes. I have re-ran it. It worked

@parthea parthea assigned olavloite and unassigned sakthivelmanii Jul 27, 2026
@sakthivelmanii
sakthivelmanii force-pushed the feat-spanner-native-uuid branch 9 times, most recently from 068fa1e to 66fa086 Compare July 28, 2026 10:50
@sakthivelmanii
sakthivelmanii force-pushed the feat-spanner-native-uuid branch from 66fa086 to b17241d Compare July 28, 2026 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants