Skip to content

Commit dceb10b

Browse files
committed
backlog: TASK-38 AC#6 discharged -> DRAFT-22 (relation-qualifier inversion, Low, out of scope); AC#3 reworded honest
1 parent 809ee13 commit dceb10b

2 files changed

Lines changed: 82 additions & 5 deletions
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
---
2+
id: DRAFT-22
3+
title: >-
4+
BUG native: relation qualifier is not folded and is INVERTED vs oracle on
5+
quoted forms (public-API-safe)
6+
status: Draft
7+
assignee: []
8+
created_date: '2026-07-24 21:24'
9+
labels:
10+
- native
11+
- parity
12+
- bug
13+
- identifier-folding
14+
- internal-api
15+
dependencies: []
16+
references:
17+
- src/plan.rs
18+
- TASK-38
19+
- TASK-28
20+
documentation:
21+
- doc-1 (DataFusion function catalogue — parity oracle)
22+
priority: low
23+
type: bug
24+
---
25+
26+
## Description
27+
28+
<!-- SECTION:DESCRIPTION:BEGIN -->
29+
Found by Wren while measuring TASK-38's AC#6 (2026-07-24), BEFORE writing code. Measurements are his, direct against both engines — not inferred.
30+
31+
THE DIVERGENCE
32+
native's RELATION-qualifier handling is not merely unfolded — it is INVERTED vs the DataFusion oracle on quoted forms:
33+
34+
ORACLE NATIVE
35+
__this__.age [{'v':7}] Unknown column: __this__ DIVERGE
36+
\"__this__\".age [{'v':7}] Unknown column: __this__ DIVERGE
37+
\"__THIS__\".age No field named \"__THIS__\" [{'v':7}] DIVERGE (inverted)
38+
__THIS__.age [{'v':7}] [{'v':7}] agree
39+
__THIS__.AGE [{'v':7}] [{'v':7}] agree
40+
41+
ROOT CAUSE
42+
DataFusion REGISTERS the relation under a FOLDED name: ctx.from_arrow(name=\"__THIS__\") is stored as __this__. So on the oracle, unquoted __THIS__ folds and hits, quoted-lower \"__this__\" hits, quoted-exact \"__THIS__\" MISSES. Native compares the qualifier raw, giving the exact opposite answer on both quoted forms. Same for a user table `t`: oracle accepts T.age and \"t\".age, rejects \"T\".age; native inverts.
43+
44+
WHY LOW — UNREACHABLE THROUGH THE PUBLIC API (measured via SQLTransform)
45+
SELECT __this__.age FROM __THIS__ -> fit() RAISES ValueError 'Column qualifier __this__ does not reference...' (clean, loud, at build time)
46+
SELECT \"__THIS__\".age FROM __THIS__ -> batch=[1,2,3] infer=[1,2,3], agree
47+
Only a direct InferFn(...) call — an INTERNAL API, not the user surface — reaches the inverted behaviour, and it fails LOUDLY, never a silent wrong value. So user impact today is nil; this is correctness-of-the-engine-internals, not a user-facing parity gap.
48+
49+
WHY SEPARATE FROM TASK-38 (Wren's reasoning, PM-agreed)
50+
1. Different branch: TASK-38 is the STRUCT-COLUMN branch (plan.rs:1077 fallback). This is the RELATION branch (plan.rs relation resolution). Different code, different fix.
51+
2. The fix here is to match DataFusion's REGISTER-TIME folding of relation names — a second core change on top of TASK-38's Expr::Column quote-carrying, touching how plan.rs resolves relations.
52+
3. Standing rule: one core change at a time. TASK-38 is already a core Expr::Column shape change; do not stack a plan.rs relation-resolution change on the same PR.
53+
54+
WHEN PICKED UP: pin with an xfail-strict differential test first (the standing native-bug process), then fix. Not pinned yet — discovered by ad-hoc measurement during TASK-38 recon, deliberately not added to TASK-38's branch to keep concerns separate.
55+
56+
DRAFT / Low pending AmirHossein's review. Promote if the internal-API inversion starts to matter (e.g. if InferFn's qualifier handling gets exposed, or a user-facing path starts reaching it).
57+
<!-- SECTION:DESCRIPTION:END -->
58+
59+
## Acceptance Criteria
60+
<!-- AC:BEGIN -->
61+
- [ ] #1 native folds relation qualifiers to match DataFusion's register-time folding: unquoted T -> t hits, quoted-lower "t" hits, quoted-exact "T" misses (mirroring the oracle's stored folded name)
62+
- [ ] #2 The quoted-form inversion is gone: "__THIS__".age and "__this__".age give the same answer on native as on the oracle
63+
- [ ] #3 A differential test pins relation-qualifier folding across unquoted/quoted-lower/quoted-exact for both a generated relation (__THIS__) and a user table, passing on both engines
64+
- [ ] #4 No regression to the struct-column-branch folding delivered in TASK-38, and the 96-test windowed-transform suite stays green
65+
<!-- AC:END -->

backlog/tasks/task-38 - BUG-native-unquoted-struct-column-qualifier-is-not-folded-S.x-—-TASK-28s-flagged-ceiling-now-reachable.md

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ status: To Do
77
assignee:
88
- Wren
99
created_date: '2026-07-24 00:36'
10-
updated_date: '2026-07-24 21:21'
10+
updated_date: '2026-07-24 21:25'
1111
labels:
1212
- native
1313
- parity
@@ -61,11 +61,11 @@ RELATED: TASK-37 (native has no struct(...)/make_array(...) dispatch) and TASK-3
6161
## Acceptance Criteria
6262
<!-- AC:BEGIN -->
6363
- [ ] #1 Expr::Column carries the qualifier's quote information end-to-end (change table: Option<String> to a quote-preserving representation, or add a table_quoted flag) through expr_build -> plan, so quote_style is no longer discarded before the struct-column branch can use it
64-
- [ ] #2 At the struct-column field-access fallback (plan.rs:1077), an UNQUOTED qualifier folds to lowercase (S -> s) while a QUOTED qualifier stays case-exact ("S" stays S) — matching DataFusion and consistent with TASK-28's unquoted-folding rule for ordinary columns. This is the AC to get exactly right: fold unquoted, never fold quoted
65-
- [ ] #3 Relation qualifiers (__THIS__, __STATE__, generated join relations) are UNAFFECTED — they must NOT fold. Full suite stays green: the naive one-line fold broke 96 tests by folding __THIS__ -> __this__; the fix must fold ONLY on the struct-column branch, never the relation branch
64+
- [ ] #2 At the struct-column field-access fallback (plan.rs:1077), an UNQUOTED qualifier folds to lowercase (S -> s) while a QUOTED qualifier stays case-exact ("S" stays S) — matching DataFusion and consistent with TASK-28. The AC to get exactly right: fold unquoted, never fold quoted
65+
- [ ] #3 Relation qualifiers (__THIS__/__STATE__/generated joins) UNAFFECTED BY THIS CHANGE — must NOT fold here, full suite stays green (the naive fold broke 96 tests). This AC is about not BREAKING the relation branch; its OWN separate oracle inversion is out of scope, tracked in DRAFT-22
6666
- [ ] #4 The xfail_on_native marker on tests/test_diff_types.py::test_uppercase_qualifier_field_access is removed and the test passes on both engines vs the DataFusion oracle, in the same commit as the fix
67-
- [ ] #5 TASK-28 AC#5's ceiling note in expr_build.rs is updated or removed — the gap it flagged 'unreachable' is now closed, so leaving a stale unreachable comment would misdescribe reality
68-
- [ ] #6 MEASURE the sibling case: does the same quote-loss affect a CamelCase TABLE/relation alias (a user-supplied qualifier that IS a relation)? If reachable and divergent from the oracle, fix it in scope or file a follow-up with the measurement — do not leave it unmeasured now that quote-carrying is being built
67+
- [ ] #5 TASK-28 AC#5's ceiling note in expr_build.rs is updated or removed — the gap it flagged 'unreachable' is now closed, so a stale unreachable comment would misdescribe reality
68+
- [ ] #6 AC#6 (measure the sibling relation-alias case) DISCHARGED: measured during recon, found a separate relation-branch inversion, filed as DRAFT-22. Nothing further required in TASK-38
6969
<!-- AC:END -->
7070

7171
## Comments
@@ -101,4 +101,16 @@ WHY the ticket's original framing was wrong (Wren, measured):
101101

102102
This is why the ticket said 'unreachable' at TASK-28 time and why the honest fix is bigger. AmirHossein chose correctness over the stopgap. Added AC#6: measure whether a CamelCase TABLE alias hits the same quote-loss (Wren flagged it unmeasured) — fix in scope or follow-up, don't leave it hanging. Priority stays Medium (user-facing severity unchanged; scope grew, importance didn't).
103103
---
104+
105+
author: Iris (PM)
106+
created: 2026-07-24 21:25
107+
---
108+
AC#6 DISCHARGED by measurement (Wren, 2026-07-24, before writing code — exactly the instruction). It found MORE than the CamelCase-alias case: native's relation-qualifier handling is INVERTED vs the oracle on quoted forms (DataFusion registers the relation under a folded name, so quoted-exact \"__THIS__\" MISSES on the oracle while native accepts it; quoted-lower is the reverse). Full measurements in DRAFT-22.
109+
110+
SCOPE CALL (mine, Wren-agreed): the relation-qualifier inversion stays OUT of TASK-38 and is filed as DRAFT-22 (Low). Reasons: (1) it's the RELATION branch, TASK-38 is the STRUCT-COLUMN branch — different code, different fix; (2) fixing it means matching DataFusion's register-time relation folding, a second core change in plan.rs relation resolution on top of TASK-38's Expr::Column shape change; (3) one core change per PR. And it's LOW: measured unreachable through the public API — SQLTransform.fit raises loudly or agrees; only a direct internal InferFn call hits the inversion, and always loudly, never a silent wrong value.
111+
112+
AC#3 REWORDED for honesty (Wren's catch): the old wording implied the relation branch is currently CORRECT. It is not — it's inverted. AC#3 now says relation qualifiers are unaffected BY THIS CHANGE (don't break them, per the 96-test finding) and points to DRAFT-22 for their own divergence. Two different claims that the AC had conflated.
113+
114+
Wren is proceeding with TASK-38's brainstorm+plan for the struct-column branch only — unaffected by this call either way, so no reason to make him wait.
115+
---
104116
<!-- COMMENTS:END -->

0 commit comments

Comments
 (0)