[danking]

Use the logical-row prefix for compact PiecewiseSequence Take and sliced execution. A single contiguous request streams full selected pages directly into initialized output and reuses boundary-page scratch while preserving the sliced source validity. Compact ranges/repeats remain compact; multiple runs reuse touched decoded pages and can retain up to the full touched array.

Missing-prefix Take and sliced execution keep their existing implementations. No old-format fast path or unrelated Slice error fix is introduced. All public semantics originate in #10825; this PR only adds the indexed execution.

Fifth replacement for #10771, after #10828. All 63 PCO library tests and all-target/all-feature Clippy pass. Fresh current-head public file compatibility passed both directions against unchanged base33675. The final new-only matched resource run completed with all 26 oracles passing. Current GitHub CI and human review remain separate gates.

Existing CI and CodSpeed are reused; no new workflow. AI-assisted development with OpenAI Codex. Normal CI and the required human reviews remain necessary before merge; this PR remains draft.

Measured on an AMD EPYC 7B13 cloud VM: three OS-timer samples within one process per version, fixed workloads, resident buffers, no IO measurement. These are point estimates, not ABBA or confidence intervals; whole-process CPU/RSS includes fixtures and oracles. Final-stack comparisons do not isolate individual commits.

Explicit final-stack tradeoffs: nullable repeated Take rose from 52.43 to 67.71 ms (+29.1%, +15.28 ms); metadata-open/drop batches rose +33.2% dense/+31.6% nullable. These adverse results are retained for maintainer judgment, not hidden or auto-tuned away. No overall weighted speedup is claimed.

Final-stack near-full Slice was 1.26×/2.01× faster dense/nullable; compact four-range Take was 0.02674/0.03143 ms versus 42.85/68.20 ms original baseline. These gains avoid decode work, not measured disk reads.

CI correction: the existing compact stock-ticker expected size was updated 55,344→55,396 (+52 bytes with optional metadata), preserving its semantic roundtrip and default cases. The indexed-slice brace received pinned formatting. Fresh standard CI runs passed the source, platform, formatting and TSAN checks for these minimal corrections; no unrelated decoder fix is bundled.

Final CI audit: source/platform/formatting/TSAN checks have passed; remaining performance gates are CodSpeed and required human review. All PRs remain draft; no performance waiver or merge approval is claimed.

[codspeed]

Merging this PR will improve performance by 58.26%

4 warnings, 1 note

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ Unable to measure the execution of 2 benchmarks

These benchmarks measured no execution time, so their results are not comparable and count as unchanged. This usually happens when the compiler removes the code under test.

Prevent compiler optimizations

See affected benchmarks

⚠️ 18 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ Base and head ran on different runner types

Some benchmarks with significant performance changes were compared across different runner types, so the changes might not come from the code. Run base and head on the same runner type, or use CodSpeed Macro Runners for more stable results.

Learn more about Macro Runners

Open the report in CodSpeed to investigate

ℹ️ 40 benchmarks have no flamegraph

The flamegraph wasn't generated: the symbols of the profiling trace could not be resolved, because no information about the language frames was collected.

See affected benchmarks

⚡ 7 improved benchmarks
✅ 2458 untouched benchmarks
⏩ 12 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	CPU Simulation	selection[("ranges", true)]	2,074.4 µs	694.8 µs	×3
⚡	CPU Simulation	selection[("ranges", false)]	1,692.1 µs	697.8 µs	×2.4
⚡	CPU Simulation	selection[("near_full", true)]	540.1 ms	323.3 ms	+67.06%
⚡	CPU Simulation	chunked_dict_primitive_canonical_into[f64, (1000, 1000, 10)]	200.7 µs	146.4 µs	+37.1%
⚡	CPU Simulation	selection[("near_full", false)]	344.2 ms	287.7 ms	+19.61%
⚡	CPU Simulation	metadata_open_batch[false]	652.6 ms	577.7 ms	+12.97%
⚡	CPU Simulation	metadata_open_batch[true]	646.6 ms	582.5 ms	+11%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing dk/pco-selective-ranges (26de111) with dk/pco-selective-take (eea0ae2)}

Footnotes

1. 12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[danking]

@claude review

[github-actions]

Claude finished @danking's task in 1m 25s —— View job

---

Review: Stream PCO slices and compact PiecewiseSequence range takes

• Read the PR diff against origin/dk/pco-selective-take
• Read take.rs, access.rs, the Pco::execute / decompress paths and the PiecewiseSequenceArray constructor
• Post review

