perf(substrait): avoid quadratic field lookup when consuming ReadRel by ole-baranov · Pull Request #26190 · apache/datafusion

ole-baranov · 2026-10-10T19:02:10Z

Which issue does this PR close?

There is no issue.

Rationale for this change

Substrait ReadRel consumption performed a linear scan over the table schema for every field, leading to O(m × n) lookups when scanning wide tables. This commit introduces a hash‑map lookup (unqualified_name_index) to map column names to indices once per schema, reducing the complexity to O(m + n). The change also adds a benchmark to expose the performance regression and confirm the improvement.

What changes are included in this PR?

Added a new dev‑dependency criterion and a benchmark group consume_wide_read in Cargo.toml and Cargo.lock.
Implemented a unqualified_name_index helper in utils.rs to cache field indices.
Updated read_rel.rs and apply_projection to use the new index map.
Added benchmark source file benches/consume_wide_read.rs to measure read performance across varying table widths.
Adjusted Cargo metadata to include the new benchmark target.

What is the testing strategy for this PR?

Run the full test suite with cargo test --profile ci --workspace --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption.
Run the new benchmark with cargo bench --bench consume_wide_read.
No existing unit tests were modified; the benchmark validates that the performance improvement does not regress.

Are there any user‑facing changes?

No. The change is purely an internal performance improvement and an additional benchmark; there are no API modifications.

bvolpato

LGTM, thanks for the improvement. The fast path preserves the existing lookup fallback for missing and duplicate names. Substrait checks, Clippy, and the wide-read benchmark smoke run pass locally.

askalt

The patch looks good to me.

kosiew · 2026-11-03T07:50:51Z

@ole-baranov
Thank you for your contribution.

Can you amend the description to follow .github/pull_request_template.md?

ole-baranov · 2026-11-03T08:08:56Z

@ole-baranov Thank you for your contribution.

Can you amend the description to follow .github/pull_request_template.md?

Haven’t seen it. Thanks.

codecov-commenter · 2026-11-05T09:18:03Z

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.80%. Comparing base (75ddead) to head (d97794b).
⚠️ Report is 390 commits behind head on main.

Files with missing lines	Patch %	Lines
...ubstrait/src/logical_plan/consumer/rel/read_rel.rs	80.00%	1 Missing ⚠️
...usion/substrait/src/logical_plan/consumer/utils.rs	92.30%	0 Missing and 1 partial ⚠️

@@            Coverage Diff             @@
##             main   #26190      +/-   ##
==========================================
- Coverage   82.80%   82.80%   -0.01%     
==========================================
  Files        1149     1149              
  Lines      452205   452218      +13     
  Branches   452205   452218      +13     
==========================================
+ Hits       374458   374464       +6     
- Misses      54950    54953       +3     
- Partials    22797    22801       +4

Files with missing lines	Coverage Δ
...ubstrait/src/logical_plan/consumer/rel/read_rel.rs	`72.00% <80.00%> (-0.23%)`	⬇️
...usion/substrait/src/logical_plan/consumer/utils.rs	`86.00% <92.30%> (+0.09%)`	⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kosiew · 2026-11-05T09:42:42Z

@ole-baranov
Can you share your benchmark figures?

My tests do not show improved performance:

group                      baseline                               changed
-----                      --------                               -------
consume_wide_read/100      1.00     17.6±0.62µs        ? ?/sec    1.04     18.3±0.25µs        ? ?/sec
consume_wide_read/1000     1.00    219.1±6.51µs        ? ?/sec    1.03    225.0±4.00µs        ? ?/sec
consume_wide_read/10000    1.00      2.6±0.11ms        ? ?/sec    1.03      2.7±0.05ms        ? ?/sec

ole-baranov · 2026-11-05T12:46:29Z

@ole-baranov Can you share your benchmark figures?

My tests do not show improved performance:

group                      baseline                               changed
-----                      --------                               -------
consume_wide_read/100      1.00     17.6±0.62µs        ? ?/sec    1.04     18.3±0.25µs        ? ?/sec
consume_wide_read/1000     1.00    219.1±6.51µs        ? ?/sec    1.03    225.0±4.00µs        ? ?/sec
consume_wide_read/10000    1.00      2.6±0.11ms        ? ?/sec    1.03      2.7±0.05ms        ? ?/sec

I separated the commits to make testing more convenient. You can now run tests on these two commits separately.
On my PC, I got these results:

before the change:

consume_wide_read/100   time:   [48.976 µs 49.349 µs 49.802 µs]
consume_wide_read/1000  time:   [2.7695 ms 2.7868 ms 2.8056 ms]
consume_wide_read/10000 time:   [239.59 ms 241.71 ms 243.91 ms]

after the one:

consume_wide_read/100   time:   [31.077 µs 31.262 µs 31.494 µs]
consume_wide_read/1000  time:   [465.22 µs 469.76 µs 475.38 µs]
consume_wide_read/10000 time:   [5.3852 ms 5.4306 ms 5.4836 ms]

With critcmp got this:

group                      feature                                main
-----                      -------                                ----
consume_wide_read/100      1.00     30.8±0.89µs        ? ?/sec    1.64     50.4±1.57µs        ? ?/sec
consume_wide_read/1000     1.00   462.8±20.91µs        ? ?/sec    6.23      2.9±0.08ms        ? ?/sec
consume_wide_read/10000    1.00      5.4±0.20ms        ? ?/sec    48.43   262.7±3.82ms        ? ?/sec

Measure Substrait consumption of `SELECT c0` over a table with 100, 1000 and 10000 columns, to track the cost of ReadRel schema handling on wide tables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

