Skip to content

fix: plan prompt_jev in sort, window, group by, and joins; keep filters first - #3

Merged
eddietejeda merged 1 commit into
mainfrom
fix/plan-placement
Sep 24, 2026
Merged

eddietejeda merged 1 commit into
mainfrom
fix/plan-placement

Conversation

@eddietejeda

@eddietejeda eddietejeda commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Problems (found by a broad query test suite)

  1. ORDER BY prompt_jev(...), rank() OVER (ORDER BY prompt_jev(...)), GROUP BY prompt_jev(...), and prompt_jev in a join condition all failed with DataFusion's async functions should not be called directly. DataFusion 55 only plans async calls inside Projection and Filter.
  2. With a CTE and a struct field read, e.g. WITH c AS (SELECT prompt_jev(.., choice := [..]) AS k FROM t WHERE ..) SELECT k.choice FROM c, DataFusion's push_down_leaf_projections moved get_field(k, 'choice') through the filter and substituted k's definition without rechecking placement. The call then ran on every row before the filter, which is how a 70 KiB row that the filter excluded still failed the query.

Fixes

  • HoistJev analyzer rule: lifts calls out of Sort, Window, Aggregate, and Join into a projection beneath, referencing them by column. Rewritten expressions are aliased to their original names so nothing above changes. A join-condition call spanning both sides gets a clear planning error instead of an internal one.
  • LeafPushdownGuard: wraps DataFusion's extract_leaf_expressions and push_down_leaf_projections so they skip plans containing the call and run unchanged for every other plan. A test asserts the optimization still fires for ordinary queries.
  • README documents both behaviours.

Tests

31 pass (7 new): sort, window, group by, one-sided join, two-sided join error, filter-before-inference through a CTE, and leaf pushdown still active without prompt_jev.

…rs first

DataFusion 55 lifts async calls only out of Projection and Filter, so a
prompt_jev in ORDER BY, OVER (...), GROUP BY, or a join condition hit the
synchronous path and failed with "async functions should not be called
directly". A new analyzer rule hoists such calls into a projection beneath
the node and refers to them by column, aliasing rewritten expressions so
the plan above keeps its column names. A call in a join condition that
spans both sides is refused with a clear planning error.

DataFusion's leaf-expression pushdown also moved get_field(k, 'x')
through a filter and substituted k's definition without rechecking
placement, so a prompt_jev call ran on every row before the filter. The
two pushdown rules are now wrapped to skip plans that contain the call;
other plans keep the optimization.
@eddietejeda
eddietejeda requested a review from a team as a code owner September 24, 2026 00:14
@eddietejeda
eddietejeda requested review from rohan-hotdata and removed request for a team September 24, 2026 00:14
Comment thread src/planner.rs
let (projected, substitutions) = hoist_calls(input, &refs)?;
let group_expr = group_expr
.into_iter()
.map(|e| substitute_keeping_name(e, &substitutions))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: GROUP BY ROLLUP/CUBE/GROUPING SETS (prompt_jev(...)) still fails, with an unclear error (not blocking).

Fix: handle Expr::GroupingSet separately, or return a clear plan_err! for grouping sets that contain the call. Alternatively, limit the README claim to plain GROUP BY.

For a grouping set, substitute_keeping_name produces Alias(GroupingSet(..)). Aggregate::try_new only recognizes a bare Expr::GroupingSet as the first group expression. The aliased form is treated as an ordinary expression, and planning fails. The inner expressions also change name to __jev_hoist_N, so the projection above no longer finds its columns.

Comment thread README.md
Reading several fields from one result does not repeat the request.
- **Anywhere in a query.** `prompt_jev` works in `SELECT`, `WHERE`, `ORDER BY`,
`GROUP BY`, `HAVING`, window `OVER (...)` clauses, and join conditions. A call
in a join condition must use columns from one side of the join only.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Document that a join-condition call runs on every row of its side, before the join removes any rows (not blocking).

hoist_join projects the call beneath the join input. For big JOIN tiny ON big.id = tiny.id AND prompt_jev(big.body, ..) > 0.5, every big row goes to the service, including rows without a match. Before this change, a matching condition would be evaluated only on joined pairs. Users who pay per request need this cost stated next to the "Filters run first" bullet. The README can suggest joining first in a CTE and filtering with WHERE afterwards.

@eddietejeda
eddietejeda merged commit 7164f5d into main Sep 24, 2026
3 checks passed
@eddietejeda
eddietejeda deleted the fix/plan-placement branch September 24, 2026 00:16
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