FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(spanner, sqlalchemy-spanner): fix reflection crashes, add native UUID support, and improve JsonObject by sakthivelmanii · Pull Request #17822 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

fix(spanner, sqlalchemy-spanner): fix reflection crashes, add native UUID support, and improve JsonObject - #17822

Closed
sakthivelmanii wants to merge 1 commit into
mainfrom
fix-sqlalchemy-spanner-reflection-uuid-batch-storing
ConversationCommits (1)ChecksFiles changed
Closed

fix(spanner, sqlalchemy-spanner): fix reflection crashes, add native UUID support, and improve JsonObject#17822
sakthivelmanii wants to merge 1 commit into
mainfrom
fix-sqlalchemy-spanner-reflection-uuid-batch-storing

Conversation

sakthivelmanii commented Jul 22, 2026

Copy link
Copy Markdown
Contributor
  • 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.

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:

  • 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 #<issue_number_goes_here> 🦕

sakthivelmanii requested a review from olavloite July 22, 2026 10:06
sakthivelmanii requested a review from a team as a code owner July 22, 2026 10:06
gemini-code-assist Bot reviewed Jul 22, 2026
View reviewed changes

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread packages/sqlalchemy-spanner/google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py

parthea commented Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Wait for #17880 to be merged first

Done. Please merge main into this branch

parthea marked this pull request as draft July 24, 2026 18:45
sakthivelmanii force-pushed the fix-sqlalchemy-spanner-reflection-uuid-batch-storing branch 5 times, most recently from a16c261 to 2959f26 Compare July 27, 2026 07:35
fix(spanner, sqlalchemy-spanner): fix reflection crashes, add native …
c5bae33 
…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.
sakthivelmanii force-pushed the fix-sqlalchemy-spanner-reflection-uuid-batch-storing branch from 2959f26 to c5bae33 Compare July 27, 2026 07:46
sakthivelmanii marked this pull request as ready for review July 27, 2026 08:01
olavloite reviewed Jul 27, 2026
View reviewed changes

olavloite 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread packages/google-cloud-spanner/tests/unit/test_datatypes.py
expected = json.dumps(data, sort_keys=True, separators=(",", ":"))
data_jsonobject = JsonObject(JsonObject(data))
self.assertEqual(data_jsonobject.serialize(), expected)

olavloite Jul 27, 2026

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can we add a test for a nested JsonObject structure? For example a JsonObject containing an array of JsonObject?

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

olavloite olavloite left review comments
+1 more reviewer
gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

sakthivelmanii

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL