fix(spanner, sqlalchemy-spanner): fix reflection crashes, add native UUID support, and improve JsonObject#17822
Conversation
…UUID support, and improve JsonObject - Exclude SEARCH indexes and guard None column_sorting in SpannerDialect.get_multi_indexes to prevent reflection AttributeError crashes. - Register TOKENLIST in _type_map to enable table reflection for TOKENLIST columns without KeyError. - Add native UUID support in SpannerDialect (_type_map, _type_map_inv, SpannerDDLCompiler.visit_UUID, SpannerTypeCompiler.visit_UUID/visit_uuid) while preserving STRING(36) backward compatibility. - Fix spanner_storing column resolution in SpannerDDLCompiler.visit_create_index for unbound columns in Alembic batch mode. - Add to_python() method and public properties (is_null, is_array, is_scalar) to JsonObject in google-cloud-spanner. - Add unit tests in test_dialect.py and mockserver integration tests in test_dialect_integration.py.
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements across google-cloud-spanner and sqlalchemy-spanner. Specifically, it adds helper properties and a to_python() method to JsonObject for easier unwrapping of native Python objects. In sqlalchemy-spanner, it adds support for native UUID and TOKENLIST types, improves the handling of STORING clauses in index creation, and excludes SEARCH indexes from get_multi_indexes. Feedback is provided to guard against a potential TypeError in get_multi_indexes if the column orderings array (row[5]) is None.
| "column_sorting": { | ||
| col: order.lower() for col, order in zip(row[3], row[5]) | ||
| col: (order.lower() if order else None) | ||
| for col, order in zip(row[3], row[5]) | ||
| }, |
There was a problem hiding this comment.
If row[5] (the column orderings array) is None (which can happen if the database returns a null array for certain index types or configurations), attempting to zip it with row[3] will raise a TypeError: zip argument #2 must support iteration. Guarding row[5] with a fallback to an empty list row[5] or [] prevents this potential crash.
| "column_sorting": { | |
| col: order.lower() for col, order in zip(row[3], row[5]) | |
| col: (order.lower() if order else None) | |
| for col, order in zip(row[3], row[5]) | |
| }, | |
| "column_sorting": { | |
| col: (order.lower() if order else None) | |
| for col, order in zip(row[3], row[5] or []) | |
| }, |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