fix(unparser): keep a join that is another join's right input on that join's right (refs spiceai/spiceai#14373) by grokspice · Pull Request #233 · spiceai/datafusion

grokspice · 2026-09-25T16:40:26Z

Summary

A join that is the right input of another join was linearised into the enclosing FROM in the wrong order. The nested Join arm walks its own left side into the relation the enclosing join handed down (its right_relation), but attaches its own right side with select.pop_from() … push_join … push_from, which pops the shared SelectBuilder's last FROM entry — the enclosing join's left relation. The enclosing join then appends JOIN b after it, so a ⋈ (b ⋈ c) came out as FROM a INNER JOIN c ON b.id = c.id FULL JOIN b ON a.id = b.id, with b referenced before it is in scope. PostgreSQL and DuckDB reject it; an engine that binds lazily runs a different join tree (spiceai/spiceai#14373).

Changes

SelectBuilder counts the join right-inputs the walk is inside (enter_right_join_input / leave_right_join_input / in_right_join_input). The Join arm reads it at entry: a join reached inside some join's right input is a joined table of its own, not a further FROM entry.
Such a join builds TableFactor::NestedJoin { (left ⋈ right) } into the relation it was handed (RelationBuilder::nested_join, a new TableFactorBuilder::NestedJoin variant that keeps an optional alias), instead of appending to the shared FROM. Top-level and left-deep joins keep the linear form, so no existing snapshot changes.
push_source_idents collects the sources inside a nested joined table (recursively) for the dangling-qualifier check, which otherwise stripped b. and c. from the select list because it saw neither table in FROM.
A nested right input's scan predicates no longer reach the shared WHERE when the outer join null-extends that input (Copilot's finding): for a LEFT JOIN they fold into the outer ON, mirroring the existing left-input hoist under a RIGHT JOIN; under a FULL JOIN — on either input — fix(unparser): keep a FULL JOIN input's scan filters scoped when the input is a join (refs spiceai/spiceai#12593) #231's scoping keeps the filtered scan in a derived table of its own, and neither hoist runs below a FULL JOIN's input; under a RIGHT JOIN WHERE is right.
Both hoists leave the predicate above in place while an input is walked and split off what the walk added by position (selection_conjunct_count / take_selection_added_after), so a mark join in that input can still rewrite the mark the predicate reads (the adversarial review's finding on an earlier head).
Refused, each pinned by a test: a contributed conjunct holding a subquery under a LEFT JOIN (right input) or a RIGHT JOIN (left input) (some dialects refuse it in ON, and WHERE is not this join's to use); a mark join on an input an outer join null-extends (NOT c.mark above (b MARK c) RIGHT JOIN a would keep rows the plan drops, since EXISTS is never NULL — a mark join on a preserved input still rewrites in place); an outer join's subquery conjunct scoped onto a joined input, on either side (it would be wrapped anonymously, hiding the names the enclosing query still uses).
An aliased join that is another join's right input — also behind a filter and a further alias — is derived as its own scope ((SELECT … FROM b INNER JOIN c … WHERE (b.id = 'x')) AS j), so its filters stay inside and the alias is what the enclosing query addresses (Copilot's finding, 18461e7d5).
Test right_nested_join_keeps_its_shape_on_the_right: a ⋈ (b ⋈ c) for Inner (with a scan filter on b, which stays in WHERE), Full (unfiltered), the filtered nested input under Left (folded into ON), Right (WHERE) and Full (scan derived, at the top and nested), the subquery conjunct (refused), the mark join on null-extended inputs (refused) and on preserved inputs (rewritten in place), the scoped subquery on a joined input (refused), an aliased nested join (derived under its alias), and an aliased renaming nested join under SQLite (names injected into the derived table, since SQLite refuses AS j (x, y)).

Reproduction

cargo test -p datafusion-sql --test sql_integration -- right_nested_join_keeps_its_shape, with the test's plan a INNER JOIN (b[b.id = 'x'] INNER JOIN c).

Before (unparser from origin/spiceai-54, test from this branch):

-SELECT a.id, b.id, c.id FROM a INNER JOIN (b INNER JOIN c ON b.id = c.id) ON a.id = b.id WHERE (b.id = 'x')
+SELECT a.id, b.id, c.id FROM a INNER JOIN c ON b.id = c.id INNER JOIN b ON a.id = b.id WHERE (b.id = 'x')
test cases::plan_to_sql::right_nested_join_keeps_its_shape_on_the_right ... FAILED

After (this branch):

test cases::plan_to_sql::right_nested_join_keeps_its_shape_on_the_right ... ok

An intermediate state, with the FROM fixed but the source collection not yet taught about nested joins, showed why the third change is needed:

+SELECT a.id, id, id FROM a INNER JOIN (b INNER JOIN c ON b.id = c.id) ON a.id = b.id WHERE (b.id = 'x')

Test plan

At head 9261744df (merged with spiceai-54 4cf679211):

cargo test -p datafusion-sql --test sql_integration -- right_nested_join_keeps_its_shape: 1 passed. The same test fails on origin/spiceai-54 with the linearised FROM pasted above.
cargo test -p datafusion-sql --test sql_integration (whole suite): 745 passed, 22 failed. The 22 are the same names that fail on spiceai-54 alone (pre-existing snapshot drift, for example roundtrip_statement).
cargo test -p datafusion-sql --lib: 90 passed.
cargo clippy -p datafusion-sql --all-targets --all-features --no-deps -- -D warnings: clean.
The three FULL JOIN snapshots the merge with fix(unparser): keep a FULL JOIN input's scan filters scoped when the input is a join (refs spiceai/spiceai#12593) #231 produced return the plan's rows on DuckDB and PostgreSQL (EXCEPT ALL against a CTE formulation is empty in both directions); the SQLite snapshot runs on sqlite3, where the pre-fix SQL is a syntax error.

Review gate

Re-stated for head be489c24 — the put this branch's three new walks under recursive_protection commit, which answers Copilot's two recursion-guard threads. These
passes attest that commit; the record for the earlier heads is kept underneath.

Adversarial review: ran — codex, base origin/spiceai-54, exit=0,
Verdict: needs-attention. (Credits were exhausted on the previous heads and are back.)
It raised nothing against the guards themselves and one medium finding against this
branch's earlier commit: the aliased-join early return at plan.rs:2586-2594 emits a
column list on the table alias without the supports_column_alias_in_table_alias test
that both paths below it apply, so a dialect that refuses AS j(x) (SQLite, BigQuery,
Snowflake) would get unsupported SQL. Not fixed here and not reproduced — the
structural claim holds on a read, but reaching the branch needs
in_right_join_input() + is_joined_relation() + a renaming alias simultaneously, and I
have not built such a plan. Carried in full, with its evidence class, to
fix(unparser): keep a join that is another join's right input on that join's right (refs spiceai/spiceai#14373) #233 (comment) so it survives
this head.
/security-review: no findings, scoped by hand to be489c24. (The skill's own
scoping resolved to the whole spiceai-54 base delta rather than this commit.) The
commit adds three #[cfg_attr(feature = "recursive_protection", recursive::recursive)]
attributes and nothing else — no new I/O, parsing, deserialization, unsafe, or
dependency; it grows a stack under an existing feature flag.
/simplify: no change — three attribute lines, already the crate's existing form.

Earlier heads

Adversarial review: skipped — codex exited 1 on this head (a1e011d) and on b4d3ce7, 001e21e, 18461e7, 1baae1a: "Your workspace is out of credits. Ask your workspace owner to refill in order to continue."; grok is not a permitted engine for a Grok-authored diff. Three earlier heads were reviewed: 5226f6a — job review-muh7jgyl-sb1lrv, needs-attention, 1 finding (a filtered join nested on a FULL JOIN's left let its predicate escape into the outer ON), fixed in 7ac9c4f by watching both FULL JOIN inputs; 7ac9c4f — job review-muh7vni6-yur484, needs-attention, 1 finding (with the predicate above kept visible, a mark join on a null-extended input had its NULL mark replaced by a never-NULL EXISTS, so NOT c.mark kept rows the plan drops), fixed in 1baae1a by refusing a mark join there; 2fd92b0 — exited 1 out of credits at the time. Copilot's seven threads (nested right input predicates under LEFT JOIN, subquery conjuncts folded into ON, the left hoist hiding the predicate from a mark join, the scoped subquery on a joined input, the aliased nested join, a projected joined input, a filtered aliased join) are each fixed and resolved with the measurement in the thread.
/security-review: no findings on the initial diff — the right-input counter is entered and left around the one recursive call with the result captured first, so every ? on that path leaves it balanced; the new failure paths return internal_err!/not_impl_err!; the nested-join traversal is bounded by the plan's own join depth. The later commits add refusals and a positional split of the WHERE conjuncts; no new I/O, parsing or logging path.
/simplify: applied — the local flag renamed to is_right_join_input to mirror in_right_join_input(), and the builder's method spacing matched; the suggestion to wrap the NestedJoin variant in a dedicated builder struct like Derived/Unnest was declined (it carries two fields and no build-time options, so a struct would add surface without a use). Light checks re-run: the regression test and clippy -p datafusion-sql --all-targets --no-deps are clean on the amended head.

Refs spiceai/spiceai#14373 (closes once spiceai/spiceai moves its datafusion pin past this commit and adds the fork_patches.md row)

… join's right (refs spiceai/spiceai#14373) The nested `Join` arm walked its left side into the relation the enclosing join handed down, then attached its right side by popping the shared `SelectBuilder`'s last `FROM` entry — the enclosing join's left relation — so `a ⋈ (b ⋈ c)` came out as `FROM a INNER JOIN c ON b.id = c.id FULL JOIN b ON a.id = b.id`, with `b` referenced before it is in scope. A join reached inside some join's right input now builds a parenthesised joined table into the relation it was handed, and the dangling-qualifier check learns the sources inside it. Top-level and left-deep joins keep the linear form.

grokspice · 2026-09-25T16:40:30Z

@copilot review

Copilot

Copilot review overview

🟡 Changes recommended

Predicates from a nested right input can leak into WHERE and change LEFT JOIN semantics.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Prevent nested right predicates leaking into LEFT JOIN WHERE · New

What changed in this PR

Fixes SQL unparsing for joins used as another join’s right input, preserving the intended nested join tree.

Changes:

Tracks traversal inside right-side join inputs.
Emits right-nested joins as parenthesized NestedJoin factors.
Preserves nested source qualifiers and adds regression snapshots.

File	Description
`datafusion/sql/src/unparser/plan.rs`	Builds nested right-side joins and collects their source identifiers.
`datafusion/sql/src/unparser/ast.rs`	Adds traversal state and nested-join relation support.
`datafusion/sql/tests/cases/plan_to_sql.rs`	Tests inner and full right-nested join output.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…N's ON, refuse them under a FULL JOIN A join nested on the right routes its scans' filters to the shared `WHERE`, which for a LEFT JOIN discards the left rows the join preserves. Mirror the left-input hoist: set the accumulated predicate aside for the walk, take what the subtree contributed, and fold it into this join's `ON`. A FULL JOIN preserves both inputs, so a contribution there is refused (its scoping is #231's). A RIGHT JOIN preserves the right input, where `WHERE` is right.

Copilot

Copilot review overview

🟡 Changes recommended

Nested subquery predicates can be moved back into an outer JOIN ON, producing SQL rejected by supported dialects such as BigQuery.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Prevent subqueries from being hoisted into LEFT JOIN ON clauses · New

Resolved since last review (1)

Prevent nested right predicates leaking into LEFT JOIN WHERE

…d right input is walked, and refuse a subquery it would fold into ON Setting the accumulated predicate aside for the walk hid a mark column that a mark join in the right input rewrites in place, so it came back unbound (`WHERE c.mark` with `c` gone). The predicate stays put; what the walk adds is `AND`ed on after it and split off by position. A contributed conjunct holding a subquery is refused rather than folded into the outer `ON`, which some dialects reject, the same way an inner join keeps such a conjunct out of `ON`.

Copilot

Copilot review overview

🟡 Changes recommended

A mark predicate can remain unresolved when a nested RIGHT JOIN temporarily removes the enclosing selection.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Nested RIGHT JOIN hoist drops existing predicates · New

Resolved since last review (1)

Prevent subqueries from being hoisted into LEFT JOIN ON clauses

…s left input is walked too The left-side hoist set the accumulated predicate aside for the walk, so a mark join in that input could not rewrite the mark the predicate reads — `WHERE c.mark` above `(b MARK c) RIGHT JOIN d`, with `c` gone — and, nested on the right of an outer join, the unchanged predicate then counted as a kept conjunct. Both hoists now use the same count-and-split: the predicate stays, what the walk adds is split off by position. Pinned for the mark join as the input of a LEFT, FULL and RIGHT JOIN, and nested inside a RIGHT JOIN that is itself a LEFT JOIN's right input.

Copilot

Copilot review overview

🟢 Approved

The implementation matches the reported defect and includes comprehensive regression coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Nested RIGHT JOIN hoist drops existing predicates

… JOIN's left input too The FULL JOIN guard watched only its right input, so a filtered join nested on its left — at the top, or itself the right input of a LEFT JOIN — let the filter escape into the enclosing `WHERE` or the outer `ON`, where it rejected the rows the FULL JOIN preserves. Both inputs are watched the same way now and either contribution is refused. The test that pinned the top-level shape's `WHERE` placement as a known limitation now pins the refusal; the scoping that lifts it is #231's.

Copilot

Copilot review overview

🟡 Changes recommended

A scoped subquery predicate can still hide qualifiers by deriving an entire joined right input.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Preserve qualifiers when wrapping joined right inputs as derived tables · New

…ped subquery on a joined input A mark join's mark is replaced by `EXISTS`, which is never NULL; on an input an outer join null-extends the mark is NULL for the rows that join adds, so `NOT c.mark` above it kept rows the plan drops. Such a mark join is refused (a mark join on a preserved input still rewrites in place). A subquery conjunct the outer join's `ON` scopes onto a joined right input would be wrapped in an anonymous derived table that hides the names the enclosing query still uses; that is refused too.

Copilot

Copilot review overview

🟡 Changes recommended

Aliasing a nested right join can hide child qualifiers still emitted in hoisted predicates, producing invalid SQL.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Aliased nested joins hoist predicates with out-of-scope qualifiers · New

Resolved since last review (1)

Preserve qualifiers when wrapping joined right inputs as derived tables

…put as its own scope Walked into the shared SELECT, its scans' filters were hoisted or placed under names the alias then hides (`b.id = 'x'` beside `(b JOIN c) AS j`). Derived, they stay inside, the alias is what the enclosing query addresses, and its columns are this side's select items.

#231 landed scoping for a FULL JOIN input's predicates: a join nested in such an input derives its filtered scans, and the FULL JOIN fails with an internal error if an input still contributes to WHERE. This branch had refused those shapes until #231 landed, so the merge keeps #231's mechanism for FULL JOINs and this branch's for the rest: - The RIGHT JOIN left-input and LEFT JOIN right-input hoists into ON use this branch's positional split (take_selection_added_after), and are skipped below a FULL JOIN's input, where #231's walk keeps every predicate scoped. #231's restore_selection lost its last caller and is removed. - This branch's own FULL JOIN refusals are dropped; the shapes they pinned now unparse with the filtered scan derived, and the test asserts those snapshots (checked equal to the plan's rows on DuckDB and PostgreSQL). - The mark-join refusal on null-extended inputs is kept; under a FULL JOIN, #231's semi/anti/mark refusal fires first, and the test expects it there. - Both SubqueryAlias additions are kept: the aliased joined right input is derived first, then #231's aliased filtered scan. - The join input walks go through spiceai-54's walk_join_input.

…joined right input The early return that derives an aliased join reached as another join's right input passed the alias's renamed columns straight into the table alias, skipping the supports_column_alias_in_table_alias check the other derived paths apply. SQLite then got `AS j (x, y)`, which it rejects; the names now go inside the derived table instead.

krinart · 2026-10-01T17:35:47Z

Hold lifted: the finding in #233 (comment), which is also the "Previously missed" finding in #233 (review), is reproduced and fixed in 4b916a70c.

Reproduced. The plan was a LEFT JOIN (Projection[b.id AS x, c.id AS y](Projection[b.id, c.id](b ⋈ c))) AS j. Under SqliteDialect it emitted:

SELECT `a`.`id`, `j`.`x`, `j`.`y` FROM `a` LEFT OUTER JOIN (SELECT `b`.`id`, `c`.`id` FROM `b` INNER JOIN `c` ON `b`.`id` = `c`.`id`) AS `j` (`x`, `y`) ON `a`.`id` = `j`.`x`

sqlite3 rejects it with Parse error: near "(": syntax error. The aliased joined-right-input early return now applies the same supports_column_alias_in_table_alias / inject_column_aliases_into_subquery fallback as the derived paths below it. SQLite now gets the following, which sqlite3 runs:

SELECT `a`.`id`, `j`.`x`, `j`.`y` FROM `a` LEFT OUTER JOIN (SELECT `b`.`id` AS `x`, `c`.`id` AS `y` FROM `b` INNER JOIN `c` ON `b`.`id` = `c`.`id`) AS `j` ON `a`.`id` = `j`.`x`

The default dialect keeps AS j (x, y). Both are pinned at the end of right_nested_join_keeps_its_shape_on_the_right.

The Distinct half of that review's headline is answered in its thread (r4112269889).

Merged spiceai-54 (4cf679211) in a2700d6a0. #231 landed the FULL JOIN input scoping that this PR had deferred to, so the merge keeps #231's mechanism for FULL JOINs and this PR's mechanism for the rest:

The RIGHT JOIN left-input and LEFT JOIN right-input hoists into ON keep this PR's positional split. They are skipped below a FULL JOIN's input, where fix(unparser): keep a FULL JOIN input's scan filters scoped when the input is a join (refs spiceai/spiceai#12593) #231 already keeps every predicate scoped. restore_selection lost its last caller, so I removed it.
This PR's FULL JOIN refusals are gone. The three shapes they pinned now unparse with the filtered scan derived. On DuckDB and PostgreSQL, each emitted query returns the same multiset as a CTE formulation of the plan (EXCEPT ALL is empty in both directions):
- a FULL JOIN ((SELECT b.id FROM b WHERE (b.id = 'x')) AS b INNER JOIN c ON b.id = c.id) ON a.id = b.id
- (SELECT b.id FROM b WHERE (b.id = 'x')) AS b INNER JOIN c ON b.id = c.id FULL JOIN d ON b.id = d.id
- a LEFT OUTER JOIN (((SELECT b.id FROM b WHERE (b.id = 'x')) AS b INNER JOIN c ON b.id = c.id) FULL JOIN d ON b.id = d.id) ON a.id = d.id
The mark-join refusal on null-extended inputs stays. For a FULL JOIN (b MARK c), fix(unparser): keep a FULL JOIN input's scan filters scoped when the input is a join (refs spiceai/spiceai#12593) #231's own semi/anti/mark refusal fires first, and the test now expects that message there.

cargo test -p datafusion-sql --test sql_integration      745 passed; 22 failed
  (plan_to_sql: 383 passed vs spiceai-54's 382; the same 22 failed, diffed by name)
cargo test -p datafusion-sql --lib                       90 passed
cargo clippy -p datafusion-sql --all-targets --all-features --no-deps -- -D warnings   clean

The 22 are pre-existing snapshot failures on the base, unrelated to this PR.

Copilot

Copilot review overview

🟡 Changes recommended

Subquery hoisting and nested aliases still produce unsupported or unbound SQL in specific join shapes.

Review effort: Balanced
Findings: 1

Open (1)

Reject left-side subquery hoisting into RIGHT JOIN ON · New

Resolved since last review (1)

Recurse through Distinct when deriving joined input

…d fold into ON The mirror of the LEFT JOIN right-input refusal: a nested left input of a RIGHT JOIN can contribute a subquery conjunct (a semi join's EXISTS), which some dialects refuse in ON and which WHERE cannot hold for the null-extended side.

Copilot

Copilot review overview

🟡 Changes recommended

Joined inputs behind nested aliases and scoped left inputs can still emit unbound qualifiers.

Review effort: Balanced
Findings: 2

Open (2)

Recurse through nested aliases when deriving joined input scope · New
Reject scoped left input when it is a joined relation · New

Resolved since last review (1)

Reject left-side subquery hoisting into RIGHT JOIN ON

…and refuse a scoped subquery on a joined left input is_joined_relation stopped at a SubqueryAlias, so a right input shaped outer(Filter(inner(b JOIN c))) missed the derived scope and left the inner alias's filter naming `i` where only `o` is in scope. It now looks through aliases, and the outer alias derives the whole input. A RIGHT JOIN's ON conjunct scoped onto a joined left input was wrapped in an anonymous derived table that hid `b` and `c` from the enclosing query; it is now refused, as the right-input mirror already was.

Copilot

Copilot review overview

🟡 Changes recommended

Context-sensitive alias derivation can cause valid nested EXISTS build-side plans to be rejected.

Review effort: Balanced
Findings: 1

Open (1)

EXISTS analysis mishandles nested aliased join build scopes · New

Resolved since last review (2)

Reject scoped left input when it is a joined relation
Recurse through nested aliases when deriving joined input scope

…ild side

Copilot

Copilot review overview

🔵 Needs a closer look

The extensive join-scoping changes affect nuanced SQL semantics across multiple join types and dialects.

Review effort: Balanced
Findings: None

Resolved since last review (1)

EXISTS analysis mishandles nested aliased join build scopes

krinart · 2026-10-01T18:11:43Z

Re #233 (review) (🔵, no findings, head abc192733): Copilot asks for a human to check the join-scoping semantics, so this PR now needs a human approval, not another Copilot round.

Copilot raised five concrete findings this session, and each one is answered in its thread with a before/after reproduction:

left-hoist subquery: refused (r4158602844)
nested aliases: derived (r4158699825)
scoped subquery on a joined left input: refused (r4158700458)
EXISTS build side: the j.id key is fixed and pinned; the j.cid key keeps base's refusal, which is a follow-up (r4158771488)
dialect column-alias fallback: fixed (fix(unparser): keep a join that is another join's right input on that join's right (refs spiceai/spiceai#14373) #233 (comment))

Every SQL shape this PR changes or newly emits ran on DuckDB, PostgreSQL 17 or sqlite3 against the plan's semantics.

cargo test -p datafusion-sql --test sql_integration      745 passed; 22 failed (the same 22 names fail on spiceai-54 alone)
cargo clippy -p datafusion-sql --all-targets --all-features --no-deps -- -D warnings   clean
#233 + #234 merged together: conflict-free; sql_integration 749 passed / same 22 failed;
cargo test -p datafusion --test core_integration -- sql::unparser (TPC-H, ClickBench round-trips)   4 passed

Backport #233, #234 and #245 to spiceai-55 (nested right-input joins, join-input LIMIT, footer-cache hit map)

…n fixes backported from 54 (#14800) * build: pin the DataFusion 55 / Arrow 59 fork lines Moves every fork to its DataFusion 55 line: arrow-rs 59.3.0, DataFusion 55.1.0, Ballista (upstream main on DataFusion 55), federation, table-providers, iceberg-rust 0.10.1, duckdb-rs 1.4.4 on arrow 59, ADBC 24, snowflake-rs, spark-connect-rs, spice-rs and spicebench on arrow 59. datafusion-functions-json comes from crates.io (0.55.6); delta-kernel stays on 0.27.1 with its default engine split into delta_kernel_default_engine. docs/dev/fork_patches.md is re-audited against the new lines; the pins stay marked TEMPORARY until the fork pull requests merge. * feat: upgrade to DataFusion 55 and Arrow 59 - Statistics flow through DataFusion 55's StatisticsContext. The deprecated partition_statistics returns Absent for built-in plans on 55, so every wrapper implements child_stats_requests / statistics_from_inputs and the optimizer rules, join sizing and Flight batch sizing read real statistics again. - ExecutionPlan / TableProvider wrappers implement the new required and defaulted methods explicitly (apply_expressions, replace_children, try_to_proto, merge_into, ...), forwarding where they wrap. - Arrow 59 refuses a MAP whose entries field is nullable: Flight SQL reads replace the declared schema before decoding and normalize the map afterwards, and legacy IPC maps decode as a list (arrow_tools map_entries). - Delta Lake refuses Void and interval columns with a structured error instead of mis-reading them. - Ballista 55: Vcores, the protocol-version handshake, task-id based cancellation and the append-only task-info model. - Vortex 0.86 and the remaining API moves (TableReference, TableScanBuilder, TableSchema builder, codec proto converter, WriteOp::MergeInto). * ci: validate the datafusion-table-providers pin against spiceai-55-patches The DataFusion 55 line of the fork is spiceai-55-patches. * revert: drop the unfinished guard tests d65aa29 picked up by accident d65aa29 was meant to change only the CI branch check; it also committed work-in-progress test edits. Restore those files to 98800a8; the finished tests land in their own commit. * fix(duckdb): push regexp_count down NULL-preserving, as DataFusion 55 evaluates it DataFusion 55 (apache/datafusion#24239) makes regexp_count propagate NULL, as PostgreSQL does. The DuckDB rendering still wrapped the count in coalesce(.., 0) to match the old kernel, so a DuckDB-accelerated dataset answered 0 where local evaluation answers NULL, and a WHERE on the count kept rows local evaluation drops. Render it as len(regexp_extract_all(..)), which is NULL for a NULL input or pattern. * test(cayenne): count -0.0 as equal to 0.0 in the boundary-values test DataFusion 55 compares floats in SQL by IEEE 754 value, so float_val = 0.0 matches the -0.0 row too (4 rows, not 3). A MemTable in the same session returns the same rows, so this is the expectation, not Cayenne. * build: repin the forks and guard their new patches - Ballista 8bf37ad0: the executor poll loop accepts a vcore semaphore with no permits yet. Upstream's assert panicked the loop at startup, so no Spice executor registered and every cluster query failed. - DataFusion d98ae645 (MSRV-clean unparser), federation 9ca84a39, table-providers 661d16d1, iceberg 575d81a1: their own tests now run against the Spice DataFusion (and, for table-providers, ADBC) forks. - Guard tests for the unparser rescope, eager aggregation through StatisticsContext, SchemaCastScanExec statistics forwarding and Ballista TaskInfo fields 11/12; the ledger names them and records the Ballista fix. - The distributed EXPLAIN snapshot now reads Ballista 55's UnknownPartitioning(1) where it printed None. * build: point the fork pins at the spiceai-*-patches-2 branches The DataFusion 55 lines of arrow-rs, datafusion, datafusion-ballista, datafusion-federation, datafusion-table-providers, duckdb-rs, snowflake-rs and spark-connect-rs are reviewed on new spiceai-*-patches-2 branches, leaving the earlier spiceai-*-patches branches untouched. Same revisions; only the branch names in the pin comments, the fork-patch ledger and the table-providers pin check change. * fix: re-root a delegated projection on the inner plan before swapping DataFusion 55's projection pushdown reads a projection's input as the node being swapped with: `ProjectionExec` collapses the chain starting at `projection.input()`, `CoalescePartitionsExec` rebuilds from its child. A wrapper that handed its inner plan the projection unchanged (its input being the wrapper) got it back still above the wrapper, and the pushdown then nested another copy on every step: the Cayenne maintained-aggregate soundness test grew to gigabytes and never finished. CayenneAccelerationExec, the DuckDB aggregate pushdown marker and PartitionedUnionExec now rebuild the projection over their inner plan (whose schema they share) before delegating. Regression tests cover the Cayenne wrapper and the DuckDB marker. * fix(cayenne): read legacy inline map data under Arrow 59 and keep the type policy - Inline data written under a nullable map-entries declaration is read through arrow_tools::map_entries, since Arrow 59 refuses that declaration at decode. - The primary-key fast-path test's control query no longer uses a bare SUM, which DataFusion 55 answers from statistics without a scan. - RunEndEncoded columns stay refused: the Vortex pin can store them, but accepting them is a product change, so the drift test records the exception. * fix: bring the remaining tests and connectors in line with DataFusion 55 and Arrow 59 - Flight: repair a server's nullable map-entries declaration before decoding (Arrow 59 refuses it), as the Flight SQL path does. - Federation: SQLite and MySQL refuse a filter on a volatile projection output (DataFusion fork unparser), Turso forwards it, MSSQL opts out. - Declared types: parse the decimal form directly so an out-of-range precision/scale still names the problem under arrow-rs's stricter parser. - Expectations that DataFusion 55 changed: predicate reordering (reorder_q8), binary literals rendered as X'..', sorts moved below projections (vector scans), the NSQL context's version and Spark function list, and the null-aware anti join corrected in Ballista's distributed planner. - json_get tests follow datafusion-functions-json 0.55 (negative array indices, json_as_text for a string cast, integral floats in json_get_int). * fix(vortex): port the merged point-lookup and key-block code to DataFusion 55 and Vortex 0.86 Trunk's point-lookup and key-block changes were written against DataFusion 54 and the earlier Vortex pin. The scan now binds its projection and filter before handing them to Vortex (the reader takes bound expressions), the point read keeps trunk's optimize-then-bind of its conjuncts, dynamic filters are detected through DynamicFilterTracking, and TableSchema is built with From. Two merged tests move to the current Vortex Arrow conversion and DataFusion's non-zero batch_size. * fix(arrow_tools): keep the producer's dictionary ids when repairing a map declaration Repairing a nullable map-entries declaration re-encoded the IPC schema, and re-encoding renumbers dictionaries from zero while the dictionary batches that follow keep the producer's ids. A producer that numbers them differently then failed to decode, or decoded each dictionary column against another's dictionary and returned wrong values with no error (reproduced on the Flight SQL read, the Flight read and the Flight subscribe paths). The repair now patches the schema message in place: one byte per map field (the entries nullability, or the Map-to-List type tag for the decodable form), verified with the flatbuffers verifier and checked against the expected schema; an unexpected layout is refused rather than passed through. * fix(runtime): keep the built-in semantics of functions datafusion-spark 55 would shadow datafusion-spark 55 registers Spark versions of power/pow, atan2 and concat_ws over the built-ins (infinity instead of an error for zero to a negative power, Float64 widening, array flattening) and adds hypot, monthname, quote and weekday. None of that is a deliberate change, so those functions join trunc, date_trunc and date_part in the withheld list, and a test pins exactly which names resolve to a Spark implementation. The NSQL context now lists the functions the session actually registers, so it no longer advertises the withheld ones. * fix(duckdb): keep a binary literal out of SQL sent to DuckDB DataFusion 55's unparser renders a binary literal as X'ff', which DuckDB 1.4.4 reads as the text 'xff': through DuckDB federation, b = X'ff' returned the row holding the bytes 'xff' instead of 0xFF (and X'' missed the empty blob), with no error. Expressions with a binary literal now stay local; a NULL still pushes down. Covered end to end by duckdb_binary_literal_filters_agree_with_local_evaluation. * build: pin DataFusion at 006f3d21, which drops input sums from join statistics DataFusion 55 answers a bare SUM from exact statistics, and inner/outer join statistics carried each input's sum through, so SUM over a join returned the whole input table's sum (silent wrong results; caught by the Cayenne result-correctness parity tests). The fork fix is spiceai/datafusion#239; the ledger records it with those parity tests as its guard. * test(runtime-datafusion): drop a redundant clone in the binary-literal test * test: fix the Spark built-ins table width and report the pinned DataFusion revision - the_built_session_keeps_the_built_in_math_and_string_functions: the expected table padded one column a character wider than the rendered output; the values were already right. - spice-substrait-compliance reports DATAFUSION_FORK_REV, which still named the pre-upgrade revision; it now matches the workspace pin, as its test requires. * test(oracle): re-record the pushdown plan for DataFusion 55's decimal literal display DataFusion 55 prints a decimal literal as Decimal128(123.45,10,2) where 54 printed Decimal128(Some(12345),10,2). The SQL sent to Oracle is unchanged. * build: repin the forks to their merged fix PRs and carry spiceai-54 forward - DataFusion f22d1a74: spiceai-54 forward-merged into the 55 line (spiceai/datafusion#241), bringing #237 (a Date32 literal's cast uses the dialect's date type; SQLite read CAST('…' AS DATE) as a number, so date ranges pushed to SQLite matched nothing) and metadata-column listing pruning. - arrow-rs be4918e7, Ballista 7c4d54c2, table-providers 3045960a, iceberg 50e982dc, arrow-adbc 674d6184: the merged CI fix PRs. - Guard for #237: a Date32 range unparsed for SQLite keeps the rows it selects (runs the SQL on SQLite with --features sqlite); it fails on the previous pin. - Ledger rows for both; the substrait harness reports the new DataFusion pin. * build: pin DataFusion at ae719330, the merge of spiceai/datafusion#241 Same tree as f22d1a74 (spiceai-54 forward-merged into the 55 line); now the head of spiceai-55-patches-2 rather than a temporary branch. * style: rustfmt the SQLite date-range federation test * build: pin arrow-rs at its spiceai-59 merge and DataFusion at spiceai-55-patches-3 - arrow-rs: 2e2cc330, the merge of spiceai/arrow-rs#29 into spiceai-59 (same tree as the previous pin). - DataFusion: fc84f2d1 on spiceai-55-patches-3, which carries the Date32 literal fix (spiceai/datafusion#237) but not the metadata-column listing pruning (#229); the ledger records that pruning as not carried on 55 until it has a guard here. - Correct the spark-connect-rs branch annotation to spiceai-59-patches-2. * build: pin DataFusion at the spiceai-55 merge, and iceberg-rust and snowflake-rs at their arrow-rs bumps - DataFusion: 02550cf9, the merge of spiceai/datafusion#243 into spiceai-55 (same tree as the previous pin); no longer a temporary pin. - iceberg-rust: 0284be3d and snowflake-rs: b95a74e8, which move each fork's own arrow-rs pin to 2e2cc330 (Cargo manifests only; Spice builds the same code). * build: repin ballista, table-providers, iceberg-rust and spark-connect-rs to their branch heads - datafusion-ballista 557ae4ea: arrow-rs/DataFusion landed pins, and the Rust 1.99 clippy fixes (#70, #71, #72). - datafusion-table-providers 5f50cfd5 and iceberg-rust a0bd6b04: arrow-rs/DataFusion landed pins (#82, #53). - spark-connect-rs 3bf6421a: regenerated lockfile. datafusion-federation and duckdb-rs stay at 9ca84a39 and 8ee43073: table-providers still pins exactly those revisions, and a second revision of either would put two copies of the crate in the graph. Their newer heads only change their own Cargo manifests and lockfile. * build: pin iceberg-rust, snowflake-rs, spark-connect-rs, spice-rs and spicebench at their merges The fork PRs these pins pointed at have merged, mostly as squash merges, so several pinned commits are no longer on their base branches. Each pin now points at the merge on the fork's base branch: - iceberg-rust spiceai-0.10.1-df-55 @ 3e2a14e (spiceai/iceberg-rust#50, #54) - snowflake-rs spiceai-59 @ f555738 (spiceai/snowflake-rs#13, #15) - spark-connect-rs spiceai-59-2 @ 18ae9bd (spiceai/spark-connect-rs#15, #16) - spice-rs trunk @ 4429f39 (spiceai/spice-rs#103) - spicebench trunk @ 8a90555 (spiceai/spicebench#284) datafusion-federation stays at 9ca84a3, which is on spiceai-55 because spiceai/datafusion-federation#88 was a merge commit; only its branch note changes. What changes for Spice: lint-only rewrites in snowflake-api, and tests in iceberg-datafusion and spark-connect-core. The other differences are in the forks' own [patch.crates-io] sections, which cargo ignores for a dependency. spice-rs's tree is identical. * build: pin the forks at their landed version-branch revisions - arrow-adbc 6e4119ac (spiceai-24, #7), iceberg-rust 3e2a14e8 (spiceai-0.10.1-df-55, #50), datafusion-table-providers 465926a3 (spiceai-55, #80), snowflake-rs f5557381 (spiceai-59, #13), spark-connect-rs 18ae9bd3 (spiceai-59-2, #15), spice-rs 4429f395 and spicebench 8a90555d (trunk). - datafusion-federation stays at 9ca84a39, now on spiceai-55 (#88): datafusion-table-providers pins exactly that revision. - duckdb-rs stays at 8ee43073 and temporary: #49 was squash-merged, so that revision is not on spiceai-1.4.4, and datafusion-table-providers pins it, so both have to move together. - datafusion-ballista is unchanged; its version PR is still open. * build: pin datafusion-ballista at upstream 55.0.0-rc1 spiceai/datafusion-ballista#73 merged upstream's 55.0.0-rc1 tag into spiceai-55-patches-2, the head of spiceai/datafusion-ballista#68. #68 is still open, waiting on license-compliance decisions. Pin 718429f, the #73 merge, until #68 lands on spiceai-55. ballista-core, -executor, -scheduler, -history and -api-types move from 54.0.0 to 55.0.0; no packages are added or removed in Cargo.lock. `cargo check -p spiced --locked` passes on rustc 1.98.1. * docs(fork_patches): record the datafusion-ballista pin at 718429f4 (upstream 55.0.0-rc1, #73) * build: pin datafusion-ballista at a7c4c585, the merge of spiceai/datafusion-ballista#68 into spiceai-55 * build: pin datafusion-ballista at the spiceai-55 merge of #68 spiceai/datafusion-ballista#68 merged into spiceai-55 as a7c4c58, a merge commit whose tree is identical to 718429f, the previous pin (`git diff 718429f a7c4c58` is empty). Repoint the pin at the landed revision and drop the TEMPORARY marker from the fork-patch ledger, as scripts/check_fork_patches.py asks. The ledger's ballista section now names spiceai-55 and upstream 55.0.0-rc1 as its base. Cargo.lock changes only the source revision of the five ballista packages. * perf(cayenne): keep small Vortex scans whole on the main query path DataFusion 55 lowered `repartition_file_min_size` from 10 MiB to 1 MiB (apache/datafusion#22439), so the main Cayenne scan byte-range-splits a 1-10 MiB dimension table `target_partitions` ways and pays a Vortex footer open per range. The main scan now opts small scans out below 10 MiB, as the internal and protected-snapshot scans already do below 256 MiB; Parquet listing scans keep the new default. `small_snapshot_groups_opt_out_of_repartitioning` now asserts the main scan's opt-out and fails with the threshold back at 0. * docs(fork_patches): record duckdb-rs on the protected spiceai-1.4.4-patches-2 head, which table-providers pins * test(duckdb): build CreateExternalTable with DataFusion 55's locations field * ci: validate the datafusion-table-providers pin against spiceai-55, where #80 landed * test(cayenne): size the point-lookup fan-out control above the main scan's 10 MiB opt-out * fix(data-accelerator-api): implement DataFusion 55's apply_expressions for KeepFirstExec * test: port trunk's new tests to DataFusion 55 (CreateExternalTable locations, common::TableReference) * fix(cayenne): keep -0.0 and 0.0 equal in pushed-down float comparisons and memory-mode indexes DataFusion 55 compares floats with the two zeros equal (apache/datafusion#22835), while Vortex and the Arrow row encoding order them apart. Spell out both zeros when a float comparison or IN list with a zero literal is pushed into a Vortex scan, keep float column-vs-column comparisons above the scan, and hash memory-mode index keys with the file-mode KeyEncoder. * build: move to iceberg-rust 0.11.0 (RC) and opendal 0.58 Points the iceberg crates at spiceai/iceberg-rust#55, which merges the upstream 0.11.x release branch onto the DataFusion 55 fork line. opendal moves to 0.58 to match iceberg-storage-opendal, so one opendal stack is built instead of two. 0.58 removes the OperatorBuilder step: Operator::new returns the operator with the same default layers that finish() applied. * Bump datafusion to 55.2-rc1 * build: pin iceberg-rust at spiceai-0.11.0-df-55 (6317d03e) spiceai/iceberg-rust#55 merged into spiceai-0.11.0-df-55. Same Rust code as the previous pin (e02547cb); the branch adds only a Python CI pin and a doc-comment fix. fork_patches.md records the new branch and that SigV4 is now an AuthManager in the fork's REST catalog. * build: keep the lockfile's unrelated resolutions from the DataFusion bump The previous commit's lockfile update also re-resolved five crates with range requirements (hashbrown, heck, base64). Restore them so the iceberg repin changes only the iceberg-rust source lines. * build: pin iceberg-rust at spiceai-0.11.0-df-55 (7735caa2) Picks up spiceai/iceberg-rust#49 (IcebergTableProvider::catalog and ::table_ident) and #56 (the fork's own DataFusion pin, which does not affect this build). #49's fork_patches row and its compile guard land with #14588, which first calls the getters. * build: pin datafusion at spiceai/datafusion#250 (55.2.0-rc1 and the 54 backports) Moves the DataFusion fork from 02550cf9 (55.1.0) to 1749ce4e, the head of spiceai/datafusion#250, which merges into #249: `spiceai-55` at 55.2.0-rc1 (spiceai/datafusion#248) plus the `spiceai-54` patches #233, #234 and #245 carried onto 55, and #250's review fix. The guards for those follow in the next commit. The pin names a pull request branch, so the ledger row is marked TEMPORARY and `scripts/check_fork_patches.py` refuses to let it land until #250 and #249 merge and the pin moves to the `spiceai-55` merge commit. The 55.2.0-rc1 merge changes no Spice patch. Outside `Cargo.lock`, the fork's 02550cf9..27351ab0 diff has the same patch-id as upstream's 55.1.0..55.2.0-rc1 diff, except `datafusion-spark`'s `quote.rs`, which the fork's backport had already made identical to 55.2.0-rc1's. That backport's ledger row is dropped; upstream carries the fix as apache/datafusion#25277. `Cargo.lock` changes only the 36 DataFusion entries. * test: guard the DataFusion unparser and footer-cache fixes backported from 54 Each backported fix gets a guard here, so the next re-cut of the fork cannot drop it silently, plus a ledger row naming the guard. - spiceai/datafusion#233 (#14373): a join that is another join's right input stays a parenthesised joined table on that join's right. `a_join_that_is_another_joins_right_input_stays_on_its_right` checks that `b` is introduced before the outer `ON` names it and that a LEFT JOIN keeps a filter from inside it out of `WHERE`. With the `sqlite` feature it runs the SQL on SQLite and compares the rows with DataFusion's for the same plan. - spiceai/datafusion#234 (#14375): a `Limit` that is a join input bounds that input, not the join. `a_limit_on_a_join_input_bounds_that_input_rather_than_the_join`, with the same row comparison. - spiceai/datafusion#250: a projection over a join that passes up a mark join's mark is refused rather than unparsed with the mark unbound. `a_mark_a_projection_over_a_join_passes_up_is_refused`. - spiceai/datafusion#245 (#12952): the file metadata cache frees an evicted footer's hit counter. 55's `DefaultCache` already does, so 55 carries no patch; `crates/cayenne/tests/footer_cache_hit_counter_test.rs` pins the behaviour by counting the live heap across 20,000 cold footer reads into a full cache. At 02550cf9 the three unparser guards fail. SQLite rejects the nested join's SQL ("ON clause references tables to its right"), returns 1 row where the plan returns 2 for `b FULL JOIN (c LIMIT 1)`, and the mark join renders as `… LEFT OUTER JOIN (b) ON a.id = b.id WHERE NOT c.mark`. Against a local DataFusion build with the hit-counter prune removed, the footer-cache guard measures 2,646,176 bytes of heap growth. At 1749ce4e all four pass, and the footer-cache growth is 0 bytes. * build: pin datafusion at spiceai/datafusion#249's merge commit Moves the fork pin from #250's branch head to bebc4d58, the merge of #249 into `spiceai-55`. Its tree is #249's head, cbda233a: 55.2.0-rc1 plus the backported #233, #234 and #245. The pin names a landed revision, so the TEMPORARY marker comes off and `scripts/check_fork_patches.py` passes. #250 is not part of this revision, so its guard and ledger row are removed. * Update datafusion version to 55.2 in Cargo.toml * docs(fork_patches): record the unparser frame refactor #249 carries `cbda233a6` is the fourth Spice commit at the pinned revision. It moves the join-input bookkeeping that #233 and #234 add out of `select_to_sql_recursively_inner`, so an unoptimised build's frame stays where it was. It had no ledger row, so a re-cut could drop it without the audit noticing. Its row is a GAP, with an Open gaps entry. What it buys is stack headroom in a debug build: without it, the fork's `roundtrip_statement` needs 2,112 KiB and overflows a 2 MiB test thread. This repo runs tests on 8 MiB threads, so no test here sees the difference deterministically. The entry names the fork-side check to run at a re-cut instead. * Fix tests * build: pin DataFusion at spiceai/datafusion#251, so MAX of a partition column skips empty partitions (#14801) * build: pin datafusion at spiceai/datafusion#251's merge commit Moves the DataFusion fork pin from #249's merge commit to the head of `spiceai-55`, which adds two patches on top of it: - #251: a file that holds no rows gets no partition-column min/max. Without it, `MIN`/`MAX` of a partition column is answered from the listing's statistics with the value of a partition whose files are all empty: `max(p)` answers '99' for a `p=99` directory holding one empty file, where the rows give '3'. - #240: the listing prunes files by metadata-column predicates before opening them and reports those filters `Exact` (#229 on `spiceai-54`). Neither changes a manifest, so `Cargo.lock` only moves its source revision. Each patch gets a ledger row and a guard in the listing connector's tests, which the gate runs as library tests. * fix(listing): keep every predicate residual on the `_location` fast path The pin now carries spiceai/datafusion#240, so the inner listing reports a metadata-column predicate such as `_size < 50` `Exact`. `MetadataPruningListingTable` handed pushdown classification to the inner listing whenever a `_location` predicate was present, but its `_location` fast path opens the named objects and applies no other predicate, so `_location = x AND _size < 50` returned the rows of a file the `_size` predicate excludes. Report every predicate `Inexact` on that path as well, so a residual `FilterExec` re-applies it. This is #14790's change, which makes the same fix for partition predicates, taken byte for byte so that whichever of the two lands second merges cleanly. The new test goes through `create_listing_table` with `_location` and `_size` enabled. * test(metadata): expect the listing to answer `_size = 2319` exactly With spiceai/datafusion#240 the listing prunes files by a metadata-column predicate and reports it `Exact`, so the plan for `met_size`, which enables only `_size`, has no residual `Filter`. The scan lists only the four 2319-byte files; `data_0.parquet` is 2317 bytes, as its S3 `Content-Length` shows. Regenerated by running `metadata::s3_metadata_columns` and accepted with `cargo insta accept`. The rest of the test passes unchanged at this pin, including the `_location` plans and the returned rows. * fix(data-connector-api): satisfy pedantic clippy in the metadata-pruning listing test `make lint-rust`'s `--tests` pass rejected two lints in `listing_table_prunes_files_by_a_metadata_column_predicate`: `redundant_closure_for_method_calls` (`|group| group.len()`) and `format_collect` (`map(format!).collect::<String>()`). Use `FileGroup::len` and build the file contents with `writeln!` into one `String`. The test is unchanged in what it checks. --------- Co-authored-by: Luke Kim <[email protected]> Co-authored-by: Sergei Grebnov <[email protected]>

…n fixes backported from 54 (spiceai#14800) * build: pin the DataFusion 55 / Arrow 59 fork lines Moves every fork to its DataFusion 55 line: arrow-rs 59.3.0, DataFusion 55.1.0, Ballista (upstream main on DataFusion 55), federation, table-providers, iceberg-rust 0.10.1, duckdb-rs 1.4.4 on arrow 59, ADBC 24, snowflake-rs, spark-connect-rs, spice-rs and spicebench on arrow 59. datafusion-functions-json comes from crates.io (0.55.6); delta-kernel stays on 0.27.1 with its default engine split into delta_kernel_default_engine. docs/dev/fork_patches.md is re-audited against the new lines; the pins stay marked TEMPORARY until the fork pull requests merge. * feat: upgrade to DataFusion 55 and Arrow 59 - Statistics flow through DataFusion 55's StatisticsContext. The deprecated partition_statistics returns Absent for built-in plans on 55, so every wrapper implements child_stats_requests / statistics_from_inputs and the optimizer rules, join sizing and Flight batch sizing read real statistics again. - ExecutionPlan / TableProvider wrappers implement the new required and defaulted methods explicitly (apply_expressions, replace_children, try_to_proto, merge_into, ...), forwarding where they wrap. - Arrow 59 refuses a MAP whose entries field is nullable: Flight SQL reads replace the declared schema before decoding and normalize the map afterwards, and legacy IPC maps decode as a list (arrow_tools map_entries). - Delta Lake refuses Void and interval columns with a structured error instead of mis-reading them. - Ballista 55: Vcores, the protocol-version handshake, task-id based cancellation and the append-only task-info model. - Vortex 0.86 and the remaining API moves (TableReference, TableScanBuilder, TableSchema builder, codec proto converter, WriteOp::MergeInto). * ci: validate the datafusion-table-providers pin against spiceai-55-patches The DataFusion 55 line of the fork is spiceai-55-patches. * revert: drop the unfinished guard tests d65aa29 picked up by accident d65aa29 was meant to change only the CI branch check; it also committed work-in-progress test edits. Restore those files to 98800a8; the finished tests land in their own commit. * fix(duckdb): push regexp_count down NULL-preserving, as DataFusion 55 evaluates it DataFusion 55 (apache/datafusion#24239) makes regexp_count propagate NULL, as PostgreSQL does. The DuckDB rendering still wrapped the count in coalesce(.., 0) to match the old kernel, so a DuckDB-accelerated dataset answered 0 where local evaluation answers NULL, and a WHERE on the count kept rows local evaluation drops. Render it as len(regexp_extract_all(..)), which is NULL for a NULL input or pattern. * test(cayenne): count -0.0 as equal to 0.0 in the boundary-values test DataFusion 55 compares floats in SQL by IEEE 754 value, so float_val = 0.0 matches the -0.0 row too (4 rows, not 3). A MemTable in the same session returns the same rows, so this is the expectation, not Cayenne. * build: repin the forks and guard their new patches - Ballista 8bf37ad0: the executor poll loop accepts a vcore semaphore with no permits yet. Upstream's assert panicked the loop at startup, so no Spice executor registered and every cluster query failed. - DataFusion d98ae645 (MSRV-clean unparser), federation 9ca84a39, table-providers 661d16d1, iceberg 575d81a1: their own tests now run against the Spice DataFusion (and, for table-providers, ADBC) forks. - Guard tests for the unparser rescope, eager aggregation through StatisticsContext, SchemaCastScanExec statistics forwarding and Ballista TaskInfo fields 11/12; the ledger names them and records the Ballista fix. - The distributed EXPLAIN snapshot now reads Ballista 55's UnknownPartitioning(1) where it printed None. * build: point the fork pins at the spiceai-*-patches-2 branches The DataFusion 55 lines of arrow-rs, datafusion, datafusion-ballista, datafusion-federation, datafusion-table-providers, duckdb-rs, snowflake-rs and spark-connect-rs are reviewed on new spiceai-*-patches-2 branches, leaving the earlier spiceai-*-patches branches untouched. Same revisions; only the branch names in the pin comments, the fork-patch ledger and the table-providers pin check change. * fix: re-root a delegated projection on the inner plan before swapping DataFusion 55's projection pushdown reads a projection's input as the node being swapped with: `ProjectionExec` collapses the chain starting at `projection.input()`, `CoalescePartitionsExec` rebuilds from its child. A wrapper that handed its inner plan the projection unchanged (its input being the wrapper) got it back still above the wrapper, and the pushdown then nested another copy on every step: the Cayenne maintained-aggregate soundness test grew to gigabytes and never finished. CayenneAccelerationExec, the DuckDB aggregate pushdown marker and PartitionedUnionExec now rebuild the projection over their inner plan (whose schema they share) before delegating. Regression tests cover the Cayenne wrapper and the DuckDB marker. * fix(cayenne): read legacy inline map data under Arrow 59 and keep the type policy - Inline data written under a nullable map-entries declaration is read through arrow_tools::map_entries, since Arrow 59 refuses that declaration at decode. - The primary-key fast-path test's control query no longer uses a bare SUM, which DataFusion 55 answers from statistics without a scan. - RunEndEncoded columns stay refused: the Vortex pin can store them, but accepting them is a product change, so the drift test records the exception. * fix: bring the remaining tests and connectors in line with DataFusion 55 and Arrow 59 - Flight: repair a server's nullable map-entries declaration before decoding (Arrow 59 refuses it), as the Flight SQL path does. - Federation: SQLite and MySQL refuse a filter on a volatile projection output (DataFusion fork unparser), Turso forwards it, MSSQL opts out. - Declared types: parse the decimal form directly so an out-of-range precision/scale still names the problem under arrow-rs's stricter parser. - Expectations that DataFusion 55 changed: predicate reordering (reorder_q8), binary literals rendered as X'..', sorts moved below projections (vector scans), the NSQL context's version and Spark function list, and the null-aware anti join corrected in Ballista's distributed planner. - json_get tests follow datafusion-functions-json 0.55 (negative array indices, json_as_text for a string cast, integral floats in json_get_int). * fix(vortex): port the merged point-lookup and key-block code to DataFusion 55 and Vortex 0.86 Trunk's point-lookup and key-block changes were written against DataFusion 54 and the earlier Vortex pin. The scan now binds its projection and filter before handing them to Vortex (the reader takes bound expressions), the point read keeps trunk's optimize-then-bind of its conjuncts, dynamic filters are detected through DynamicFilterTracking, and TableSchema is built with From. Two merged tests move to the current Vortex Arrow conversion and DataFusion's non-zero batch_size. * fix(arrow_tools): keep the producer's dictionary ids when repairing a map declaration Repairing a nullable map-entries declaration re-encoded the IPC schema, and re-encoding renumbers dictionaries from zero while the dictionary batches that follow keep the producer's ids. A producer that numbers them differently then failed to decode, or decoded each dictionary column against another's dictionary and returned wrong values with no error (reproduced on the Flight SQL read, the Flight read and the Flight subscribe paths). The repair now patches the schema message in place: one byte per map field (the entries nullability, or the Map-to-List type tag for the decodable form), verified with the flatbuffers verifier and checked against the expected schema; an unexpected layout is refused rather than passed through. * fix(runtime): keep the built-in semantics of functions datafusion-spark 55 would shadow datafusion-spark 55 registers Spark versions of power/pow, atan2 and concat_ws over the built-ins (infinity instead of an error for zero to a negative power, Float64 widening, array flattening) and adds hypot, monthname, quote and weekday. None of that is a deliberate change, so those functions join trunc, date_trunc and date_part in the withheld list, and a test pins exactly which names resolve to a Spark implementation. The NSQL context now lists the functions the session actually registers, so it no longer advertises the withheld ones. * fix(duckdb): keep a binary literal out of SQL sent to DuckDB DataFusion 55's unparser renders a binary literal as X'ff', which DuckDB 1.4.4 reads as the text 'xff': through DuckDB federation, b = X'ff' returned the row holding the bytes 'xff' instead of 0xFF (and X'' missed the empty blob), with no error. Expressions with a binary literal now stay local; a NULL still pushes down. Covered end to end by duckdb_binary_literal_filters_agree_with_local_evaluation. * build: pin DataFusion at 006f3d21, which drops input sums from join statistics DataFusion 55 answers a bare SUM from exact statistics, and inner/outer join statistics carried each input's sum through, so SUM over a join returned the whole input table's sum (silent wrong results; caught by the Cayenne result-correctness parity tests). The fork fix is spiceai/datafusion#239; the ledger records it with those parity tests as its guard. * test(runtime-datafusion): drop a redundant clone in the binary-literal test * test: fix the Spark built-ins table width and report the pinned DataFusion revision - the_built_session_keeps_the_built_in_math_and_string_functions: the expected table padded one column a character wider than the rendered output; the values were already right. - spice-substrait-compliance reports DATAFUSION_FORK_REV, which still named the pre-upgrade revision; it now matches the workspace pin, as its test requires. * test(oracle): re-record the pushdown plan for DataFusion 55's decimal literal display DataFusion 55 prints a decimal literal as Decimal128(123.45,10,2) where 54 printed Decimal128(Some(12345),10,2). The SQL sent to Oracle is unchanged. * build: repin the forks to their merged fix PRs and carry spiceai-54 forward - DataFusion f22d1a74: spiceai-54 forward-merged into the 55 line (spiceai/datafusion#241), bringing spiceai#237 (a Date32 literal's cast uses the dialect's date type; SQLite read CAST('…' AS DATE) as a number, so date ranges pushed to SQLite matched nothing) and metadata-column listing pruning. - arrow-rs be4918e7, Ballista 7c4d54c2, table-providers 3045960a, iceberg 50e982dc, arrow-adbc 674d6184: the merged CI fix PRs. - Guard for spiceai#237: a Date32 range unparsed for SQLite keeps the rows it selects (runs the SQL on SQLite with --features sqlite); it fails on the previous pin. - Ledger rows for both; the substrait harness reports the new DataFusion pin. * build: pin DataFusion at ae719330, the merge of spiceai/datafusion#241 Same tree as f22d1a74 (spiceai-54 forward-merged into the 55 line); now the head of spiceai-55-patches-2 rather than a temporary branch. * style: rustfmt the SQLite date-range federation test * build: pin arrow-rs at its spiceai-59 merge and DataFusion at spiceai-55-patches-3 - arrow-rs: 2e2cc330, the merge of spiceai/arrow-rs#29 into spiceai-59 (same tree as the previous pin). - DataFusion: fc84f2d1 on spiceai-55-patches-3, which carries the Date32 literal fix (spiceai/datafusion#237) but not the metadata-column listing pruning (spiceai#229); the ledger records that pruning as not carried on 55 until it has a guard here. - Correct the spark-connect-rs branch annotation to spiceai-59-patches-2. * build: pin DataFusion at the spiceai-55 merge, and iceberg-rust and snowflake-rs at their arrow-rs bumps - DataFusion: 02550cf9, the merge of spiceai/datafusion#243 into spiceai-55 (same tree as the previous pin); no longer a temporary pin. - iceberg-rust: 0284be3d and snowflake-rs: b95a74e8, which move each fork's own arrow-rs pin to 2e2cc330 (Cargo manifests only; Spice builds the same code). * build: repin ballista, table-providers, iceberg-rust and spark-connect-rs to their branch heads - datafusion-ballista 557ae4ea: arrow-rs/DataFusion landed pins, and the Rust 1.99 clippy fixes (spiceai#70, spiceai#71, spiceai#72). - datafusion-table-providers 5f50cfd5 and iceberg-rust a0bd6b04: arrow-rs/DataFusion landed pins (spiceai#82, spiceai#53). - spark-connect-rs 3bf6421a: regenerated lockfile. datafusion-federation and duckdb-rs stay at 9ca84a39 and 8ee43073: table-providers still pins exactly those revisions, and a second revision of either would put two copies of the crate in the graph. Their newer heads only change their own Cargo manifests and lockfile. * build: pin iceberg-rust, snowflake-rs, spark-connect-rs, spice-rs and spicebench at their merges The fork PRs these pins pointed at have merged, mostly as squash merges, so several pinned commits are no longer on their base branches. Each pin now points at the merge on the fork's base branch: - iceberg-rust spiceai-0.10.1-df-55 @ 3e2a14e (spiceai/iceberg-rust#50, spiceai#54) - snowflake-rs spiceai-59 @ f555738 (spiceai/snowflake-rs#13, spiceai#15) - spark-connect-rs spiceai-59-2 @ 18ae9bd (spiceai/spark-connect-rs#15, spiceai#16) - spice-rs trunk @ 4429f39 (spiceai/spice-rs#103) - spicebench trunk @ 8a90555 (spiceai/spicebench#284) datafusion-federation stays at 9ca84a3, which is on spiceai-55 because spiceai/datafusion-federation#88 was a merge commit; only its branch note changes. What changes for Spice: lint-only rewrites in snowflake-api, and tests in iceberg-datafusion and spark-connect-core. The other differences are in the forks' own [patch.crates-io] sections, which cargo ignores for a dependency. spice-rs's tree is identical. * build: pin the forks at their landed version-branch revisions - arrow-adbc 6e4119ac (spiceai-24, spiceai#7), iceberg-rust 3e2a14e8 (spiceai-0.10.1-df-55, spiceai#50), datafusion-table-providers 465926a3 (spiceai-55, spiceai#80), snowflake-rs f5557381 (spiceai-59, spiceai#13), spark-connect-rs 18ae9bd3 (spiceai-59-2, spiceai#15), spice-rs 4429f395 and spicebench 8a90555d (trunk). - datafusion-federation stays at 9ca84a39, now on spiceai-55 (spiceai#88): datafusion-table-providers pins exactly that revision. - duckdb-rs stays at 8ee43073 and temporary: spiceai#49 was squash-merged, so that revision is not on spiceai-1.4.4, and datafusion-table-providers pins it, so both have to move together. - datafusion-ballista is unchanged; its version PR is still open. * build: pin datafusion-ballista at upstream 55.0.0-rc1 spiceai/datafusion-ballista#73 merged upstream's 55.0.0-rc1 tag into spiceai-55-patches-2, the head of spiceai/datafusion-ballista#68. spiceai#68 is still open, waiting on license-compliance decisions. Pin 718429f, the spiceai#73 merge, until spiceai#68 lands on spiceai-55. ballista-core, -executor, -scheduler, -history and -api-types move from 54.0.0 to 55.0.0; no packages are added or removed in Cargo.lock. `cargo check -p spiced --locked` passes on rustc 1.98.1. * docs(fork_patches): record the datafusion-ballista pin at 718429f4 (upstream 55.0.0-rc1, spiceai#73) * build: pin datafusion-ballista at a7c4c585, the merge of spiceai/datafusion-ballista#68 into spiceai-55 * build: pin datafusion-ballista at the spiceai-55 merge of spiceai#68 spiceai/datafusion-ballista#68 merged into spiceai-55 as a7c4c58, a merge commit whose tree is identical to 718429f, the previous pin (`git diff 718429f a7c4c58` is empty). Repoint the pin at the landed revision and drop the TEMPORARY marker from the fork-patch ledger, as scripts/check_fork_patches.py asks. The ledger's ballista section now names spiceai-55 and upstream 55.0.0-rc1 as its base. Cargo.lock changes only the source revision of the five ballista packages. * perf(cayenne): keep small Vortex scans whole on the main query path DataFusion 55 lowered `repartition_file_min_size` from 10 MiB to 1 MiB (apache/datafusion#22439), so the main Cayenne scan byte-range-splits a 1-10 MiB dimension table `target_partitions` ways and pays a Vortex footer open per range. The main scan now opts small scans out below 10 MiB, as the internal and protected-snapshot scans already do below 256 MiB; Parquet listing scans keep the new default. `small_snapshot_groups_opt_out_of_repartitioning` now asserts the main scan's opt-out and fails with the threshold back at 0. * docs(fork_patches): record duckdb-rs on the protected spiceai-1.4.4-patches-2 head, which table-providers pins * test(duckdb): build CreateExternalTable with DataFusion 55's locations field * ci: validate the datafusion-table-providers pin against spiceai-55, where spiceai#80 landed * test(cayenne): size the point-lookup fan-out control above the main scan's 10 MiB opt-out * fix(data-accelerator-api): implement DataFusion 55's apply_expressions for KeepFirstExec * test: port trunk's new tests to DataFusion 55 (CreateExternalTable locations, common::TableReference) * fix(cayenne): keep -0.0 and 0.0 equal in pushed-down float comparisons and memory-mode indexes DataFusion 55 compares floats with the two zeros equal (apache/datafusion#22835), while Vortex and the Arrow row encoding order them apart. Spell out both zeros when a float comparison or IN list with a zero literal is pushed into a Vortex scan, keep float column-vs-column comparisons above the scan, and hash memory-mode index keys with the file-mode KeyEncoder. * build: move to iceberg-rust 0.11.0 (RC) and opendal 0.58 Points the iceberg crates at spiceai/iceberg-rust#55, which merges the upstream 0.11.x release branch onto the DataFusion 55 fork line. opendal moves to 0.58 to match iceberg-storage-opendal, so one opendal stack is built instead of two. 0.58 removes the OperatorBuilder step: Operator::new returns the operator with the same default layers that finish() applied. * Bump datafusion to 55.2-rc1 * build: pin iceberg-rust at spiceai-0.11.0-df-55 (6317d03e) spiceai/iceberg-rust#55 merged into spiceai-0.11.0-df-55. Same Rust code as the previous pin (e02547cb); the branch adds only a Python CI pin and a doc-comment fix. fork_patches.md records the new branch and that SigV4 is now an AuthManager in the fork's REST catalog. * build: keep the lockfile's unrelated resolutions from the DataFusion bump The previous commit's lockfile update also re-resolved five crates with range requirements (hashbrown, heck, base64). Restore them so the iceberg repin changes only the iceberg-rust source lines. * build: pin iceberg-rust at spiceai-0.11.0-df-55 (7735caa2) Picks up spiceai/iceberg-rust#49 (IcebergTableProvider::catalog and ::table_ident) and spiceai#56 (the fork's own DataFusion pin, which does not affect this build). spiceai#49's fork_patches row and its compile guard land with spiceai#14588, which first calls the getters. * build: pin datafusion at spiceai/datafusion#250 (55.2.0-rc1 and the 54 backports) Moves the DataFusion fork from 02550cf9 (55.1.0) to 1749ce4e, the head of spiceai/datafusion#250, which merges into spiceai#249: `spiceai-55` at 55.2.0-rc1 (spiceai/datafusion#248) plus the `spiceai-54` patches spiceai#233, spiceai#234 and spiceai#245 carried onto 55, and spiceai#250's review fix. The guards for those follow in the next commit. The pin names a pull request branch, so the ledger row is marked TEMPORARY and `scripts/check_fork_patches.py` refuses to let it land until spiceai#250 and spiceai#249 merge and the pin moves to the `spiceai-55` merge commit. The 55.2.0-rc1 merge changes no Spice patch. Outside `Cargo.lock`, the fork's 02550cf9..27351ab0 diff has the same patch-id as upstream's 55.1.0..55.2.0-rc1 diff, except `datafusion-spark`'s `quote.rs`, which the fork's backport had already made identical to 55.2.0-rc1's. That backport's ledger row is dropped; upstream carries the fix as apache/datafusion#25277. `Cargo.lock` changes only the 36 DataFusion entries. * test: guard the DataFusion unparser and footer-cache fixes backported from 54 Each backported fix gets a guard here, so the next re-cut of the fork cannot drop it silently, plus a ledger row naming the guard. - spiceai/datafusion#233 (spiceai#14373): a join that is another join's right input stays a parenthesised joined table on that join's right. `a_join_that_is_another_joins_right_input_stays_on_its_right` checks that `b` is introduced before the outer `ON` names it and that a LEFT JOIN keeps a filter from inside it out of `WHERE`. With the `sqlite` feature it runs the SQL on SQLite and compares the rows with DataFusion's for the same plan. - spiceai/datafusion#234 (spiceai#14375): a `Limit` that is a join input bounds that input, not the join. `a_limit_on_a_join_input_bounds_that_input_rather_than_the_join`, with the same row comparison. - spiceai/datafusion#250: a projection over a join that passes up a mark join's mark is refused rather than unparsed with the mark unbound. `a_mark_a_projection_over_a_join_passes_up_is_refused`. - spiceai/datafusion#245 (spiceai#12952): the file metadata cache frees an evicted footer's hit counter. 55's `DefaultCache` already does, so 55 carries no patch; `crates/cayenne/tests/footer_cache_hit_counter_test.rs` pins the behaviour by counting the live heap across 20,000 cold footer reads into a full cache. At 02550cf9 the three unparser guards fail. SQLite rejects the nested join's SQL ("ON clause references tables to its right"), returns 1 row where the plan returns 2 for `b FULL JOIN (c LIMIT 1)`, and the mark join renders as `… LEFT OUTER JOIN (b) ON a.id = b.id WHERE NOT c.mark`. Against a local DataFusion build with the hit-counter prune removed, the footer-cache guard measures 2,646,176 bytes of heap growth. At 1749ce4e all four pass, and the footer-cache growth is 0 bytes. * build: pin datafusion at spiceai/datafusion#249's merge commit Moves the fork pin from spiceai#250's branch head to bebc4d58, the merge of spiceai#249 into `spiceai-55`. Its tree is spiceai#249's head, cbda233a: 55.2.0-rc1 plus the backported spiceai#233, spiceai#234 and spiceai#245. The pin names a landed revision, so the TEMPORARY marker comes off and `scripts/check_fork_patches.py` passes. spiceai#250 is not part of this revision, so its guard and ledger row are removed. * Update datafusion version to 55.2 in Cargo.toml * docs(fork_patches): record the unparser frame refactor spiceai#249 carries `cbda233a6` is the fourth Spice commit at the pinned revision. It moves the join-input bookkeeping that spiceai#233 and spiceai#234 add out of `select_to_sql_recursively_inner`, so an unoptimised build's frame stays where it was. It had no ledger row, so a re-cut could drop it without the audit noticing. Its row is a GAP, with an Open gaps entry. What it buys is stack headroom in a debug build: without it, the fork's `roundtrip_statement` needs 2,112 KiB and overflows a 2 MiB test thread. This repo runs tests on 8 MiB threads, so no test here sees the difference deterministically. The entry names the fork-side check to run at a re-cut instead. * Fix tests * build: pin DataFusion at spiceai/datafusion#251, so MAX of a partition column skips empty partitions (spiceai#14801) * build: pin datafusion at spiceai/datafusion#251's merge commit Moves the DataFusion fork pin from spiceai#249's merge commit to the head of `spiceai-55`, which adds two patches on top of it: - spiceai#251: a file that holds no rows gets no partition-column min/max. Without it, `MIN`/`MAX` of a partition column is answered from the listing's statistics with the value of a partition whose files are all empty: `max(p)` answers '99' for a `p=99` directory holding one empty file, where the rows give '3'. - spiceai#240: the listing prunes files by metadata-column predicates before opening them and reports those filters `Exact` (spiceai#229 on `spiceai-54`). Neither changes a manifest, so `Cargo.lock` only moves its source revision. Each patch gets a ledger row and a guard in the listing connector's tests, which the gate runs as library tests. * fix(listing): keep every predicate residual on the `_location` fast path The pin now carries spiceai/datafusion#240, so the inner listing reports a metadata-column predicate such as `_size < 50` `Exact`. `MetadataPruningListingTable` handed pushdown classification to the inner listing whenever a `_location` predicate was present, but its `_location` fast path opens the named objects and applies no other predicate, so `_location = x AND _size < 50` returned the rows of a file the `_size` predicate excludes. Report every predicate `Inexact` on that path as well, so a residual `FilterExec` re-applies it. This is spiceai#14790's change, which makes the same fix for partition predicates, taken byte for byte so that whichever of the two lands second merges cleanly. The new test goes through `create_listing_table` with `_location` and `_size` enabled. * test(metadata): expect the listing to answer `_size = 2319` exactly With spiceai/datafusion#240 the listing prunes files by a metadata-column predicate and reports it `Exact`, so the plan for `met_size`, which enables only `_size`, has no residual `Filter`. The scan lists only the four 2319-byte files; `data_0.parquet` is 2317 bytes, as its S3 `Content-Length` shows. Regenerated by running `metadata::s3_metadata_columns` and accepted with `cargo insta accept`. The rest of the test passes unchanged at this pin, including the `_location` plans and the returned rows. * fix(data-connector-api): satisfy pedantic clippy in the metadata-pruning listing test `make lint-rust`'s `--tests` pass rejected two lints in `listing_table_prunes_files_by_a_metadata_column_predicate`: `redundant_closure_for_method_calls` (`|group| group.len()`) and `format_collect` (`map(format!).collect::<String>()`). Use `FileGroup::len` and build the file contents with `writeln!` into one `String`. The test is unchanged in what it checks. --------- Co-authored-by: Luke Kim <[email protected]> Co-authored-by: Sergei Grebnov <[email protected]>

Copilot AI balanced review requested due to automatic review settings September 25, 2026 16:40

grokspice self-assigned this Sep 25, 2026

Copilot started reviewing on behalf of grokspice September 25, 2026 16:41

View session

grokspice mentioned this pull request Sep 25, 2026

unparser: a join that is the right input of a join is linearised into the outer FROM in the wrong order spiceai/spiceai#14373

Closed

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

Copilot AI review requested due to automatic review settings September 25, 2026 16:50

Copilot started reviewing on behalf of grokspice September 25, 2026 16:50

View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

Copilot AI review requested due to automatic review settings September 25, 2026 16:58

Copilot started reviewing on behalf of grokspice September 25, 2026 16:58

View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

Copilot AI review requested due to automatic review settings September 25, 2026 17:04

Copilot started reviewing on behalf of grokspice September 25, 2026 17:05

View session

Copilot AI previously approved these changes Sep 25, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 25, 2026 17:09

Copilot AI dismissed their stale review, a newer Copilot review was requested September 25, 2026 17:09

Copilot started reviewing on behalf of grokspice September 25, 2026 17:09

View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

Copilot AI review requested due to automatic review settings September 25, 2026 17:15

Copilot started reviewing on behalf of grokspice September 25, 2026 17:16

View session

grokspice mentioned this pull request Sep 25, 2026

fix(unparser): derive a Limit that is a join input under the scan's own name, unconditionally (refs spiceai/spiceai#14375) #234

Merged

5 tasks done

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

krinart added 2 commits October 1, 2026 10:32

Copilot AI balanced review requested due to automatic review settings October 1, 2026 17:34

Copilot started reviewing on behalf of grokspice October 1, 2026 17:34

View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

Copilot AI balanced review requested due to automatic review settings October 1, 2026 17:47

Copilot started reviewing on behalf of grokspice October 1, 2026 17:48

View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

Copilot AI balanced review requested due to automatic review settings October 1, 2026 17:58

Copilot started reviewing on behalf of grokspice October 1, 2026 17:59

View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

Comment thread datafusion/sql/src/unparser/plan.rs Resolved

test(unparser): pin an aliased joined right input inside an EXISTS bu…

abc1927

…ild side

Copilot AI balanced review requested due to automatic review settings October 1, 2026 18:06

Copilot started reviewing on behalf of grokspice October 1, 2026 18:07

View session

Copilot AI reviewed Oct 1, 2026

View reviewed changes

grokspice requested a review from krinart October 3, 2026 02:18

krinart mentioned this pull request Oct 5, 2026

Merge upstream DataFusion 55.2.0-rc1 into spiceai-55 #248

Merged

lukekim approved these changes Oct 5, 2026

View reviewed changes

lukekim merged commit 087715f into spiceai-54 Oct 5, 2026
2 checks passed

lukekim deleted the fix/14373-right-nested-join-order branch October 5, 2026 20:53

This was referenced Oct 5, 2026

Backport #233, #234 and #245 to spiceai-55 (nested right-input joins, join-input LIMIT, footer-cache hit map) #249

Merged

fix(unparser): refuse a mark join whose mark a projection over a join passes up (review fix for #249) #250

Closed

krinart added a commit that referenced this pull request Oct 5, 2026

Merge pull request #249 from spiceai/spiceai-55-patches-5

bebc4d5

Backport #233, #234 and #245 to spiceai-55 (nested right-input joins, join-input LIMIT, footer-cache hit map)

krinart mentioned this pull request Oct 6, 2026

chore: pin the fork at spiceai/datafusion#249 and guard the DataFusion fixes backported from 54 spiceai/spiceai#14800

Merged

Conversation

grokspice commented Sep 25, 2026 • edited by krinart Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Reproduction

Test plan

Review gate

Earlier heads

Uh oh!

grokspice commented Sep 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approved

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

krinart commented Oct 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

krinart commented Oct 1, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

grokspice commented Sep 25, 2026 •

edited by krinart

Loading