Skip to content

Commit 795e3e9

Browse files
ahrzbclaude
andcommitted
fix(codegen): fold struct-column qualifier; +equality/fold coverage (Tasks 3-4 review)
Review of Tasks 3-4 flagged an unquoted uppercase struct-column qualifier not being folded: S.x built a lookup for column 'S' while DataFusion folds it to 's'. Fold the qualifier when the validate rewrite reinterprets it as a column (safe: the relation lookup already matched it raw and missed). Locked with an xfail_on_native diff test -- native has the same (worse) gap, wants a ticket. Also: comment the deliberate drop of the plan's 4-part catalog raise (lets t.s.a.b resolve via layered FieldAccess), and add list-inequality coverage. Left unfixed (noted for tickets): struct=scalar returns False where DataFusion type-errors; field access on a NULL struct (DataFusion returns 0, not NULL) -- both out of Phase B's committed surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 7a502a9 commit 795e3e9

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

sql_transform/_codegen/plan.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,10 @@ def _convert_expr(e: exp.Expression) -> Any:
317317
# [s,a,b] -> FieldAccess(Column(s,a), b); the 2-part `s.x` stays a Column
318318
# (table.column vs struct.field is ambiguous until schemas resolve, in
319319
# _validate_expr). The qualifier (parts[0]) names a relation and stays
320-
# raw; column/field parts fold.
320+
# raw; column/field parts fold. No `catalog` raise: deliberately dropped
321+
# from the plan so `t.s.a.b` (table + nested struct) resolves via layered
322+
# FieldAccess rather than erroring -- only a true 4-part catalog.db.tbl.col
323+
# (never emitted in __THIS__-scoped SQL) would misparse.
321324
quals = [q for q in (e.args.get("catalog"), e.args.get("db")) if q is not None]
322325
if e.args.get("table"):
323326
quals.append(e.args["table"])
@@ -747,7 +750,12 @@ def _validate_expr(e: Any, resolved, row_schemas, static_schemas, used) -> Any:
747750
# and re-validate (mirrors native plan.rs). A relation alias
748751
# always wins, so this only runs after the alias lookup fails;
749752
# the base column resolves below, erroring if it isn't real.
750-
fa = FieldAccess(Column(table=None, name=e.table), e.name)
753+
# Fold the qualifier as a column here (the relation lookup above
754+
# matched it raw): an unquoted `MyStruct.x` base folds to
755+
# `mystruct` like any column, matching DataFusion. (Quotedness is
756+
# lost by this point, so a quoted mixed-case struct base is
757+
# unsupported -- exotic and untested.)
758+
fa = FieldAccess(Column(table=None, name=e.table.lower()), e.name)
751759
return _validate_expr(fa, resolved, row_schemas, static_schemas, used)
752760
real, is_row = entry
753761
schema = (row_schemas if is_row else static_schemas).get(real)

tests/test_diff_types.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,17 @@ def test_qualified_struct_field_access():
148148
)
149149

150150

151+
def test_uppercase_qualifier_field_access(xfail_on_native):
152+
# An unquoted struct-column qualifier folds like any column: `S.x` -> `s.x`,
153+
# matching DataFusion. Native doesn't fold the qualifier (raises "Unknown
154+
# column: S") -- parity gap, wants its own ticket.
155+
xfail_on_native("native does not fold an unquoted struct-column qualifier (S.x)")
156+
check(
157+
"SELECT S.x AS v FROM t",
158+
{"t": rows({"s": "struct{x:int}"}, [{"s": {"x": 7}}])},
159+
)
160+
161+
151162
def test_struct_equality_true():
152163
check(
153164
"SELECT (s = s) AS eq FROM t",
@@ -170,6 +181,13 @@ def test_list_equality():
170181
)
171182

172183

184+
def test_list_inequality():
185+
check(
186+
"SELECT (l = [9, 9, 9]) AS eq FROM t",
187+
{"t": rows({"l": "list[int]"}, [{"l": [1, 2, 3]}])},
188+
)
189+
190+
173191
def test_unnest_struct_expands_columns():
174192
check(
175193
"SELECT unnest(named_struct('x', a, 'y', b)) FROM t",

0 commit comments

Comments
 (0)