You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "zhuqi-lucas (via GitHub)" <gi...@apache.org> on 2026/03/28 04:06:49 UTC

[PR] feat: add sort pushdown benchmark [datafusion]

zhuqi-lucas opened a new pull request, #21213:
URL: https://github.com/apache/datafusion/pull/21213

   ## Which issue does this PR close?
   
   Related to https://github.com/apache/datafusion/issues/17348
   Precursor to https://github.com/apache/datafusion/pull/21182
   
   ## Rationale for this change
   
   Add a dedicated benchmark for sort pushdown optimization, split out from #21182 per reviewer request. This allows running baseline benchmarks before and after the optimization lands.
   
   ## What changes are included in this PR?
   
   New `sort-pushdown` benchmark subcommand with 4 queries testing sort elimination:
   
   | Query | Description |
   |-------|-------------|
   | Q1 | `ORDER BY l_orderkey ASC` (full scan) |
   | Q2 | `ORDER BY l_orderkey ASC LIMIT 100` |
   | Q3 | `SELECT * ORDER BY l_orderkey ASC` (wide) |
   | Q4 | `SELECT * ORDER BY l_orderkey ASC LIMIT 100` (wide) |
   
   Each query can be run with or without `--sorted` flag:
   - **Without `--sorted`**: baseline with full SortExec
   - **With `--sorted`**: declares `WITH ORDER`, enabling sort elimination
   
   ### Usage
   
   ```bash
   # Prepare data
   ./bench.sh data sort_pushdown
   
   # Baseline
   ./bench.sh run sort_pushdown
   
   # With sort elimination
   ./bench.sh run sort_pushdown_sorted
   ```
   
   ### Files Changed
   
   | File | Change |
   |------|--------|
   | `benchmarks/src/sort_pushdown.rs` | New benchmark module |
   | `benchmarks/src/lib.rs` | Register module |
   | `benchmarks/src/bin/dfbench.rs` | Register subcommand |
   | `benchmarks/bench.sh` | Add data/run entries |
   
   ## Test plan
   
   - [x] `cargo clippy -p datafusion-benchmarks` — 0 warnings
   - [x] `cargo check -p datafusion-benchmarks` — compiles
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add sort pushdown benchmark and SLT tests [datafusion]

Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on PR #21213:
URL: https://github.com/apache/datafusion/pull/21213#issuecomment-4152818296

   Thanks @alamb and @xudong963 review, merged now.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add sort pushdown benchmark and SLT tests [datafusion]

Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas merged PR #21213:
URL: https://github.com/apache/datafusion/pull/21213


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add sort pushdown benchmark and SLT tests [datafusion]

Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #21213:
URL: https://github.com/apache/datafusion/pull/21213#issuecomment-4158608164

   Benchmark for [this request](https://github.com/apache/datafusion/pull/21213#issuecomment-4158598504) failed.
   
   Last 20 lines of output:
   <details><summary>Click to expand</summary>
   
   ```
       stats_merge
       strpos
       struct_query_sql
       substr
       substr_index
       substring
       sum
       to_char
       to_hex
       to_timestamp
       topk_aggregate
       topk_repartition
       translate
       trim
       trunc
       unhex
       upper
       uuid
       window_query_sql
       with_hashes
   ```
   
   </details>
   
   ---
   [File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add sort pushdown benchmark and SLT tests [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #21213:
URL: https://github.com/apache/datafusion/pull/21213#discussion_r3006697814


##########
benchmarks/src/sort_pushdown.rs:
##########
@@ -0,0 +1,306 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Benchmark for sort pushdown optimization.
+//!
+//! Tests performance of sort elimination when files are non-overlapping and
+//! internally sorted (declared via `--sorted` / `WITH ORDER`).
+//!
+//! # Usage
+//!
+//! ```text
+//! # Prepare sorted TPCH lineitem data (SF=1)
+//! ./bench.sh data sort_pushdown
+//!
+//! # Baseline (no WITH ORDER, full SortExec)
+//! ./bench.sh run sort_pushdown
+//!
+//! # With sort elimination (WITH ORDER, SortExec removed)
+//! ./bench.sh run sort_pushdown_sorted
+//! ```
+//!
+//! # Reference Results
+//!
+//! Measured on 300k rows, 8 non-overlapping sorted parquet files, single partition,
+//! debug build (results vary by hardware; relative speedup is the key metric):
+//!
+//! ```text
+//! Query | Description          | baseline (ms) | sort eliminated (ms) | speedup
+//! ------|----------------------|---------------|---------------------|--------
+//! Q1    | ASC full scan        |           159 |                  91 |  43%
+//! Q2    | ASC LIMIT 100        |            36 |                  12 |  67%
+//! Q3    | ASC full (wide, *)   |           487 |                 333 |  31%
+//! Q4    | ASC LIMIT 100 (wide) |           119 |                  30 |  74%
+//! ```
+//!
+//! Key observations:
+//! - **LIMIT queries benefit most** (67-74%): sort elimination + limit pushdown
+//!   means only the first few rows are read before stopping.
+//! - **Full scans** (31-43%): saving comes from eliminating the O(n log n) sort
+//!   step entirely.
+//! - **Wide projections** amplify the benefit: larger rows make sorting more
+//!   expensive, so eliminating it saves more.
+
+use clap::Args;
+use futures::StreamExt;
+use std::path::PathBuf;
+use std::sync::Arc;
+
+use datafusion::datasource::TableProvider;
+use datafusion::datasource::file_format::parquet::ParquetFormat;
+use datafusion::datasource::listing::{
+    ListingOptions, ListingTable, ListingTableConfig, ListingTableUrl,
+};
+use datafusion::error::Result;
+use datafusion::execution::SessionStateBuilder;
+use datafusion::physical_plan::display::DisplayableExecutionPlan;
+use datafusion::physical_plan::{displayable, execute_stream};
+use datafusion::prelude::*;
+use datafusion_common::DEFAULT_PARQUET_EXTENSION;
+use datafusion_common::instant::Instant;
+
+use crate::util::{BenchmarkRun, CommonOpt, QueryResult, print_memory_stats};
+
+#[derive(Debug, Args)]
+pub struct RunOpt {
+    /// Common options
+    #[command(flatten)]
+    common: CommonOpt,
+
+    /// Sort pushdown query number. If not specified, runs all queries
+    #[arg(short, long)]
+    pub query: Option<usize>,
+
+    /// Path to data files (lineitem). Only parquet format is supported.
+    /// Data should be pre-sorted by l_orderkey ASC for meaningful results.
+    #[arg(required = true, short = 'p', long = "path")]
+    path: PathBuf,
+
+    /// Path to JSON benchmark result to be compared using `compare.py`
+    #[arg(short = 'o', long = "output")]
+    output_path: Option<PathBuf>,
+
+    /// Mark the first column (l_orderkey) as sorted via WITH ORDER.
+    /// When set, enables sort elimination for matching queries.
+    #[arg(short = 't', long = "sorted")]
+    sorted: bool,
+}
+
+pub const SORT_PUSHDOWN_QUERY_START_ID: usize = 1;
+pub const SORT_PUSHDOWN_QUERY_END_ID: usize = 4;
+
+impl RunOpt {
+    const TABLES: [&'static str; 1] = ["lineitem"];
+
+    /// Queries benchmarking sort elimination when files are non-overlapping
+    /// and internally sorted (WITH ORDER declared via `--sorted`).
+    ///
+    /// With `--sorted`: ParquetSource returns Exact, files are verified
+    /// non-overlapping by statistics → SortExec eliminated, no SPM needed
+    /// for single partition.
+    ///
+    /// Without `--sorted`: baseline with full SortExec.
+    const QUERIES: [&'static str; 4] = [

Review Comment:
   as a minor suggestion I found it much easier to work with benchmarks when they are outside the runner (e.g. in separte files like https://github.com/apache/datafusion/blob/main/benchmarks/queries/clickbench/queries/q1.sql and https://github.com/apache/datafusion/blob/main/benchmarks/queries/q1.sql)
   
   as then it is easier to find what is being run and it can be tweaked and run using datafusion-cli



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add sort pushdown benchmark [datafusion]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #21213:
URL: https://github.com/apache/datafusion/pull/21213#discussion_r3004119722


##########
benchmarks/src/sort_pushdown.rs:
##########
@@ -0,0 +1,306 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Benchmark for sort pushdown optimization.
+//!
+//! Tests performance of sort elimination when files are non-overlapping and
+//! internally sorted (declared via `--sorted` / `WITH ORDER`).
+//!
+//! # Usage
+//!
+//! ```text
+//! # Prepare sorted TPCH lineitem data (SF=1)
+//! ./bench.sh data sort_pushdown
+//!
+//! # Baseline (no WITH ORDER, full SortExec)
+//! ./bench.sh run sort_pushdown
+//!
+//! # With sort elimination (WITH ORDER, SortExec removed)
+//! ./bench.sh run sort_pushdown_sorted
+//! ```
+//!
+//! # Reference Results
+//!
+//! Measured on 300k rows, 8 non-overlapping sorted parquet files, single partition,
+//! debug build (results vary by hardware; relative speedup is the key metric):

Review Comment:
   The reference-results section claims the benchmark was run on "8 non-overlapping sorted parquet files" / single partition, but `./bench.sh data sort_pushdown` currently just calls `data_tpch` (tpchgen-cli `--parts=1`) and `./bench.sh run sort_pushdown*` doesn't force `--partitions 1`. Either adjust the reference-results text to match the default dataset/run configuration, or update the bench script/data generation to actually produce multi-part sorted inputs and set partitions explicitly.
   ```suggestion
   //! Measured on approximately 300k rows using the default dataset prepared via
   //! `./bench.sh data sort_pushdown`, debug build (results vary by hardware; the
   //! relative speedup is the key metric):
   ```



##########
benchmarks/bench.sh:
##########
@@ -1070,6 +1084,22 @@ run_external_aggr() {
     debug_run $CARGO_COMMAND --bin external_aggr -- benchmark --partitions 4 --iterations 5 --path "${TPCH_DIR}" -o "${RESULTS_FILE}" ${QUERY_ARG}
 }
 
+# Runs the sort pushdown benchmark (without WITH ORDER)
+run_sort_pushdown() {
+    TPCH_DIR="${DATA_DIR}/tpch_sf1"
+    RESULTS_FILE="${RESULTS_DIR}/sort_pushdown.json"
+    echo "Running sort pushdown benchmark (no WITH ORDER)..."
+    debug_run $CARGO_COMMAND --bin dfbench -- sort-pushdown --iterations 5 --path "${TPCH_DIR}" -o "${RESULTS_FILE}" ${QUERY_ARG} ${LATENCY_ARG}
+}

Review Comment:
   The new sort-pushdown benchmark is described as single-partition in the module docs, but the bench runner doesn't pass `--partitions` here (so it will use DataFusion's default target partitions). If you want stable, comparable results and to match the documented plan shape (no SPM for single partition), consider passing `--partitions 1` (or documenting the expected/default partitions) in these runner functions.



##########
benchmarks/bench.sh:
##########
@@ -309,6 +313,10 @@ main() {
                     # same data as for tpch
                     data_tpch "1" "parquet"
                     ;;
+                sort_pushdown|sort_pushdown_sorted)
+                    # same data as for tpch
+                    data_tpch "1" "parquet"
+                    ;;

Review Comment:
   `sort_pushdown|sort_pushdown_sorted` reuses `data_tpch "1" "parquet"`, which currently generates parquet with `--parts=1`. That means the benchmark will typically run against a single lineitem parquet file, not multiple non-overlapping files as described. If the intent is to benchmark cross-file sort elimination, consider generating TPCH parquet with multiple parts for this benchmark (and documenting that the files are expected to be sorted by `l_orderkey`).



##########
benchmarks/src/sort_pushdown.rs:
##########
@@ -0,0 +1,306 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Benchmark for sort pushdown optimization.
+//!
+//! Tests performance of sort elimination when files are non-overlapping and
+//! internally sorted (declared via `--sorted` / `WITH ORDER`).
+//!
+//! # Usage
+//!
+//! ```text
+//! # Prepare sorted TPCH lineitem data (SF=1)
+//! ./bench.sh data sort_pushdown
+//!
+//! # Baseline (no WITH ORDER, full SortExec)
+//! ./bench.sh run sort_pushdown
+//!
+//! # With sort elimination (WITH ORDER, SortExec removed)
+//! ./bench.sh run sort_pushdown_sorted
+//! ```
+//!
+//! # Reference Results
+//!
+//! Measured on 300k rows, 8 non-overlapping sorted parquet files, single partition,
+//! debug build (results vary by hardware; relative speedup is the key metric):
+//!
+//! ```text
+//! Query | Description          | baseline (ms) | sort eliminated (ms) | speedup
+//! ------|----------------------|---------------|---------------------|--------
+//! Q1    | ASC full scan        |           159 |                  91 |  43%
+//! Q2    | ASC LIMIT 100        |            36 |                  12 |  67%
+//! Q3    | ASC full (wide, *)   |           487 |                 333 |  31%
+//! Q4    | ASC LIMIT 100 (wide) |           119 |                  30 |  74%
+//! ```
+//!
+//! Key observations:
+//! - **LIMIT queries benefit most** (67-74%): sort elimination + limit pushdown
+//!   means only the first few rows are read before stopping.
+//! - **Full scans** (31-43%): saving comes from eliminating the O(n log n) sort
+//!   step entirely.
+//! - **Wide projections** amplify the benefit: larger rows make sorting more
+//!   expensive, so eliminating it saves more.
+
+use clap::Args;
+use futures::StreamExt;
+use std::path::PathBuf;
+use std::sync::Arc;
+
+use datafusion::datasource::TableProvider;
+use datafusion::datasource::file_format::parquet::ParquetFormat;
+use datafusion::datasource::listing::{
+    ListingOptions, ListingTable, ListingTableConfig, ListingTableUrl,
+};
+use datafusion::error::Result;
+use datafusion::execution::SessionStateBuilder;
+use datafusion::physical_plan::display::DisplayableExecutionPlan;
+use datafusion::physical_plan::{displayable, execute_stream};
+use datafusion::prelude::*;
+use datafusion_common::DEFAULT_PARQUET_EXTENSION;
+use datafusion_common::instant::Instant;
+
+use crate::util::{BenchmarkRun, CommonOpt, QueryResult, print_memory_stats};
+
+#[derive(Debug, Args)]
+pub struct RunOpt {
+    /// Common options
+    #[command(flatten)]
+    common: CommonOpt,
+
+    /// Sort pushdown query number. If not specified, runs all queries
+    #[arg(short, long)]

Review Comment:
   `--query` accepts any `usize`, but the code later does `query_id - 1` / indexes into `QUERIES`, which will panic for `--query 0` or any value > 4. Consider adding a clap value range (1..=4) and/or validating `query_id` in `run()` and returning a DataFusion error instead of panicking.
   ```suggestion
       #[arg(short, long, value_parser = clap::value_parser!(usize).range(1..=4))]
   ```



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add sort pushdown benchmark and SLT tests [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #21213:
URL: https://github.com/apache/datafusion/pull/21213#issuecomment-4158598504

   run benchmark sort_pushdown


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add sort pushdown benchmark and SLT tests [datafusion]

Posted by "adriangbot (via GitHub)" <gi...@apache.org>.
adriangbot commented on PR #21213:
URL: https://github.com/apache/datafusion/pull/21213#issuecomment-4158607828

   🤖 Criterion benchmark running (GKE) | [trigger](https://github.com/apache/datafusion/pull/21213#issuecomment-4158598504)
   **Instance:** `c4a-highmem-16` (12 vCPU / 65 GiB) | `Linux bench-c4158598504-618-6l6p5 6.12.55+ #1 SMP Sun Feb  1 08:59:41 UTC 2026 aarch64 GNU/Linux`
   <details><summary>CPU Details (lscpu)</summary>
   
   ```
   Architecture:                            aarch64
   CPU op-mode(s):                          64-bit
   Byte Order:                              Little Endian
   CPU(s):                                  16
   On-line CPU(s) list:                     0-15
   Vendor ID:                               ARM
   Model name:                              Neoverse-V2
   Model:                                   1
   Thread(s) per core:                      1
   Core(s) per cluster:                     16
   Socket(s):                               -
   Cluster(s):                              1
   Stepping:                                r0p1
   BogoMIPS:                                2000.00
   Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
   L1d cache:                               1 MiB (16 instances)
   L1i cache:                               1 MiB (16 instances)
   L2 cache:                                32 MiB (16 instances)
   L3 cache:                                80 MiB (1 instance)
   NUMA node(s):                            1
   NUMA node0 CPU(s):                       0-15
   Vulnerability Gather data sampling:      Not affected
   Vulnerability Indirect target selection: Not affected
   Vulnerability Itlb multihit:             Not affected
   Vulnerability L1tf:                      Not affected
   Vulnerability Mds:                       Not affected
   Vulnerability Meltdown:                  Not affected
   Vulnerability Mmio stale data:           Not affected
   Vulnerability Reg file data sampling:    Not affected
   Vulnerability Retbleed:                  Not affected
   Vulnerability Spec rstack overflow:      Not affected
   Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
   Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
   Vulnerability Spectre v2:                Mitigation; CSV2, BHB
   Vulnerability Srbds:                     Not affected
   Vulnerability Tsa:                       Not affected
   Vulnerability Tsx async abort:           Not affected
   Vulnerability Vmscape:                   Not affected
   ```
   
   </details>
   
   Comparing feat/sort-pushdown-benchmark (7c58710105ca373b52bf1d687f2883feaffc244b) to 06b4791 (merge-base) [diff](https://github.com/apache/datafusion/compare/06b4791d7f04ca0a2c7e8258d436cde48a98b784..7c58710105ca373b52bf1d687f2883feaffc244b)
   BENCH_NAME=sort_pushdown
   BENCH_COMMAND=cargo bench --features=parquet --bench sort_pushdown
   BENCH_FILTER=
   Results will be posted here when complete
   
   ---
   [File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [PR] feat: add sort pushdown benchmark and SLT tests [datafusion]

Posted by "zhuqi-lucas (via GitHub)" <gi...@apache.org>.
zhuqi-lucas commented on code in PR #21213:
URL: https://github.com/apache/datafusion/pull/21213#discussion_r3007355239


##########
benchmarks/src/sort_pushdown.rs:
##########
@@ -0,0 +1,306 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Benchmark for sort pushdown optimization.
+//!
+//! Tests performance of sort elimination when files are non-overlapping and
+//! internally sorted (declared via `--sorted` / `WITH ORDER`).
+//!
+//! # Usage
+//!
+//! ```text
+//! # Prepare sorted TPCH lineitem data (SF=1)
+//! ./bench.sh data sort_pushdown
+//!
+//! # Baseline (no WITH ORDER, full SortExec)
+//! ./bench.sh run sort_pushdown
+//!
+//! # With sort elimination (WITH ORDER, SortExec removed)
+//! ./bench.sh run sort_pushdown_sorted
+//! ```
+//!
+//! # Reference Results
+//!
+//! Measured on 300k rows, 8 non-overlapping sorted parquet files, single partition,
+//! debug build (results vary by hardware; relative speedup is the key metric):
+//!
+//! ```text
+//! Query | Description          | baseline (ms) | sort eliminated (ms) | speedup
+//! ------|----------------------|---------------|---------------------|--------
+//! Q1    | ASC full scan        |           159 |                  91 |  43%
+//! Q2    | ASC LIMIT 100        |            36 |                  12 |  67%
+//! Q3    | ASC full (wide, *)   |           487 |                 333 |  31%
+//! Q4    | ASC LIMIT 100 (wide) |           119 |                  30 |  74%
+//! ```
+//!
+//! Key observations:
+//! - **LIMIT queries benefit most** (67-74%): sort elimination + limit pushdown
+//!   means only the first few rows are read before stopping.
+//! - **Full scans** (31-43%): saving comes from eliminating the O(n log n) sort
+//!   step entirely.
+//! - **Wide projections** amplify the benefit: larger rows make sorting more
+//!   expensive, so eliminating it saves more.
+
+use clap::Args;
+use futures::StreamExt;
+use std::path::PathBuf;
+use std::sync::Arc;
+
+use datafusion::datasource::TableProvider;
+use datafusion::datasource::file_format::parquet::ParquetFormat;
+use datafusion::datasource::listing::{
+    ListingOptions, ListingTable, ListingTableConfig, ListingTableUrl,
+};
+use datafusion::error::Result;
+use datafusion::execution::SessionStateBuilder;
+use datafusion::physical_plan::display::DisplayableExecutionPlan;
+use datafusion::physical_plan::{displayable, execute_stream};
+use datafusion::prelude::*;
+use datafusion_common::DEFAULT_PARQUET_EXTENSION;
+use datafusion_common::instant::Instant;
+
+use crate::util::{BenchmarkRun, CommonOpt, QueryResult, print_memory_stats};
+
+#[derive(Debug, Args)]
+pub struct RunOpt {
+    /// Common options
+    #[command(flatten)]
+    common: CommonOpt,
+
+    /// Sort pushdown query number. If not specified, runs all queries
+    #[arg(short, long)]
+    pub query: Option<usize>,
+
+    /// Path to data files (lineitem). Only parquet format is supported.
+    /// Data should be pre-sorted by l_orderkey ASC for meaningful results.
+    #[arg(required = true, short = 'p', long = "path")]
+    path: PathBuf,
+
+    /// Path to JSON benchmark result to be compared using `compare.py`
+    #[arg(short = 'o', long = "output")]
+    output_path: Option<PathBuf>,
+
+    /// Mark the first column (l_orderkey) as sorted via WITH ORDER.
+    /// When set, enables sort elimination for matching queries.
+    #[arg(short = 't', long = "sorted")]
+    sorted: bool,
+}
+
+pub const SORT_PUSHDOWN_QUERY_START_ID: usize = 1;
+pub const SORT_PUSHDOWN_QUERY_END_ID: usize = 4;
+
+impl RunOpt {
+    const TABLES: [&'static str; 1] = ["lineitem"];
+
+    /// Queries benchmarking sort elimination when files are non-overlapping
+    /// and internally sorted (WITH ORDER declared via `--sorted`).
+    ///
+    /// With `--sorted`: ParquetSource returns Exact, files are verified
+    /// non-overlapping by statistics → SortExec eliminated, no SPM needed
+    /// for single partition.
+    ///
+    /// Without `--sorted`: baseline with full SortExec.
+    const QUERIES: [&'static str; 4] = [

Review Comment:
   Good suggestion\! Moved queries to separate SQL files under `benchmarks/queries/sort_pushdown/` (q1.sql - q4.sql). Also added `--queries-path` arg so users can point to custom query files. The queries can now be run directly with datafusion-cli too.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org