Skip to content

fix(sqlalchemy-spanner): handle empty filter_names in multi-table reflection - #18843

Open
rocksnow1942 wants to merge 1 commit into
googleapis:mainfrom
rocksnow1942:fix-18838-empty-filter-names
Open

rocksnow1942 wants to merge 1 commit into
googleapis:mainfrom
rocksnow1942:fix-18838-empty-filter-names

Conversation

@rocksnow1942

Copy link
Copy Markdown

_get_table_filter_query() rendered () for an empty filter_names, so the get_multi_*() queries failed with 400 Syntax error: Unexpected ")". SQLAlchemy passes an empty filter_names when there's nothing to reflect, so these failed:

  • MetaData().reflect(engine) on a database without tables
  • MetaData.reflect() with only=[], or when all the tables are already in the MetaData
  • Alembic >= 1.18 autogenerate on an empty database, since it pre-loads each schema with the get_multi_*() methods

This PR renders (FALSE) instead, so an empty filter_names returns no tables.

SQLAlchemy doesn't define what an empty filter_names means. Its built-in dialects check if filter_names:, so they treat [] like None and return all tables, while the IN UNNEST(@filter_names) filter of #18205 returns none. This PR returns none, which avoids reflecting the whole schema when there's nothing to load. I'm happy to switch to the other behavior if you prefer.

Tested with the unit and mockserver tests and the compliance suite 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 #18838

…lection

An empty filter_names rendered "()" in the WHERE clause of the get_multi_*()
queries, so MetaData.reflect() failed with a syntax error when it had no
tables to load, e.g. on an empty database. Render an always-false condition
instead, so an empty filter_names returns no tables.

Fixes googleapis#18838

@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 fixes an issue where an empty filter_names argument in _get_table_filter_query would generate invalid SQL (()). The fix ensures that an empty filter renders a condition that is always false ((FALSE)), selecting no tables. Additionally, comprehensive unit and integration tests have been added to verify this behavior during metadata reflection and multi-column retrieval. There are no review comments, and I have no additional feedback to provide.

@rocksnow1942
rocksnow1942 marked this pull request as ready for review October 24, 2026 09:52
@rocksnow1942
rocksnow1942 requested a review from a team as a code owner October 24, 2026 09:52

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

Development

Successfully merging this pull request may close these issues.

[sqlalchemy-spanner]: _get_table_filter_query([]) generates invalid SQL '() AND ' when filter_names is empty

1 participant