refactor: share the internal name and split the long functions - #6
Merged
Merged
Conversation
No behaviour, error-message, or public API change.
- new src/names.rs: INTERNAL_FUNCTION plus the is_jev / is_jev_expr
predicates and an async_exprs wrapper that carries the single
#[allow(deprecated)]; sql.rs, udf.rs, planner.rs and optimizer.rs now
use them instead of repeating the literal.
- optimizer.rs: both rules start from one async_node helper; module doc
now describes both rules in code order.
- planner.rs: hoist_calls_named folded into hoist_calls with a prefix
parameter; module doc rewritten as a description of the two rules.
- sql.rs: rewrite_statement split into parse_call and parse_batch_size;
criteria() match arms reformatted.
- udf.rs: plan_batches (with a Batches struct) and probabilities()
extracted from invoke_async_with_args and answer().
- tests/sql.rs split into tests/sql/{main,common,syntax,execution,
planning}.rs; the same 36 tests, unchanged.
eddietejeda
requested review from
rohan-hotdata
and removed request for
a team
September 24, 2026 16:05
| } | ||
| /// Read one `prompt_jev` call: the input expression and the question it asks. | ||
| /// The question is not validated here; the caller does that before rewriting. | ||
| fn parse_call(f: &mut ast::Function) -> Result<(Expr, Question)> { |
There was a problem hiding this comment.
nit: Take &ast::Function and match on &f.args at line 172 (not blocking).
parse_call only reads f. The caller now replaces f.args itself after the call returns. The &mut signature suggests that parse_call mutates the call, but it does not.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Behaviour-preserving tidy-up. No public API, error message, or Cargo.lock change; the same 36 tests pass.
names.rs(new): one constant for the internal function name, the logical and physical "is this the call" predicates, and the single#[allow(deprecated)]accessor forAsyncFuncExec::async_exprs. Previously the name was a literal in four files and the predicate was written twice.sql.rs:rewrite_statementsplit intoparse_callandparse_batch_size; the entry point now reads find → parse → validate → replace, and builds the new argument list directly.criteria()reformatted to one statement per line.udf.rs:plan_batchesand a smallBatchesstruct extracted frominvoke_async_with_args;probabilitiesextracted fromanswer.optimizer.rs/planner.rs: shared prologue for the two physical rules, thehoist_callswrapper folded into one function, module docs rewritten to describe the current rules rather than their history.tests/sql.rs(893 lines) split intotests/sql/{common,syntax,execution,planning}.rswith the mock and helpers shared fromcommon. Test bodies are unchanged.Verification
cargo fmt --check,cargo clippy --locked --all-targets -- -D warnings,cargo test --locked(36 pass),cargo doc --no-deps(0 warnings).git diff main -- Cargo.lockis empty.