jayzhan211 opened a new pull request, #26436: URL: https://github.com/apache/datafusion/pull/26436
## Which issue does this PR close? - Fixes a regression from #25503 (not released yet, so it would ship in 56.0.0). ## Rationale for this change Since #25503, a selective filter over `Utf8View`/`BinaryView` columns keeps the whole input string buffers alive. ClickBench Q21-Q23 use about 2-3x more memory and Q22/Q23 run 1.6-1.7x slower with the default `schema_force_view_types = true`. #25503 made `FilterExec` call `BatchCoalescer::push_batch_with_filter` instead of `filter_record_batch` + `push_batch`. When at most 1/16 of the rows are selected, arrow-select 60 takes a sparse path that filters the views but reuses the source data buffers as they are (`InProgressByteViewArray::copy_rows_by_filter_from` calls `append_views_and_update_buffer_index(..., false)`). The old path went through `push_batch`, which sees that the filtered array uses less than half of its buffers (`need_gc`) and copies the selected strings into compact buffers. So each output batch now pins every data page it was filtered from, and operators that hold batches (aggregation, sort, TopK) hold all of them. arrow-rs main has the same code. Repro (ClickBench `hits_partitioned`, default settings): ```sql SET datafusion.execution.parquet.binary_as_string = true; CREATE EXTERNAL TABLE hits STORED AS PARQUET LOCATION 'benchmarks/data/hits_partitioned'; SELECT * FROM hits WHERE "URL" LIKE '%google%' ORDER BY "EventTime" LIMIT 10; -- Q23 ``` ## What changes are included in this PR? `LimitedBatchCoalescer::push_batch_with_filter` sends batches that have a `Utf8View` or `BinaryView` column with data buffers through `filter_record_batch` + `push_batch`, where arrow compacts sparse string views. All other batches keep the fused path from #25503. This changes nothing for view batches when the filter is not sparse: arrow already materializes the filter and calls `push_batch` for them. So the only path that changes is the sparse one for view columns, which goes back to its behavior before #25503. ## What is the testing strategy for this PR? - New unit test `test_push_batch_with_filter_compacts_string_views`: a 1/100 filter over 1600 long strings. Without the change the output keeps all 114,688 input string bytes; with it, less than a quarter. - Benchmarks, release-nonlto, same commit (5f2158269e) with and without this PR, plus #25503 reverted for reference; median of 3 interleaved runs, peak RSS: | Query | main | this PR | #25503 reverted | |---|---|---|---| | ClickBench Q21 | 0.60 s, 2.1 GB | 0.59 s, 1.1 GB | 0.62 s, 1.2 GB | | ClickBench Q22 | 2.14 s, 4.1 GB | 1.40 s, 1.5 GB | 1.26 s, 1.4 GB | | ClickBench Q23 | 7.10 s, 6.9 GB | 4.35 s, 3.4 GB | 4.33 s, 3.4 GB | | ClickBench total (43 queries) | 29.8 s | 25.5 s | 25.9 s | | TPC-H SF10 total (22 queries) | 6.89 s | 6.88 s | 6.93 s | Other ClickBench queries differ only within run-to-run noise. TPC-H keeps the #25503 result. ## Are there any user-facing changes? No. -- 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]
