refactor(DTL-1780): consolidate aliasing under ALIAS — drop COLUMN/COUNT field_alias#2699
Open
m1n0 wants to merge 2 commits into
Open
refactor(DTL-1780): consolidate aliasing under ALIAS — drop COLUMN/COUNT field_alias#2699m1n0 wants to merge 2 commits into
m1n0 wants to merge 2 commits into
Conversation
…rop COLUMN/COUNT field_alias Three aliasing mechanisms existed in the SQL AST: COLUMN.field_alias, COUNT.field_alias, and the general ALIAS wrapper introduced in PR #2695. ALIAS is strictly more powerful (wraps any SqlExpression); the per-class attributes only worked for COLUMN/COUNT and existed for historical reasons. Changes: - Drop COLUMN.field_alias and COUNT.field_alias dataclass attributes. - Drop the corresponding dispatcher branches in sql_dialect (base + athena override). - COLUMN.AS(alias) now returns ALIAS(self, alias) instead of a new COLUMN. - COLUMN.IN(table_alias) no longer propagates field_alias. - STAR.convert_to_standard_column drops the field_alias parameter (no caller passes it). - Migrate call sites in soda-core, soda-postgres, soda-redshift, soda-athena to ALIAS wrappers. Soda-extensions migrated in a matching branch. No SQL output changes — metadata queries produce identical strings. [REPLAY=OFF] because PR #2695's alias-emission already invalidated the cached snapshots; nightly recording job will refresh after merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
m1n0
force-pushed
the
DTL-1780-consolidate-aliasing-under-alias
branch
from
0e035e0 to
afc42b1
Compare
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy, per-node aliasing attributes (COLUMN.field_alias, COUNT.field_alias) from the SQL AST and standardizes all SQL output-column aliasing through the existing ALIAS(expression, alias) wrapper (including via COLUMN.AS(...)).
Changes:
- Removed
field_aliasattributes fromCOLUMNandCOUNT, and removed corresponding SQL compilation branches in the base dialect and Athena override. - Updated
COLUMN.AS(alias)to returnALIAS(self, alias)and adjusted related helpers (COLUMN.IN,STAR.convert_to_standard_column). - Migrated metadata-query call sites (Postgres/Redshift/core metadata tables queries) to emit aliases via
ALIAS(...)/COLUMN(...).AS(...).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| soda-redshift/src/soda_redshift/statements/redshift_metadata_tables_query.py | Migrates Redshift metadata SELECT-list aliasing from field_alias to COLUMN(...).AS(...). |
| soda-postgres/src/soda_postgres/statements/postgres_metadata_tables_query.py | Uses ALIAS(...) / COLUMN(...).AS(...) for Postgres table-metadata output column names. |
| soda-postgres/src/soda_postgres/common/data_sources/postgres_data_source.py | Updates Postgres columns-metadata query to use ALIAS(...) / COLUMN(...).AS(...). |
| soda-core/src/soda_core/common/statements/metadata_tables_query.py | Replaces fallback aliasing to use ALIAS(LITERAL(None), ...) instead of COLUMN(..., field_alias=...). |
| soda-core/src/soda_core/common/sql_dialect.py | Removes field_alias emission from COLUMN and COUNT compilation; keeps canonical ALIAS rendering. |
| soda-core/src/soda_core/common/sql_ast.py | Removes field_alias fields, updates COLUMN.AS/COLUMN.IN, and adjusts STAR.convert_to_standard_column signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…/ field_alias removal
PR-2699 had zero test additions — the consolidation was relying on existing
SQL-generation tests passing. That's not enough: a regression to the
deprecated `field_alias` mechanism would slip through because callers using
`COLUMN.AS("x")` still produce SQL with the alias either way.
Six new unit tests in test_sql_generation.py:
- ALIAS renders `<expr> AS "<alias>"` with default quoting.
- ALIAS wraps arbitrary expressions (LITERAL, COUNT) — the key gain over
the per-class field_alias mechanism that only worked on COLUMN/COUNT.
- COLUMN.AS returns an ALIAS instance (not a mutated COLUMN). This is the
one a reviewer can't infer from rendered SQL alone — the type matters
because some call sites read `.alias` from the returned object.
- COLUMN and COUNT constructors reject `field_alias` kwarg via TypeError.
Pins the contract; a re-introduction (even silent) of that kwarg would
fail these tests immediately.
- ALIAS inside SELECT emits the AS clause in the final rendered SQL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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
The codebase had three aliasing mechanisms in the SQL AST:
COLUMN.field_alias(per-class attribute)COUNT.field_alias(per-class attribute)ALIAS(expression, alias)(general wrapper, introduced in PR fix: alias aggregation query columns and guard derived metrics #2695)ALIASis strictly more powerful — wraps anySqlExpression. The per-class attrs only worked forCOLUMN/COUNTand existed for historical reasons. This PR removes them in favor ofALIAS.Tracking: DTL-1780.
Changes
COLUMN.field_aliasandCOUNT.field_aliasdataclass attributes.sql_dialect.py(base + athena override).COLUMN.AS(alias)returnsALIAS(self, alias)instead of a newCOLUMN.COLUMN.IN()no longer propagatesfield_alias.STAR.convert_to_standard_columndrops the unusedfield_aliasparameter.Why now
Test plan
Related
ALIAS: PR fix: alias aggregation query columns and guard derived metrics #2695🤖 Generated with Claude Code