`ensure_schema_compatibility` and `apply_projection` looked up every field of the Substrait base schema with a linear scan of the table schema, making consumption of a read over a wide table O(m*n). Build a name->index map of the table schema once instead. Missing or duplicated names fall back to the existing DFSchema lookups, so errors and disambiguation are unchanged. Consume time for `SELECT c0` over a table with N columns (`consume_wide_read` benchmark): columns before after 100 58 us 25.5 us 1000 2.33 ms 276 us 10000 211 ms 3.16 ms Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kosiew · 2026-11-06T04:42:47Z

hi @ole-baranov,

I added required-feature protoc in #27153 because of adriangb/datafusion-benchmarking#49

Here are the runs:

run 1

group                      HEAD                                   bench2-26190
-----                      ----                                   ------------
consume_wide_read/100      1.00     32.1±0.24µs        ? ?/sec    1.03     33.2±0.31µs        ? ?/sec
consume_wide_read/1000     1.00    437.5±3.01µs        ? ?/sec    1.03    449.1±3.30µs        ? ?/sec
consume_wide_read/10000    1.00      5.2±0.02ms        ? ?/sec    1.05      5.4±0.03ms        ? ?/sec

run 2

group                      HEAD                                   bench2-26190
-----                      ----                                   ------------
consume_wide_read/100      1.00     32.0±0.50µs        ? ?/sec    1.03     33.0±0.09µs        ? ?/sec
consume_wide_read/1000     1.00    437.0±1.84µs        ? ?/sec    1.02    445.1±1.97µs        ? ?/sec
consume_wide_read/10000    1.00      5.1±0.02ms        ? ?/sec    1.05      5.4±0.02ms        ? ?/sec

Both show slightly slower performance.

ole-baranov · 2026-11-06T06:04:43Z

hi @ole-baranov,

I added required-feature protoc in #27153 because of adriangb/datafusion-benchmarking#49

Here are the runs:

run 1

group                      HEAD                                   bench2-26190
-----                      ----                                   ------------
consume_wide_read/100      1.00     32.1±0.24µs        ? ?/sec    1.03     33.2±0.31µs        ? ?/sec
consume_wide_read/1000     1.00    437.5±3.01µs        ? ?/sec    1.03    449.1±3.30µs        ? ?/sec
consume_wide_read/10000    1.00      5.2±0.02ms        ? ?/sec    1.05      5.4±0.03ms        ? ?/sec

run 2

group                      HEAD                                   bench2-26190
-----                      ----                                   ------------
consume_wide_read/100      1.00     32.0±0.50µs        ? ?/sec    1.03     33.0±0.09µs        ? ?/sec
consume_wide_read/1000     1.00    437.0±1.84µs        ? ?/sec    1.02    445.1±1.97µs        ? ?/sec
consume_wide_read/10000    1.00      5.1±0.02ms        ? ?/sec    1.05      5.4±0.02ms        ? ?/sec

Both show slightly slower performance.

hi @kosiew !
It is possibly a tooling issue. Yesterday, I ran critcmp on different criterion runs, and it showed results just like yours. My problem was that I forgot to use the --save-baseline argument for the criterion run.

Could you share the commands you used to run the tests?

github-actions Bot added the substrait Changes to the substrait crate label Oct 10, 2026

bvolpato approved these changes Oct 11, 2026

View reviewed changes

askalt approved these changes Oct 12, 2026

View reviewed changes

ole-baranov force-pushed the feature/substrait-schema-check-index branch from d97794b to 9d075fc Compare November 5, 2026 12:01

ole-baranov and others added 2 commits November 5, 2026 17:45

bench(substrait): add consume_wide_read benchmark

1b0f202

Measure Substrait consumption of `SELECT c0` over a table with 100, 1000 and 10000 columns, to track the cost of ReadRel schema handling on wide tables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ole-baranov force-pushed the feature/substrait-schema-check-index branch from 9d075fc to 9fafbd3 Compare November 5, 2026 14:45

Navigation Menu

Repository navigation

perf(substrait): avoid quadratic field lookup when consuming ReadRel - #26190

perf(substrait): avoid quadratic field lookup when consuming ReadRel#26190
ole-baranov wants to merge 2 commits into
apache:mainfrom
tarantool:feature/substrait-schema-check-index

ole-baranov commented Oct 10, 2026 •

edited

Loading

Uh oh!

bvolpato left a comment

Uh oh!

askalt left a comment

Uh oh!

kosiew commented Nov 3, 2026

Uh oh!

ole-baranov commented Nov 3, 2026

Uh oh!

codecov-commenter commented Nov 5, 2026

Uh oh!

kosiew commented Nov 5, 2026

Uh oh!

ole-baranov commented Nov 5, 2026 •

edited

Loading

Uh oh!

kosiew commented Nov 6, 2026

Uh oh!

ole-baranov commented Nov 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

ole-baranov commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

What is the testing strategy for this PR?

Are there any user‑facing changes?

Uh oh!

bvolpato left a comment

Choose a reason for hiding this comment

Uh oh!

askalt left a comment

Choose a reason for hiding this comment

Uh oh!

kosiew commented Nov 3, 2026

Uh oh!

ole-baranov commented Nov 3, 2026

Uh oh!

codecov-commenter commented Nov 5, 2026

Codecov Report

Uh oh!

kosiew commented Nov 5, 2026

Uh oh!

ole-baranov commented Nov 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kosiew commented Nov 6, 2026

Uh oh!

ole-baranov commented Nov 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ole-baranov commented Oct 10, 2026 •

edited

Loading

ole-baranov commented Nov 5, 2026 •

edited

Loading