Skip to content

Commit 6c7ee3d

Browse files
ahrzbclaude
andcommitted
feat(native): dispatch struct(...) and make_array(...) construction — TASK-37
native accepted these at fit (DataFusion) but infer() failed: struct(...) raised 'Unsupported expression' and make_array(...) 'Unknown function', on valid SQL the library accepted at fit time. Two distinct gaps, not one: - struct(a, b) / struct(a AS x, ...) parse into a first-class SqlExpr::Struct node (NOT a Function), so a new convert_expr arm handles it: positional -> c0/c1/..., named (Expr::Named) -> the alias, mirroring codegen's _convert_struct and DataFusion. The typed STRUCT<...>(...) form is rejected explicitly. - The struct branch in convert_function was dead (sqlparser never routes struct(...) there) and is removed. - make_array(...) IS a function; it now routes to Expr::List, the same path the bracket literal [a, b] already uses. The three xfail_on_native markers are removed; all three pass on both engines. array(...) (a DataFusion alias that also fails on native) is deliberately left out -- not the pinned case, and adding it would be untested surface. Flagged for a possible follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent a83b742 commit 6c7ee3d

2 files changed

Lines changed: 48 additions & 17 deletions

File tree

src/expr_build.rs

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,34 @@ pub fn convert_expr(e: &SqlExpr) -> Result<Expr, InterpError> {
8080
SqlExpr::Array(Array { elem, .. }) => Ok(Expr::List(
8181
elem.iter().map(convert_expr).collect::<Result<_, _>>()?,
8282
)),
83+
// Authored struct(...) construction. sqlparser parses `struct(...)` into a
84+
// first-class SqlExpr::Struct node (NOT a Function), so it never reaches
85+
// convert_function -- which is why this must live here. `values` holds the
86+
// elements: a bare expr is positional (field name c0, c1, ...), an
87+
// `expr AS name` is Expr::Named (explicit field name). `fields` is only
88+
// populated by the typed `STRUCT<a INT>(...)` form, which we don't support.
89+
// Field naming mirrors codegen's _convert_struct and DataFusion:
90+
// positional -> c{i}, named -> the alias as written.
91+
SqlExpr::Struct { values, fields } => {
92+
if !fields.is_empty() {
93+
return Err(InterpError::Build(
94+
"typed STRUCT<...>(...) syntax is not supported; \
95+
use struct(a, b) or struct(a AS x, ...)"
96+
.to_string(),
97+
));
98+
}
99+
let out_fields = values
100+
.iter()
101+
.enumerate()
102+
.map(|(i, v)| match v {
103+
SqlExpr::Named { expr, name } => {
104+
Ok((name.value.clone(), convert_expr(expr)?))
105+
}
106+
other => Ok((format!("c{i}"), convert_expr(other)?)),
107+
})
108+
.collect::<Result<Vec<_>, InterpError>>()?;
109+
Ok(Expr::Struct(out_fields))
110+
}
83111
SqlExpr::Cast {
84112
expr, data_type, ..
85113
} => Ok(Expr::Cast {
@@ -194,14 +222,16 @@ fn convert_function(func: &Function) -> Result<Expr, InterpError> {
194222
}
195223
return Ok(Expr::Struct(fields));
196224
}
197-
if name == "struct" {
198-
let fields = args
199-
.into_iter()
200-
.enumerate()
201-
.map(|(i, e)| (format!("c{i}"), e))
202-
.collect();
203-
return Ok(Expr::Struct(fields));
225+
// make_array(a, b) is a DataFusion builtin that constructs a list, same as
226+
// the bracket literal [a, b] (which parses as SqlExpr::Array). Route it to the
227+
// same Expr::List path. The `array(...)` spelling parses as SqlExpr::Array, not
228+
// a Function, so it is already handled in convert_expr and not needed here.
229+
if name == "make_array" {
230+
return Ok(Expr::List(args));
204231
}
232+
// NB: `struct(...)` does NOT reach here -- sqlparser parses it into a
233+
// first-class SqlExpr::Struct node, handled in convert_expr. (An earlier
234+
// `struct` case here was dead code and was removed.)
205235
Ok(Expr::Function { name, args })
206236
}
207237

tests/test_diff_types.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,30 +26,31 @@ def test_list_construct():
2626
# native has no dispatch for them at all -- so they xfail_on_native, the same
2727
# parity-gap pattern as test_list_construct_mixed_numeric_widens. Codegen
2828
# correctness is the assertion; the native gaps want their own tickets.
29-
def test_struct_construct_positional(xfail_on_native):
30-
# struct(a, b) names fields positionally c0, c1 (matches DataFusion).
31-
xfail_on_native("native has no struct(...) construction dispatch (expr_build.rs)")
29+
def test_struct_construct_positional():
30+
# struct(a, b) names fields positionally c0, c1 (matches DataFusion). Native
31+
# handles it via a SqlExpr::Struct arm in convert_expr (TASK-37); struct(...)
32+
# parses as a first-class node, not a function.
3233
check(
3334
"SELECT struct(a, b) AS s FROM t",
3435
{"t": rows({"a": "int", "b": "int"}, [{"a": 1, "b": 2}])},
3536
expect=[{"s": {"c0": 1, "c1": 2}}],
3637
)
3738

3839

39-
def test_struct_construct_named(xfail_on_native):
40-
# struct(a AS x, b AS y) parses as exp.PropertyEQ -> explicit field names.
41-
xfail_on_native("native has no struct(...) construction dispatch (expr_build.rs)")
40+
def test_struct_construct_named():
41+
# struct(a AS x, b AS y) -> explicit field names (sqlparser Expr::Named,
42+
# sqlglot exp.PropertyEQ). Native handles it in convert_expr (TASK-37).
4243
check(
4344
"SELECT struct(a AS x, b AS y) AS s FROM t",
4445
{"t": rows({"a": "int", "b": "int"}, [{"a": 1, "b": 2}])},
4546
expect=[{"s": {"x": 1, "y": 2}}],
4647
)
4748

4849

49-
def test_make_array_construct(xfail_on_native):
50-
# make_array(a, b) is a DataFusion builtin; native doesn't recognize it as a
51-
# function (only the bracket literal [a, b] reaches native's list path).
52-
xfail_on_native("native has no make_array function dispatch (expr_build.rs)")
50+
def test_make_array_construct():
51+
# make_array(a, b) is a DataFusion builtin. Native routes it to Expr::List in
52+
# convert_function -- the same construction path the bracket literal [a, b]
53+
# already uses (TASK-37).
5354
check(
5455
"SELECT make_array(a, b) AS l FROM t",
5556
{"t": rows({"a": "int", "b": "int"}, [{"a": 1, "b": 2}])},

0 commit comments

Comments
 (0)