alamb commented on PR #24426: URL: https://github.com/apache/datafusion/pull/24426#issuecomment-5342167688
## Local Q25 experiment: the regression is real, and the dynamic-only refinement makes it worse To take benchmark-runner noise out of the picture, I built `--profile=profiling` binaries at three commits — the merge-base (`00eba79`), the gate (`026fb7d`), and the dynamic-only refinement (`106653a`) — and ran ClickBench Q25 locally against `hits_partitioned` on a quiet machine, replicating the harness setup (`binary_as_string=true`, the `hits` view with the `EventDate` cast, `reorder_filters` left at default). Each cell below is 10 runs × 3 interleaved rounds (30 runs) after 2 warmups; round-to-round averages agreed within ~1ms, stdev ≤3ms. Q25: `SELECT "SearchPhrase" FROM hits WHERE "SearchPhrase" <> '' ORDER BY "SearchPhrase" LIMIT 10;` | Binary | pushdown ON avg (min) ms | pushdown OFF avg (min) ms | ON/OFF | |---|---|---|---| | merge-base `00eba79` | 107.8 (104) | 86.4 (84) | 1.25x | | gate `026fb7d` | 110.8 (108) | 88.1 (86) | 1.26x | | dynamic-only `106653a` | 115.9 (113) | 85.2 (83) | **1.36x** | Conclusions: 1. **The Q25 pushdown penalty pre-exists this PR**: enabling pushdown costs ~25% even on the merge-base. Flipping the default merely exposes it. 2. **The dynamic-only refinement (`106653a`) is a real loss on Q25** (+5ms over `026fb7d`, +8ms over base, reproduced every round) — moving `SearchPhrase <> ''` out of the RowFilter while keeping the string-keyed TopK threshold in it is the wrong trade for this shape. Notably `026fb7d`-ON ≈ base-ON, so the gate's all-or-nothing dynamic carve-out was not the main cost for Q25 either — the RowFilter machinery on this query shape (filter column = sort column = only projected column) is. 3. Pushdown OFF is identical across all three binaries (~85–88ms): the changes are clean no-ops when pushdown is disabled. **Decision: reverting `106653a` from this branch.** The Q24 improvement it showed in the bot run needs re-validation after understanding the real mechanism; the next step is profiling the ~20–30ms ON-vs-OFF delta that exists on *every* binary for Q25 (filter-phase decode, selection building, predicate cache round-trip, and the per-row threshold compare are the candidates), plus the same local matrix on Q24 (int sort key) for contrast. -- 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]
