Skip to content

Commit 809ee13

Browse files
committed
backlog: TASK-37 as PR #20 (open); DRAFT-21 array() alias parked; TASK-38 scoped to full fix (option a) per AmirHossein
1 parent 4930f0e commit 809ee13

3 files changed

Lines changed: 77 additions & 7 deletions
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
id: DRAFT-21
3+
title: >-
4+
native: dispatch the array(...) function alias (make_array's twin, left out of
5+
TASK-37)
6+
status: Draft
7+
assignee: []
8+
created_date: '2026-07-24 21:19'
9+
labels:
10+
- native
11+
- parity
12+
- containers
13+
- sql-surface
14+
dependencies: []
15+
references:
16+
- src/expr_build.rs
17+
- 'PR #20'
18+
- TASK-37
19+
documentation:
20+
- doc-1 (DataFusion function catalogue — parity oracle)
21+
priority: low
22+
type: feature
23+
---
24+
25+
## Description
26+
27+
<!-- SECTION:DESCRIPTION:BEGIN -->
28+
WHAT A USER HITS
29+
SELECT array(lat, lon) AS coords FROM __THIS__
30+
DataFusion accepts array(...) as a builtin alias for make_array(...). Native raises 'Unknown function: array' — works on transform(), fails on infer(). Same loud-failure shape as the make_array gap TASK-37 just closed; array() is simply the alias TASK-37 deliberately did not pick up.
31+
32+
WHY IT'S A SEPARATE DRAFT, NOT PART OF TASK-37
33+
During TASK-37 Wren measured array() and found it is NOT free — it needs its own dispatch line in convert_function plus a parity test. Per the standing rule (do not add untested surface for completeness), it was left out of TASK-37 rather than bundled in unmeasured. Trivial-but-not-free, by Wren's own estimate.
34+
35+
WHY LOW / DRAFT
36+
Loud failure (raises, not silent), the make_array spelling and the bracket literal [a, b] both already work as alternatives, and there is no demonstrated demand for the array() spelling specifically. Parked as a draft; promote if demand appears or if a 'native container parity, finish the long tail' pass is scoped.
37+
38+
Fix is small and known: mirror the make_array dispatch TASK-37 added, keyed on the 'array' function name, routing to the same Expr::List construction path. Add a parity test against the oracle.
39+
40+
DRAFT pending AmirHossein's review.
41+
<!-- SECTION:DESCRIPTION:END -->
42+
43+
## Acceptance Criteria
44+
<!-- AC:BEGIN -->
45+
- [ ] #1 native convert_function dispatches array(...) to the same Expr::List path make_array uses
46+
- [ ] #2 A differential parity test for array(a, b) passes on both engines vs the DataFusion oracle
47+
- [ ] #3 Confirm array() and make_array() produce identical results (they are aliases in the oracle)
48+
<!-- AC:END -->

backlog/tasks/task-37 - native-no-dispatch-for-struct...-and-make_array...-construction.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
---
22
id: TASK-37
33
title: 'native: no dispatch for struct(...) and make_array(...) construction'
4-
status: To Do
4+
status: In Progress
55
assignee:
66
- Wren
77
created_date: '2026-07-23 14:30'
8-
updated_date: '2026-07-24 14:36'
8+
updated_date: '2026-07-24 21:19'
99
labels:
1010
- native
1111
- parity
@@ -106,4 +106,12 @@ MY RULINGS on Wren's three questions (within-ticket scope calls, PM's to make):
106106

107107
This is the validate-don't-assume pattern catching an imprecise ticket before code, same as Ritchie's spec corrections. The ticket is better for it.
108108
---
109+
110+
author: Iris (PM)
111+
created: 2026-07-24 21:19
112+
---
113+
Delivered as PR #20 (OPEN, not merged) — NOT closing until merged; ACs ticked against the merged diff then. Scope verified against the PR diff: 2 files (src/expr_build.rs + tests/test_diff_types.py), matches the rewritten ACs. Wren reports all 4 ACs met, 575 passed / 9 xfailed (3 markers flipped). The recon-driven AC rewrite matched the implementation exactly — STRUCT via convert_expr SqlExpr::Struct arm, dead convert_function branch removed, make_array via convert_function dispatch.
114+
115+
Question (b) resolved by measurement: array(...) is NOT free — native raises 'Unknown function: array', so it needs its own dispatch line + parity test. Per my ruling, Wren left it OUT rather than add untested surface, and flagged it in the PR body. Spun out as a draft (see DRAFT for array() alias) — trivial-but-not-free, loud failure, no demonstrated demand, so parked not queued.
116+
---
109117
<!-- COMMENTS: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: 19 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 02:35'
10+
updated_date: '2026-07-24 21:21'
1111
labels:
1212
- native
1313
- parity
@@ -60,10 +60,12 @@ RELATED: TASK-37 (native has no struct(...)/make_array(...) dispatch) and TASK-3
6060

6161
## Acceptance Criteria
6262
<!-- AC:BEGIN -->
63-
- [ ] #1 native folds an unquoted struct-column qualifier to lowercase, matching DataFusion (SELECT S.x resolves against struct column s)
64-
- [ ] #2 A quoted qualifier stays case-exact, consistent with the TASK-28 folding rule for ordinary identifiers
65-
- [ ] #3 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
66-
- [ ] #4 TASK-28 AC#5's ceiling note in expr_build.rs is updated or removed, since the gap it flagged is now closed rather than merely unreachable
63+
- [ ] #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
66+
- [ ] #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
6769
<!-- AC:END -->
6870

6971
## Comments
@@ -87,4 +89,16 @@ created: 2026-07-24 02:35
8789
---
8890
Promoted from draft and assigned to Wren (2026-07-24, AmirHossein's go). QUEUE POSITION 4 of 4. Shares src/expr_build.rs with TASK-37 — do them back-to-back. Note AC#4: this one also requires updating TASK-28's AC#5 ceiling note in expr_build.rs, since the gap it flagged is being closed rather than staying unreachable.
8991
---
92+
93+
author: Iris (PM)
94+
created: 2026-07-24 21:21
95+
---
96+
SCOPE DECISION — AmirHossein ruled OPTION (a): the full fix, not the one-line version the ticket implied (2026-07-24). This materially expands TASK-38 from 'fold the qualifier in expr_build.rs' to a core Expr::Column change carrying quote_style through to plan.rs. ACs rewritten to (a).
97+
98+
WHY the ticket's original framing was wrong (Wren, measured):
99+
- The naive one-line fold (expr_build.rs:44, fold parts[0]) BREAKS 96 TESTS. Our own rewrite emits internally-qualified SQL (SELECT __THIS__.age / __STATE__.avg_age ... FROM __THIS__ LEFT JOIN __STATE__), so folding parts[0] turns __THIS__ -> __this__ and every windowed transform on native dies.
100+
- expr_build CANNOT distinguish a relation qualifier from a struct-column qualifier — that is only known later in plan.rs::validate_expr (resolved.get(t)). Relation -> use as-is (don't fold). Not-a-relation -> struct-field fallback at plan.rs:1077 (should fold). But by then Expr::Column stores table as a bare Option<String> and quote_style is gone. Hence (a): carry the quote info so the struct-column branch can fold unquoted / preserve quoted.
101+
102+
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).
103+
---
90104
<!-- COMMENTS:END -->

0 commit comments

Comments
 (0)