Skip to content

feat(sqlalchemy-spanner): export MAX_SIZE constant - #17922

Merged
sakthivelmanii merged 2 commits into
mainfrom
feat-spanner-export-max-size
Jul 30, 2026
Merged

feat(sqlalchemy-spanner): export MAX_SIZE constant#17922
sakthivelmanii merged 2 commits into
mainfrom
feat-spanner-export-max-size

Conversation

@sakthivelmanii

@sakthivelmanii sakthivelmanii commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Promoting _max_size to public MAX_SIZE (and dialect.max_size) allows cross-dialect applications to inspect Spanner's STRING(MAX) limit through the public API. It eliminates private symbol imports while preserving full backward compatibility for existing code.

@sakthivelmanii
sakthivelmanii requested a review from a team as a code owner July 28, 2026 12:19

@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 exposes the MAX_SIZE constant from the sqlalchemy_spanner module, making it publicly accessible and integrating it into the SpannerDialect class. It also adds a unit test to verify the export and the behavior of the int_from_size helper function. There are no review comments, so I have no feedback to provide.

}

_max_size = 2621440
MAX_SIZE = 2621440

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.

nit: should we consider renaming this to MAX_STRING_COLUMN_LENGTH? Or is the MAX_SIZE name a standard in SQLAlchemy? If we keep the name MAX_SIZE, can we add a docstring that explains what this max is for?

@sakthivelmanii
sakthivelmanii enabled auto-merge (squash) July 30, 2026 09:21
@sakthivelmanii
sakthivelmanii merged commit e15c21d into main Jul 30, 2026
41 checks passed
@sakthivelmanii
sakthivelmanii deleted the feat-spanner-export-max-size branch July 30, 2026 12:10
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.

2 participants