Skip to content

TASK-37: dispatch struct(...) and make_array(...) on the native engine - #20

Closed
claude-agent-ahrzb[bot] wants to merge 1 commit into
masterfrom
task-37-container-dispatch
Closed

TASK-37: dispatch struct(...) and make_array(...) on the native engine#20
claude-agent-ahrzb[bot] wants to merge 1 commit into
masterfrom
task-37-container-dispatch

Conversation

@claude-agent-ahrzb

Copy link
Copy Markdown

Native accepted these container-construction forms at fit (DataFusion) but infer() failed on them — valid SQL the library accepted at fit time, breaking only in production.

struct(a, b)         -> ValueError: Unsupported expression: STRUCT(a, b)
struct(a AS x, ...)  -> ValueError: Unsupported expression: STRUCT(...)
make_array(a, b)     -> ValueError: Unknown function: make_array

Two distinct gaps (the ticket framing was refined after recon)

The original ticket said "native has no struct(...) dispatch in convert_function." Measurement showed that's imprecise — convert_function had a struct case, but it was dead: sqlparser parses struct(...) into a first-class SqlExpr::Struct node that never reaches convert_function. Iris rewrote the ACs to match.

AC#1 — SqlExpr::Struct arm in convert_expr

SqlExpr::Struct { values, fields } => {
    if !fields.is_empty() { /* typed STRUCT<...>(...) unsupported */ }
    let out_fields = values.iter().enumerate().map(|(i, v)| match v {
        SqlExpr::Named { expr, name } => Ok((name.value.clone(), convert_expr(expr)?)),
        other => Ok((format!("c{i}"), convert_expr(other)?)),
    }).collect()?;
    Ok(Expr::Struct(out_fields))
}

Positional → c0/c1; named (a AS x, parsed as Expr::Named) → the alias. Mirrors codegen's _convert_struct (plan.py:390) and DataFusion.

AC#2 — removed the dead struct branch in convert_function

Confirmed unreachable (sqlparser routes struct(...) to SqlExpr::Struct); deleted.

AC#3 — make_arrayExpr::List in convert_function

if name == "make_array" {
    return Ok(Expr::List(args));   // same path the bracket literal [a, b] uses
}

Per-engine dispatch by design: native keys off the function name (sqlparser Function), codegen off the AST node (sqlglot exp.Array) — different parsers. The differential harness proves they agree, not that the code shape is shared.

AC#4 — the 3 xfail_on_native markers removed, all pass on both engines.

Scope note — array(...) left out deliberately

array(a, b) also fails on native (Unknown function: array) — a DataFusion alias, the same loud-gap class. It is not the pinned/tested case, and adding it means untested surface, so per the AC guidance it's excluded and flagged as a possible follow-up rather than folded in silently.

Verification

  • Suite 575 passed, 9 xfailed (was 572 / 12 — the 3 flipped tests now pass on both engines).
  • Mutation evidence is intrinsic: all three tests were xfail (failing) on master and pass now — the pre-fix state is the mutation.
  • Diff scoped to src/expr_build.rs + tests/test_diff_types.py. Rust change → maturin develop.

🤖 Generated with Claude Code

… 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>
claude-agent-ahrzb Bot pushed a commit that referenced this pull request Jul 24, 2026
…K-38 scoped to full fix (option a) per AmirHossein
@claude-agent-ahrzb

Copy link
Copy Markdown
Author

Closing as obsolete: the SQLTransform line this targets has been reset (#54). The DataFusion native engine, the codegen backend, and the batch transform() path are removed; what remains is the confit-served fit-and-serve path.

Nothing here is lost — the branch and its commits stay reachable if this work is ever revived.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant