[MAINTENANCE] Add integration coverage for quoted schema names on PostgreSQL - #12051
Draft
joshua-stauffer wants to merge 1 commit into
Draft
[MAINTENANCE] Add integration coverage for quoted schema names on PostgreSQL#12051joshua-stauffer wants to merge 1 commit into
joshua-stauffer wants to merge 1 commit into
Conversation
Wrapping an identifier in the dialect's quote characters is how a user asks GX to use it verbatim rather than case-folding it. `table_name` honors that: a quoted value is unwrapped and rebuilt as a quoted SQLAlchemy identifier, so the quotes govern the emitted SQL instead of becoming part of the name. A schema is not given the same treatment -- the quote characters stay inside the string, and the emitted SQL targets a schema whose name literally contains quotes. Pin the behavior down with two PostgreSQL tests that differ only in whether the schema is quoted. The bare case passes, establishing that naming a schema on the asset works; the quoted case is marked xfail(strict=True), so it reports as expected-fail today and turns into a hard failure the moment the identifier handling is corrected. The shared PostgreSQL setup targets its schema through the connection string's search_path, which never passes through GX's identifier handling, so these tests name the schema on the asset instead.
✅ Deploy Preview for niobium-lead-7998 canceled.
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wrapping an identifier in the dialect's quote characters is how a user asks GX to use that identifier verbatim rather than case-folding it.
table_namehonors that convention: a quoted value is unwrapped and rebuilt as asqlalchemy.quoted_name(..., quote=True), so the quotes govern the emitted SQL instead of becoming part of the name.A schema name never gets the same treatment.
_effective_schema_namereturns the string with the quote characters still embedded, so SQLAlchemy escapes them as data rather than honoring them as quoting:The practical consequence is that a case-sensitive schema is unreachable: quoting is the only way to preserve its case, and quoting is what breaks. The same mismatch also makes the schema-existence check in
TableAsset.test_connectioncompare a quoted name against a bare schema listing, so it reports a schema that does exist as missing.This PR adds coverage only — no behavior change.
What's here
Two PostgreSQL integration tests in
tests/integration/data_sources_and_expectations/test_quoted_schema_names.pythat differ in exactly one variable: whether the schema is handed to the asset bare or bracketed by quotes.test_bare_schema_name_reaches_the_table— passes. A control, so the companion test's failure can only be attributable to the quoting rather than to anything about naming a schema on the asset.test_quoted_schema_name_reaches_the_table—xfail(strict=True). Documents the current behavior and converts into a hard failure the moment the identifier handling is corrected, so the fix cannot land without this test being updated deliberately.The shared PostgreSQL setup targets its schema through the connection string's
search_path, which never passes through GX's identifier handling, so these tests name the schema on the asset instead. That is also why PostgreSQL is the backend here: it runs locally, and the schema is created and dropped by the existing harness.Verification
Against a local PostgreSQL container. With
--runxfail, the xfailed test fails as described above:ruff check,ruff format --check, and the repo's CI type-check configuration are all clean.Notes for review
schema_name, which is deprecated but still supported. Backends that carry a schema in structured connection details (Snowflake, SQL Server) reach the same_effective_schema_namethrough their non-deprecatedschema_property, so the defect is not confined to the deprecated argument._effective_schema_name— the single point both routes pass through — mirroring what thetable_namevalidator already does, including thedict()round-trip that re-adds the quote characters so stored configs keep their shape.