You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2026/10/02 06:54:35 UTC

[PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

adriangb opened a new pull request, #11334:
URL: https://github.com/apache/arrow-rs/pull/11334

   Stacked on https://github.com/apache/arrow-rs/pull/10555. Review only the commits after ec3b0d2c50.
   
   # Which issue does this PR close?
   
   - Part of https://github.com/apache/arrow-rs/issues/6946.
   
   # Rationale for this change
   
   A caller that reads ahead asks the plan for its first range:
   
   ```rust
   let decoder = ParquetPushDecoderBuilder::new_with_metadata(metadata).build()?;
   let first = decoder.scan_plan().next(); // what the first fetch waits for
   ```
   
   Before the first range, the planner starts one cursor for each column of the stage. On a wide file this cost is O(columns), and each column used about 200 bytes of heap memory. This PR makes each column cost less time and less memory. The plan output (order and contents) does not change.
   
   Time of `scan_plan().next()`, one row group of 1,000 rows, every column, no selection (criterion, `push_decoder/scan_plan/first_range/all`):
   
   | Columns × pages | Before | After | Change |
   |---|---|---|---|
   | 100 × 10 | 2.08 µs | 0.84 µs | −59% |
   | 1,000 × 10 | 20.2 µs | 6.39 µs | −68% |
   | 10,000 × 10 | 179 µs | 63.9 µs | −64% |
   | 100,000 × 10 | 3.16 ms | 1.83 ms | −41% |
   | 1,000 × 100 | 17.7 µs | 6.77 µs | −62% |
   
   Heap memory of one `scan_plan().next()` call (counting global allocator, same files):
   
   | Columns | Allocations before | Allocations after | Bytes before | Bytes after |
   |---|---|---|---|---|
   | 100 | 13 | 7 | 22,420 | 4,788 |
   | 1,000 | 16 | 7 | 203,064 | 41,688 |
   | 10,000 | 20 | 7 | 2,419,104 | 410,688 |
   | 100,000 | 23 | 7 | 22,089,136 | 4,100,688 |
   
   At 100,000 columns the time is limited by memory reads of the column metadata and the offset index (about 1.2–1.4 ms for only these reads), so the gain there is smaller.
   
   # What changes are included in this PR?
   
   Terms:
   
   * **Cursor**: the state that gives the next ranges of one column chunk, in file order.
   * **Stage**: one decoding step of a row group (a predicate, or the output projection). The planner merges the cursors of a stage with a heap, by first row.
   
   | Commit | Change |
   |---|---|
   | bench | Adds a 100,000 column case to the `push_decoder/scan_plan` benchmark. |
   | perf | A cursor is now only its heap entry: the sort key of its next range, the position of the column, and which range of the column chunk is next (dictionary page, data page position, or whole chunk). When an entry is popped, the planner computes the end and kind of the range again from the page locations. A stage keeps the list of selected pages of a column only when the row selection removes pages. |
   
   Before, each cursor held a full `PlannedRange` for its next range (about 128 bytes), and the heap held a copy of its sort key. The planner also looked up the offset index two times for each column.
   
   `ColumnFetch` and `StageSchedule` are still the only source of the bytes that the decoder fetches. The planner does not copy decoder logic.
   
   # Are these changes tested?
   
   Yes, by the existing tests. The tests that compare the plan order, that check that the plan covers every later decoder request, and the lazy-plan test pass. The lazy-plan test now counts heap entries instead of cursors, because the cursor list is gone (one entry for each projected column, as before).
   
   Benchmarks (`cargo bench -p parquet --features arrow,async --bench push_decoder -- scan_plan`, baseline saved on the bench commit, Apple M-series laptop). `first_range` is the first `next()`, `whole_scan` is `count()`:
   
   | Case | first_range before | first_range after | whole_scan before | whole_scan after |
   |---|---|---|---|---|
   | all/100cols_10pages | 2.08 µs | 0.84 µs | 30.1 µs | 22.6 µs |
   | selection/100cols_10pages | 23.7 µs | 21.7 µs | 61.8 µs | 51.2 µs |
   | narrow/100cols_10pages | 574 ns | 349 ns | 2.57 µs | 1.42 µs |
   | all/1000cols_10pages | 20.2 µs | 6.39 µs | 383 µs | 320 µs |
   | selection/1000cols_10pages | 227 µs | 216 µs | 642 µs | 589 µs |
   | narrow/1000cols_10pages | 1.06 µs | 647 ns | 2.70 µs | 1.71 µs |
   | all/10000cols_10pages | 179 µs | 63.9 µs | 5.26 ms | 4.42 ms |
   | selection/10000cols_10pages | 2.27 ms | 2.19 ms | 7.77 ms | 7.05 ms |
   | narrow/10000cols_10pages | 3.50 µs | 3.81 µs | 5.44 µs | 4.95 µs |
   | all/100000cols_10pages | 3.16 ms | 1.83 ms | 67.2 ms | 58.1 ms |
   | selection/100000cols_10pages | 23.7 ms | 22.8 ms | 91.3 ms | 84.3 ms |
   | narrow/100000cols_10pages | 32.1 µs | 32.1 µs | 31.1 µs | 32.3 µs |
   | all/1000cols_100pages | 17.7 µs | 6.77 µs | 3.84 ms | 3.45 ms |
   | selection/1000cols_100pages | 325 µs | 326 µs | 749 µs | 711 µs |
   | narrow/1000cols_100pages | 837 ns | 676 ns | 20.1 µs | 12.3 µs |
   
   The `narrow` cases read 10 columns, so they do not depend on this change. Three alternating reruns of `narrow` at 10,000 and 100,000 columns gave equal or faster times after the change (the +9% in the table is noise on this machine).
   
   The `selection` cases stay slow: `ColumnFetch::new` calls `RowSelection::scan_ranges` for each column (about 6 allocations per column). This PR does not change that.
   
   # Are there any user-facing changes?
   
   No. The public API and the plan output do not change.
   
   🤖 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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

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

   🤖 Benchmark completed (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/11334#issuecomment-6087338497)
   
   **Target:** push&#95;decoder
   
   Comparing claude/scan-plan-lazy-cursors (766206b2b124787a3e47ae43e9d5d434830d1fa9) to c07655f (merge-base) [diff](https://github.com/apache/arrow-rs/compare/c07655f8e986834f2aebf9de46ba1c02ac58d32a..766206b2b124787a3e47ae43e9d5d434830d1fa9)
   
   <details><summary>Run configuration</summary>
   
   ```yaml
   run benchmark push_decoder
   shards: 1
   
   ```
   
   </details>
   
   <details><summary>Details</summary>
   
   ```
   group                                                              base                                   changed
   -----                                                              ----                                   -------
   push_decoder/1buf/100000ranges                                     1.00     49.1±0.38ms        ? ?/sec    1.04     51.2±0.51ms        ? ?/sec
   push_decoder/1buf/10000ranges                                      1.01      4.9±0.03ms        ? ?/sec    1.00      4.9±0.02ms        ? ?/sec
   push_decoder/1buf/1000ranges                                       1.00    487.0±3.15µs        ? ?/sec    1.00    488.2±1.62µs        ? ?/sec
   push_decoder/1buf/500000ranges                                     1.00    274.3±2.29ms        ? ?/sec    1.03    283.5±2.32ms        ? ?/sec
   push_decoder/Nbuf/100000ranges                                     1.00     60.3±0.32ms        ? ?/sec    1.06     63.8±1.12ms        ? ?/sec
   push_decoder/Nbuf/10000ranges                                      1.00      5.7±0.03ms        ? ?/sec    1.01      5.8±0.04ms        ? ?/sec
   push_decoder/Nbuf/1000ranges                                       1.00    568.8±1.97µs        ? ?/sec    1.02    577.6±2.44µs        ? ?/sec
   push_decoder/scan_plan/first_range/all/100000cols_10pages                                                 1.00  1767.4±78.12µs        ? ?/sec
   push_decoder/scan_plan/first_range/all/10000cols_10pages           2.70    378.6±1.40µs        ? ?/sec    1.00    140.4±1.23µs        ? ?/sec
   push_decoder/scan_plan/first_range/all/1000cols_100pages           2.86     36.5±0.14µs        ? ?/sec    1.00     12.7±0.02µs        ? ?/sec
   push_decoder/scan_plan/first_range/all/1000cols_10pages            2.87     36.2±0.17µs        ? ?/sec    1.00     12.6±0.02µs        ? ?/sec
   push_decoder/scan_plan/first_range/all/100cols_10pages             2.72      4.4±0.02µs        ? ?/sec    1.00   1619.3±1.96ns        ? ?/sec
   push_decoder/scan_plan/first_range/narrow/100000cols_10pages                                              1.00     53.9±0.05µs        ? ?/sec
   push_decoder/scan_plan/first_range/narrow/10000cols_10pages        1.00      6.2±0.01µs        ? ?/sec    1.66     10.4±0.21µs        ? ?/sec
   push_decoder/scan_plan/first_range/narrow/1000cols_100pages        1.28   1476.7±5.72ns        ? ?/sec    1.00   1154.5±2.27ns        ? ?/sec
   push_decoder/scan_plan/first_range/narrow/1000cols_10pages         1.28   1483.2±6.52ns        ? ?/sec    1.00   1154.4±2.42ns        ? ?/sec
   push_decoder/scan_plan/first_range/narrow/100cols_10pages          1.85   1023.4±6.94ns        ? ?/sec    1.00    554.4±3.82ns        ? ?/sec
   push_decoder/scan_plan/first_range/selection/100000cols_10pages                                           1.00     23.4±0.12ms        ? ?/sec
   push_decoder/scan_plan/first_range/selection/10000cols_10pages     1.06      2.4±0.00ms        ? ?/sec    1.00      2.2±0.00ms        ? ?/sec
   push_decoder/scan_plan/first_range/selection/1000cols_100pages     1.01    355.6±0.39µs        ? ?/sec    1.00    353.4±0.57µs        ? ?/sec
   push_decoder/scan_plan/first_range/selection/1000cols_10pages      1.06    233.2±0.14µs        ? ?/sec    1.00    219.1±0.25µs        ? ?/sec
   push_decoder/scan_plan/first_range/selection/100cols_10pages       1.07     24.4±0.02µs        ? ?/sec    1.00     22.8±0.02µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/all/100000cols_10pages                                                  1.00     71.5±0.68ms        ? ?/sec
   push_decoder/scan_plan/whole_scan/all/10000cols_10pages            1.16      7.3±0.01ms        ? ?/sec    1.00      6.3±0.01ms        ? ?/sec
   push_decoder/scan_plan/whole_scan/all/1000cols_100pages            1.14      6.6±0.01ms        ? ?/sec    1.00      5.8±0.01ms        ? ?/sec
   push_decoder/scan_plan/whole_scan/all/1000cols_10pages             1.15    636.4±1.35µs        ? ?/sec    1.00    552.9±1.16µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/all/100cols_10pages              1.40     36.9±0.12µs        ? ?/sec    1.00     26.3±0.16µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/narrow/100000cols_10pages                                               1.00     56.0±0.06µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/narrow/10000cols_10pages         1.13      9.1±0.11µs        ? ?/sec    1.00      8.0±0.02µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/narrow/1000cols_100pages         1.33     28.7±0.06µs        ? ?/sec    1.00     21.6±0.15µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/narrow/1000cols_10pages          1.35      4.3±0.01µs        ? ?/sec    1.00      3.2±0.02µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/narrow/100cols_10pages           1.46      3.8±0.01µs        ? ?/sec    1.00      2.6±0.02µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/selection/100000cols_10pages                                            1.00     97.6±0.26ms        ? ?/sec
   push_decoder/scan_plan/whole_scan/selection/10000cols_10pages      1.09      9.9±0.01ms        ? ?/sec    1.00      9.0±0.01ms        ? ?/sec
   push_decoder/scan_plan/whole_scan/selection/1000cols_100pages      1.04   1043.3±1.99µs        ? ?/sec    1.00   1007.5±1.95µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/selection/1000cols_10pages       1.06    898.6±1.94µs        ? ?/sec    1.00    847.1±1.38µs        ? ?/sec
   push_decoder/scan_plan/whole_scan/selection/100cols_10pages        1.14     62.9±0.36µs        ? ?/sec    1.00     55.1±0.17µs        ? ?/sec
   
   ```
   
   </details>
   
   <details><summary>Per-runner information</summary>
   
   <details><summary>push&#95;decoder — shard 1/1</summary>
   
   **Node:** gk3-benchmark-cluster-nap-1s1srbox-e6d80935-rrt4
   
   **Instance:** c4a-highmem-16 (12 vCPU / 65 GiB)
   
   **uname:**
   
   ```text
   Linux bench-c6087338497-3253-h7gs6 6.12.94+ #1 SMP Fri Aug 21 08:00:16 UTC 2026 aarch64 GNU/Linux
   ```
   
   **BENCH_COMMAND:**
   
   ```sh
   cargo bench --features=arrow,async,test_common,experimental,object_store --bench push_decoder
   ```
   
   <details><summary>CPU Details (lscpu)</summary>
   
   ```text
   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>
   
   <details><summary>Resource Usage</summary>
   
   **base (merge-base)**
   | Metric | Value |
   |--------|-------|
   | Wall time | 345.1s |
   | Peak memory | 3.1 GiB |
   | Avg memory | 2.4 GiB |
   | CPU user | 341.4s |
   | CPU sys | 1.2s |
   | Peak spill | 0 B |
   
   **branch**
   | Metric | Value |
   |--------|-------|
   | Wall time | 415.1s |
   | Peak memory | 3.7 GiB |
   | Avg memory | 2.9 GiB |
   | CPU user | 412.4s |
   | CPU sys | 1.1s |
   | Peak spill | 0 B |
   
   
   </details>
   
   </details>
   
   </details>
   
   ---
   [File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
   
   <!-- benchmark-request:6087338497:finish:6e56613c909ad85d50d5096b30d448c9 -->


-- 
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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

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

   Rebased!


-- 
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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

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

   Thanks @alamb, rebased 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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #11334:
URL: https://github.com/apache/arrow-rs/pull/11334


-- 
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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #11334:
URL: https://github.com/apache/arrow-rs/pull/11334#discussion_r4233737211


##########
parquet/src/arrow/push_decoder/scan_plan/mod.rs:
##########
@@ -53,9 +54,11 @@ pub struct PlannedRange {
     /// What the range contains.
     pub kind: PageKind,
     /// First planned row that this range serves, counted from the start of
-    /// the plan. Used only to order the plan.
+    /// the plan. The tests use it to check the order of the plan.
+    #[cfg_attr(not(test), expect(dead_code))]

Review Comment:
   nice



-- 
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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

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

   run benchmark push_decoder


-- 
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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #11334:
URL: https://github.com/apache/arrow-rs/pull/11334#issuecomment-6065584205

   I think this PR can be rebased 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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

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

   🤖 Benchmark starting (GKE) | [trigger](https://github.com/apache/arrow-rs/pull/11334#issuecomment-6087338497)
   
   **Target:** push&#95;decoder
   
   **Sharding factor: 1** (1 workers).
   
   Comparing claude/scan-plan-lazy-cursors (766206b2b124787a3e47ae43e9d5d434830d1fa9) to c07655f (merge-base) [diff](https://github.com/apache/arrow-rs/compare/c07655f8e986834f2aebf9de46ba1c02ac58d32a..766206b2b124787a3e47ae43e9d5d434830d1fa9)
   
   <details><summary>Run configuration</summary>
   
   ```yaml
   run benchmark push_decoder
   shards: 1
   
   ```
   
   </details>
   
   Results will be posted when all workers finish.
   
   ---
   [File an issue](https://github.com/adriangb/datafusion-benchmarking/issues) against this benchmark runner
   
   <!-- benchmark-request:6087338497:start:150c734ea48db68ca05cf25fdcf82f57 -->


-- 
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@arrow.apache.org

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


Re: [PR] perf(parquet): store a ScanPlan cursor as one heap entry [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #11334:
URL: https://github.com/apache/arrow-rs/pull/11334#discussion_r4233216625


##########
parquet/src/arrow/push_decoder/scan_plan/mod.rs:
##########
@@ -506,58 +493,75 @@ struct RowGroupContext {
     expanded_selection: Option<RowSelection>,
     /// What each decoding stage fetches.
     stages: Arc<StageSchedule>,
+    /// The file metadata, for the byte ranges of the column chunks.
+    metadata: Arc<ParquetMetaData>,
     /// The file's page index, if it has offset indexes.
     page_index: Option<Arc<dyn PageIndexProvider>>,
 }
 
 /// The merge state of one stage.
+///
+/// A cursor of a column chunk is one heap entry: the sort key of its next
+/// range, the column and the position of that range. The other parts of the
+/// range are computed again when the entry is popped. Thus a stage stores
+/// no per-column state other than its heap entry, unless the selection
+/// removes pages.
 #[derive(Debug, Clone)]
 struct StageRanges {
     stage: ScanStage,
     conditional: bool,
-    cursors: Vec<ColumnCursor>,
-    /// The sort key of the next range of each cursor that has one, smallest
-    /// first.
-    heap: BinaryHeap<Reverse<(RangeOrder, usize)>>,
+    /// Leaf column indexes, in file order. A [`HeapEntry`] refers to a
+    /// column by its position in this list.
+    columns: Vec<usize>,
+    /// For each column in `columns`, the indexes of the data pages that the
+    /// decoder reads, if the selection removes pages. `None` if no column of
+    /// the stage has such a list.
+    selected: Option<Vec<Option<Vec<usize>>>>,
+    /// The next range of each cursor that has one, smallest first.
+    heap: BinaryHeap<Reverse<HeapEntry>>,
 }
 
 /// How the heap orders the next ranges of different column chunks: by first
 /// row, then more rows first (a dictionary page), then by file offset. The
 /// ranges of one column chunk keep file order, also a page with no planned
 /// rows that the predicate cache reads.
+///
+/// The key is `(first row, Reverse(last row), range start)`.
 type RangeOrder = (u64, Reverse<u64>, u64);
 
-fn range_order(range: &PlannedRange) -> RangeOrder {
-    (range.first_row, Reverse(range.last_row), range.range.start)
-}
+/// A heap entry: the sort key of the next range of a cursor, the position
+/// of the column in [`StageRanges::columns`], and the [`HeadPosition`] of
+/// that range. The column position breaks ties, so ranges with equal keys
+/// stay in file order of their columns.
+type HeapEntry = (RangeOrder, u32, HeadPosition);
 
-/// The ranges of one column chunk that are not yet returned.
-#[derive(Debug, Clone)]
-struct ColumnCursor {
-    column_idx: usize,
-    /// The next range of this column chunk.
-    head: Option<PlannedRange>,
-    /// Data pages the decoder reads.
-    pages: PageSet,
-    /// Position in `pages` of the next data page after `head`.
-    next_page: usize,
+/// Which range of a column chunk is next: [`CHUNK`], [`DICTIONARY`], or the
+/// position of a data page in the pages that the decoder reads.
+type HeadPosition = u32;
+
+/// The next range is the whole column chunk. The column has no offset index.
+const CHUNK: HeadPosition = u32::MAX;
+
+/// The next range is the dictionary page.
+const DICTIONARY: HeadPosition = u32::MAX - 1;
+
+/// Converts a column or page position to a heap entry field.
+fn position(position: usize) -> u32 {
+    u32::try_from(position).expect("fewer than u32::MAX - 1 columns and pages")
 }
 
 /// The data pages of a column chunk that the decoder reads, in page order.
-#[derive(Debug, Clone)]
-enum PageSet {
-    /// No offset index, so the column chunk is one range.
-    None,
-    /// Every page: there is no selection.
+#[derive(Debug, Clone, Copy)]
+enum PageSet<'a> {
+    /// Every page of the offset index, which has this many pages.
     All(usize),
     /// The pages at these indexes in the offset index.
-    Selected(Vec<usize>),
+    Selected(&'a [usize]),

Review Comment:
   nice



-- 
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@arrow.apache.org

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