Skip to content

fix(sqlalchemy-spanner): group multi-column unique constraints and add get_multi_unique_constraints - #18842

Open
rocksnow1942 wants to merge 4 commits into
googleapis:mainfrom
rocksnow1942:fix-18836-18837-unique-constraints
Open

rocksnow1942 wants to merge 4 commits into
googleapis:mainfrom
rocksnow1942:fix-18836-18837-unique-constraints

Conversation

@rocksnow1942

@rocksnow1942 rocksnow1942 commented Oct 24, 2026 •

Copy link
Copy Markdown

SpannerDialect.get_unique_constraints() read the columns of unique constraints from INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE, which has no column position, and returned one entry per column. A two-column constraint came back as two single-column constraints with the same name, in no particular order. On the emulator, which repeats the backing unique constraint of a foreign key once per referencing foreign key, one two-column constraint came back as eight entries.

SpannerDialect also didn't implement get_multi_unique_constraints(), so multi-table reflection (MetaData.reflect(), Inspector.get_multi_unique_constraints()) fell back to one query per table.

This PR:

  • adds get_multi_unique_constraints(), which reads the unique constraints of all tables in one query with their columns from KEY_COLUMN_USAGE in ORDINAL_POSITION order, and skips the columns the emulator repeats;
  • makes get_unique_constraints() call it, as get_columns(), get_pk_constraint(), get_foreign_keys() and get_indexes() do with their get_multi_*() methods;
  • replaces the skipped test_get_multi_unique_constraints compliance test with a Spanner override.

A question for reviewers: Should we leave out the unique constraints?

Spanner DDL has no unique constraints, and unique indexes aren't listed in TABLE_CONSTRAINTS, so the only UNIQUE rows there are the indexes that Spanner creates to back foreign keys that refer to columns other than the primary key (IDX_<table>_<columns>_U_<hash>). get_multi_indexes() already leaves these Spanner-managed indexes out. Should the unique constraints leave them out too? Alembic's autogenerate emits op.drop_constraint() for them, which Spanner rejects with ... is not a constraint in .... This PR keeps returning them, as before.

Tested with the unit and mockserver tests and the SQLAlchemy 1.4 and 2.x compliance suites against the emulator.

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #18836
Fixes #18837

…d get_multi_unique_constraints

get_unique_constraints() returned one entry per column of a multi-column
unique constraint, in no particular order. Add get_multi_unique_constraints(),
which reads the unique constraints of all tables in one query with their
columns in KEY_COLUMN_USAGE order, and make get_unique_constraints() use it,
like the other get_*() methods.

Fixes googleapis#18836
Fixes googleapis#18837

@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 implements the get_multi_unique_constraints method and refactors get_unique_constraints to use it, alongside adding corresponding integration and unit tests. However, a critical issue was identified in the unit tests where the test_max_size_exported function definition was accidentally deleted, causing its body to be incorrectly merged into test_get_unique_constraints.

Comment thread packages/sqlalchemy-spanner/tests/unit/test_dialect.py
The previous commit dropped its def line, so its body ran as part of
test_get_unique_constraints.
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
…unique constraints query

Also write the INFORMATION_SCHEMA table and column names in upper case, as
the replaced query did.
Group by schema, table and constraint name and collect the columns with
ARRAY_AGG(... ORDER BY ORDINAL_POSITION), instead of ordering one row per
column and grouping them in Python.
@rocksnow1942
rocksnow1942 marked this pull request as ready for review October 24, 2026 10:27
@rocksnow1942
rocksnow1942 requested a review from a team as a code owner October 24, 2026 10:27

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant