Resolve genomic columns for CLUSTER/MERGE over a derived-table FROM — Closes #164#175
Merged
Merged
Conversation
genomic_columns only read a column mapping when the enclosing FROM was a bare table, so CLUSTER/MERGE over a derived table or CTE silently fell back to the canonical chrom/start/end names — emitting SQL that references columns the source does not expose when the underlying table uses a custom mapping. Replace the bare-table lookup with a resolver that traces the genomic columns through a star-passthrough derived table or CTE to the underlying registered mapping, treats an explicit projection as transparent only when it re-exposes the canonical columns, and keeps the canonical default for an unregistered table or an empty FROM. When the source cannot be resolved — an opaque derived table, or joined sources with conflicting mappings — it now raises a clear ValueError rather than emitting broken SQL. Shared by both expanders through genomic_columns, so the fix covers CLUSTER and MERGE alike. The pre-existing dropped-WITH limitation on a CTE FROM is out of scope and tracked separately. Claude-Session: https://claude.ai/code/session_01KAYsMtN7vYuECZHeeFFzCM
Exercise the CLUSTER/MERGE derived-source resolution through the public transpile API: resolution through star-passthrough derived tables, CTEs, nested and union bodies, qualified stars, and an ancestor-defined CTE; the CTE-shadows-registered-table precedence; partial mappings and custom or defaulted strand columns; the canonical-re-expose and unregistered fallbacks that must not over-raise; and the ValueError paths for opaque sources, VALUES / table-valued functions, recursive CTEs, and conflicting joined mappings. Cover both operators where the resolver is operator-neutral. Claude-Session: https://claude.ai/code/session_01KAYsMtN7vYuECZHeeFFzCM
3d8649b to
c6ab843
Compare
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
Fix
genomic_columnsso CLUSTER and MERGE resolve the correct physical genomic columns when their FROM clause is a derived table (or, for column resolution, a CTE), instead of silently defaulting to the canonicalchrom/start/endnames. For a CTE FROM this PR fixes column resolution only; end-to-end executability is not — the whole-query rewrite still drops the enclosingWITHclause, so a CTE FROM currently emits non-executable SQL. That is a pre-existing limitation tracked separately in #174.Previously the column mapping was read only when the enclosing FROM was a bare
exp.Table; a derived-table or CTE FROM yielded no table name, so the operators fell back to the canonical columns. When the underlying source used a custom mapping, the emitted SQL then partitioned/ordered/aggregated on columns that do not exist. This was pre-existing behavior carried over from the legacyClusterTransformerand was untested (surfaced as advisory A14 in the #162 / #144 round-2 review).The fix replaces the bare-table lookup with a recursive resolver that traces the genomic columns through a star-passthrough derived table or CTE to the underlying registered mapping. Any projection containing a star (
SELECT *,SELECT alias.*, or a star alongside extra columns) passes its sources' columns through; an explicit projection is treated as transparent only when it re-exposes the canonical columns; an unregistered table or an empty FROM keeps the canonical default. When the source genuinely cannot be resolved — an opaque derived table whose columns are neither canonical nor traceable — it raises a clearValueErrorrather than emitting SQL that references non-existent columns. The resolver is reached through the singlegenomic_columnsentry point shared by both expanders, so CLUSTER and MERGE are fixed together.Trade-offs and scope notes:
ValueError. The whole-query rewrite cannot preserve a join (only the driving relation survivestransplant) and bare genomic-column window/grouping references over a join are inherently ambiguous, so the shape fails loud rather than silently dropping the join. The guard fires only on the query being rewritten; a join nested inside a traced-through derived-table or CTE body is preserved and resolves from that body's driving relation.WITHclause (a pre-existing limitation intransplant, orthogonal to column resolution and reproducible with a canonical table). That is deliberately left out of scope and tracked separately in CLUSTER/MERGE over a CTE FROM drops the WITH clause, emitting non-executable SQL #174; the CTE tests here assert only column resolution, not executability.Closes #164
Proposed changes
Recursive FROM-source resolution in
giql.expanders._genomicRewrite
genomic_columnsto delegate to a small resolver:genomic_columnsrejects a top-level JOIN FROM with a clearValueError(only the rewritten query's join would be dropped bytransplant), then delegates to the resolver, raises aValueErrorfor an opaque/unresolvable derived source, and otherwise falls back to the canonical columns._resolve_from_columnsresolves the drivingfrom_relation to a single(mapping, opaque)result. Joins on the rewrite target are rejected upstream; a join inside a traced-through body is ignored, so its column list is taken from the driving relation._resolve_sourceclassifies a source: a CTE (looked up before registered tables and matched case-insensitively, so a CTE shadows a same-named table), a registered table, an unregistered table (assumed canonical), a derived subquery, or an untraceable source (LATERAL / VALUES / table-valued function)._resolve_querydecides whether a derived-table/CTE body passes its columns through: any projection containing a star resolves from the inner sources; an explicit projection is transparent only when it re-exposes the canonicalchrom/start/endnames, otherwise the source is opaque. A set-operation body (UNION/INTERSECT/EXCEPT, viaexp.SetOperation) resolves from its left arm._find_ctewalks the enclosingWITHclauses up the ancestor chain with a case-insensitive name match; aseenset guards against a self- or mutually-recursive CTE._mapping_forreproduces the existing per-table mapping with the default-strand fallback.Test cases
New suite
tests/expanders/test_genomic_columns.py, driving the publictranspileAPI only.TestGenomicColumnResolutionSELECT *derived table on a custom-mapped tableTestGenomicColumnResolutionSELECT *derived table on a custom-mapped tableTestGenomicColumnResolutionalias.*TestGenomicColumnResolutionTestGenomicColumnResolutionTestGenomicColumnResolutionTestGenomicColumnResolutionUNION ALL/INTERSECT/EXCEPT) with a differently-mapped right armTestGenomicColumnResolutionWITHTestGenomicColumnResolutionTestGenomicColumnResolutionTestGenomicColumnResolutionTestGenomicColumnResolutionTestGenomicColumnResolutionTestGenomicColumnResolutionTestGenomicColumnResolutionTestGenomicColumnResolutionErrorsTestGenomicColumnResolutionErrorsTestGenomicColumnResolutionErrorsUNNEST) FROMTestGenomicColumnResolutionErrorsTestGenomicColumnResolutionErrorsTestGenomicColumnResolutionErrorsTestGenomicColumnResolutionErrorsTestGenomicColumnResolutionErrorsalias.*projected over a top-level JOINhttps://claude.ai/code/session_01KAYsMtN7vYuECZHeeFFzCM