I found no correctness bugs. Descriptor overflow, out-of-bounds and declared-length checks all happen in validate_runs before any output is allocated. PiecewiseSequenceArray::try_new only checks structure (children must be non-nullable unsigned integers), so the malformed-descriptor cases in rejects_invalid_indices_and_descriptors do reach the Pco guards. Zero-page sliced sources also work: the legacy branch of AccessState::span returns an all-false span with values 0..0, which stream_range handles. My notes are mostly about performance and coverage.

1. The new sliced execute path may explain the CodSpeed scan regressions (likely, not confirmed)

array.rs:285: every sliced Pco now goes through stream_range instead of PcoData::decompress. Per page, stream_range → AccessState::span → resolve_impl does two things:

• It runs unsliced_validity().slice(rows).execute_mask(..) for that page.
• It looks up array.page_index() on every loop iteration (take.rs:691).

The old path ran execute_mask once for the whole array and decoded touched pages sequentially. For scans that produce many small slices over nullable data with small pages, the per-page validity execution plus up-front zeroing could cost more than before. That matches the ~10–14% regressions in test_scan_filter_to_arrow_array[int|int_float] and test_repeated_scan_filter[int|int_float]. Base and head ran on different runner types, so this needs a same-runner A/B before anyone acknowledges it on CodSpeed. If the A/B confirms it, two possible fixes:

• Gate the new path on slice width, e.g. use stream_range only when the slice is a small fraction of the page.
• Execute the visible validity mask once and hand it to span.

2. Multi-run compact stride-1 descriptors don't stream

take.rs:148 / gather: only a single stride == 1 run reaches stream_range. With two or more stride-1 runs (the common "take a few slices" shape), gather_typed uses AccessState<true>. Every decoded page stays alive for the whole operation, and AllTrue spans are copied out of the cached PrimitiveArray rather than decoded into the output. For descriptors that cover most of the array, peak memory is about the full decoded array plus the output. The PR body says "Full pages decode into final spans", which is only true for the single-run case. Possible fixes:

• Run each stride-1 run through the stream_range decode-into-output logic, reusing one scratch buffer and decoder.
• Or reword the PR body to match what the code does.

3. The compact fallback threshold measures the wrong cost

take.rs:138-145: random is compared with descriptor_bytes(random). On the compact path grouped is always None, so no per-request sort or resolved descriptors are ever allocated. The real cost of stride > 1 runs is:

• a resolve per element (a partition_point whenever the page changes);
• keeping every touched decoded page alive.

For example, one run with stride ≈ page_len touches and retains every page while random stays small. In that case canonicalizing would be strictly cheaper, but the check never falls back. Many length-1, stride-1 runs (effectively random indices written as a PiecewiseSequence) also bypass the check completely. Consider reusing the page-touch estimate from prefer_canonical_primitive, or at least counting runs.len() plus the distinct pages a strided run spans.

4. Minor

• take.rs:65-67: indices.as_opt::<PiecewiseSequence>() is called twice. Binding it once (let sequence = indices.as_opt::<…>(); let compact = sequence.is_some();) is cleaner.
• primitive_usizes goes through checked_index, so invalid lengths or multipliers report "Pco take index is negative or exceeds usize". The message is misleading for those children. They're unsigned by construction, so this only matters for values wider than usize.
• slice.rs: the new vortex_ensure! in SliceReduce is fine. stream_range repeats the same check, so you could drop one of them.
• array.rs:285: crate::take::slice puts a slice-decode entry point inside the take module. It might sit more naturally in slice.rs or a small stream.rs next to stream_range.

5. Test coverage

• No test runs a PiecewiseSequence take on an already-sliced Pco, so slice_start != 0 combined with compact runs is untested. mixed_fragments_narrow_slices_and_compact_repeats slices only before a primitive take, and runs the compact descriptor on the unsliced array. One rstest case such as encoded.slice(17..200)?.take(piecewise) would cover the array.slice_start() + start arithmetic in the stride 0, 1 and >1 branches.
• No test has a multi-run stride-1 descriptor that crosses pages with mixed validity (the Mask::Values branch of gather_typed). The first piecewise_ranges case gets close, but the lengths (15, 20, 3) all fall within a few small pages.
• streamed_slices_preserve_float_bits_and_validity compares element by element. CLAUDE.md asks for assert_arrays_eq!, but bit-exact NaN comparison is a fair reason to deviate here. A one-line comment saying so would help.

Checks

I didn't run any builds, clippy or tests: the review was read-only, and CLAUDE.md leaves verification to the user.

[danking]

@claude review

[github-actions]

Claude finished @danking's task in 1m 42s —— View job

---

Review: Stream PCO slices and compact PiecewiseSequence range takes

