fix(spanner, sqlalchemy-spanner): fix reflection crashes, add native UUID support, and improve JsonObject - #17822
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
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.
|
Done. Please merge main into this branch |
a16c261 to
2959f26
Compare
…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.
2959f26 to
c5bae33
Compare
olavloite
left a comment
olavloite
left a comment
There was a problem hiding this comment.
Can we split this PR into at least 2, but preferably 4 separate pull requests? It currently combines changes for both the Spanner client library and SQLAlchemy. Those are released as separate packages, so there is no simple way of generating the release notes from this pull request.
Also, the changes for SQLAlchemy are actually 3 different changes. Adding UUID, adding TOKENLIST, and fixing spanner_storing handling. Best practice is to create separate pull requests for each, so they can be listed as such in the release notes.
| expected = json.dumps(data, sort_keys=True, separators=(",", ":")) | ||
| data_jsonobject = JsonObject(JsonObject(data)) | ||
| self.assertEqual(data_jsonobject.serialize(), expected) | ||
|
|
There was a problem hiding this comment.
Can we add a test for a nested JsonObject structure? For example a JsonObject containing an array of JsonObject?
3 participants
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> 🦕