Dandandan opened a new pull request, #2344: URL: https://github.com/apache/datafusion-ballista/pull/2344
## Which issue does this PR close? None filed. Found while building executed-plan goldens: the same query planned twice produced different plans, and the difference was how many copies of a predicate its scans carried. ## Rationale for this change AQE re-optimizes the physical plan after every stage completion, and Ballista's rule list re-ran `FilterPushdown` each time. Pushing an already-pushed filter appends it to the scan again, so a scan accumulated one copy of its predicate per replan that touched its branch. Across the 22 TPC-H queries at SF10, that left **17 scans in 10 queries carrying duplicated predicates — 50 redundant conjuncts**, up to six copies: ``` q2 DataSourceExec: region predicate=r_name = EUROPE AND r_name = EUROPE AND ... (6x) q7 DataSourceExec: nation predicate=(n_name = FRANCE OR n_name = GERMANY) AND ... (6x) q20 DataSourceExec: nation predicate=n_name = CANADA AND ... (5x) ``` Every copy is evaluated per row, and again per row group in the duplicated `pruning_predicate`. It also inflates the plan serialized to every task, and — since the copy count depends on stage completion order — it makes a query's plan differ between runs of the same binary on the same data. ## What changes are included in this PR? Move `FilterPushdown` into `plan_preparation_optimizers`, the list that already runs once before the per-replan rules, and drop it from the per-replan list. Two lines. ## Are these changes tested? `cargo test -p ballista-scheduler` passes (362 + 25); clippy clean. Measured on the executed plans of all 22 queries (event logs from a real run, SF10, `target_partitions=16`), predicate conjuncts per query before → after: | q2 | q3 | q5 | q7 | q8 | q10 | q11 | q19 | q20 | q21 | q1 | q18 | | -- | -- | -- | -- | -- | --- | --- | --- | --- | --- | -- | --- | | 39→7 | 10→4 | 24→6 | 72→12 | 42→8 | 10→6 | 20→4 | 94→70 | 24→8 | 18→4 | 1→1 | 0→0 | Stage counts are identical for all 22, and q1/q18 (which had no duplication) are unchanged — so nothing moved except the redundant conjuncts. Row counts unchanged. ### Runtime: no measurable effect, and none expected A paired A/B at SF10 on two executors x 4 vcores — base and this branch run back to back per query, order flipped each of 5 repetitions, best-of-2 iterations per run: | query | base | this PR | median paired ratio | | ----- | ---- | ------- | ------------------- | | q3 | 0.983s | 0.930s | 0.99 | | q7 | 1.405s | 1.406s | 1.09 | | q8 | 0.715s | 0.725s | 1.08 | | **q18 (control, no duplication)** | 4.163s | 4.192s | **1.06** | | q19 | 0.666s | 0.737s | 1.12 | The control moves as much as the affected queries, and the affected queries' pooled median (1.044) is below it, so this is measurement noise: per-pair ratios span 0.74–1.43 on a machine that was running another workload throughout. That matches what the change can be expected to do. Most duplicates sit on `region` (5 rows) and `nation` (25 rows), where evaluating a predicate six times costs nothing; on the large tables a duplicate conjunct is one extra comparison on an already-decoded column, against ~100 bytes per row of parquet decode. The case for the change is that the work has no purpose, the plan shipped to every task is smaller, and plans stop depending on stage completion order — the last of which is what a plan-stability suite needs. An earlier round-level A/B of this change reported a uniform ~10% slowdown; the controls showed the same 10%, so it was bias between halves of each round, which is why the design above pairs at the query level. ## Are there any user-facing changes? No. A filter that a later replan would have re-pushed now stays as the `FilterExec` it already is. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