• Read the PR diff against origin/dk/pco-selective-take
• Read take.rs, access.rs and decoder.rs
• Check edge cases (malformed index, all-null spans, the padding at the end of gather_typed)
• Check the tests and post the review

I found no correctness bugs for well-formed files. Things I checked that hold up:

• Descriptor validation: validate_runs checks overflow, bounds and declared length before any output is allocated.
• Slice offsets: every branch adds slice_start() correctly. The new sliced_piecewise_coordinates_are_visible_rows test covers this, which closes the coverage gap from the last review.
• All-null spans: they never decode a page. Mask::slice goes through Mask::from_buffer, which turns an all-false slice into AllFalse, so Legacy's page: 0, values: 0..0 span takes the AllFalse arm.
• Loop bound: for valid files, 0..=page_count() is enough. Indexed spans stop at page row ends. Legacy needs at most one trailing all-null span.

The PR body now says plainly that multi-run selections keep every decoded page they touch, and the latest CodSpeed run shows no scan regressions. Points 1 and 2 of my previous review are resolved.

1. gather_typed pads a short output instead of failing (take.rs, end of gather_typed)

values.values.truncate(output_len);
values.values.push_n(T::default(), output_len - values.values.len());

For valid descriptors these two lines do nothing:

• Nullable output: the buffer is already zeroed to output_len.
• Non-nullable output: the loop pushes exactly output_len values, because validate_runs checked the total.

The output can only come up short when the page-count bound ends a stride-1 loop early, which means the optional index contents are malformed. What happens then depends on nullability:

• Non-nullable output (NULLABLE = false): the missing rows are filled with zeros and the call succeeds. That is silent wrong data.
• Nullable output (NULLABLE = true): valid holds fewer bits than output_len. PrimitiveArray::new then runs try_new(..).vortex_expect(..) and panics.

The PR body says index contents get no extra consistency scans, and that's fine. But this check costs nothing, and as written the same bad file gives zeros on one path and a panic on the other. I suggest replacing the padding with vortex_ensure!(values.len == output_len, "Pco page index does not cover the requested rows").

stream_range_mode has the same early-exit issue (it predates this PR, but Pco::execute now routes every sliced array through it). A vortex_ensure!(row >= stop, ..) after its loop would turn the same failure into an error instead of zero values. Fix this →

2. The compact fallback threshold still misses some costly shapes (carried over)

random only counts runs with stride > 1, so two shapes never fall back to canonicalizing:

• Wide strides: a single run with stride ≥ page_len touches every page. DecodedPages never evicts, so all of them stay decoded, plus one resolve per element.
• Many short runs: thousands of length-1, stride-1 runs (random indices written as a PiecewiseSequence) each pay a span call, and their touched pages are retained too.

The PR body accepts that touched pages are retained. Still, adding runs.len() to the estimate, or reusing the page-touch estimate from prefer_canonical_primitive, would cover both shapes cheaply. This is not blocking.

3. Test gaps

• Non-nullable compact path: every compact test uses a nullable source, so only gather_typed::<_, _, true> runs. The Output push/extend path for NULLABLE = false (with_capacity_in, then push/extend/repeat_n) is never exercised. One piecewise_ranges case over a non-nullable input would cover it.
• Legacy index: none of the new compact or sliced-execute tests use a legacy-format array, so gather_typed::<_, Legacy, _> is untested. That includes Legacy span's all-null branch and resolve::<true>, which builds the rank table. selective_take_tests.rs already has a legacy() helper and a #[values(false, true)] legacy_format pattern you could reuse.
• Mixed validity across pages: the stride-1 Mask::Values arm is reached by the first piecewise_ranges case. A run that crosses several pages with mixed validity would cover it more strongly.

4. Minor

• take.rs:73-75: indices.as_opt::<PiecewiseSequence>() is called twice. Binding it once and deriving compact from is_some() is cleaner.
• primitive_usizes reuses checked_index, so an oversized lengths or multipliers value reports "Pco take index is negative or exceeds usize". The message doesn't fit those children.
• array.rs: crate::take::slice is a slice-decode entry point living in the take module. It would sit more naturally in slice.rs.
• slice.rs: SliceReduce gets a new vortex_ensure!, and stream_range checks the same thing again. One of them is redundant.
• streamed_slices_preserve_float_bits_and_validity compares element by element, not with assert_arrays_eq!. Bit-exact NaN comparison is a fair reason, but a one-line comment saying so would help, per CLAUDE.md.

Checks

I didn't run builds, clippy or tests: this workflow is read-only, and CLAUDE.md leaves verification to the user.