perf: Optimize `array_position` for scalar needle by neilconway · Pull Request #20532 · apache/datafusion

neilconway · 2026-02-24T18:03:39Z

Which issue does this PR close?

Closes Improve performance of array_position with scalar needle #20530

Rationale for this change

The previous implementation of array_position used compare_element_to_list for every input row. When the needle is a scalar (quite common), we can do much better by searching over the entire flat haystack values array with a single call to arrow_ord::cmp::not_distinct. We can then iterate over the resulting set bits to determine per-row results.

This is ~5-10x faster than the previous implementation for typical inputs.

What changes are included in this PR?

Implement new fast path for array_position with scalar needle
Improve docs for array_position
Don't use internal_err to report a user-visible error

Are these changes tested?

Yes, and benchmarked. Additional tests added in a separate PR (#20531)

Are there any user-facing changes?

No.

neilconway · 2026-02-24T18:04:58Z

Benchmarks:

  ┌────────────────┬─────────┬─────────┬─────────────┐
  │   Benchmark    │ Before  │  After  │ Improvement │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ found_once/10  │ 1.73 ms │ 55.6 µs │ -96.6%      │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ found_many/10  │ 1.61 ms │ 68.5 µs │ -95.5%      │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ not_found/10   │ 1.80 ms │ 47.0 µs │ -97.3%      │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ found_once/100 │ 1.90 ms │ 156 µs  │ -91.2%      │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ found_many/100 │ 2.05 ms │ 459 µs  │ -76.5%      │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ not_found/100  │ 2.13 ms │ 144 µs  │ -93.2%      │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ found_once/500 │ 2.39 ms │ 586 µs  │ -75.4%      │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ found_many/500 │ 2.68 ms │ 2.34 ms │ -12.8%      │
  ├────────────────┼─────────┼─────────┼─────────────┤
  │ not_found/500  │ 4.12 ms │ 585 µs  │ -85.8%      │
  └────────────────┴─────────┴─────────┴─────────────┘

found_many/500 is not surprising as a worst-case: the bitmap we get back from arrow_ord::cmp::not_distinct is dense, so we aren't able to save much work. But at least it's still a win.

neilconway · 2026-02-24T18:05:47Z

Note that this PR includes the commit from #20531; happy to remove that or we can land that change as part of this PR and close #20531, whatever folks prefer.

comphead · 2026-02-26T16:27:45Z

+                let arr_from = resolve_start_from(third_arg.first(), num_rows)?;
+
+                let result = match haystack.data_type() {
+                    List(_) => {


should we support FixedSizeList?

We could. The current code works with FixedSizeList because the input list will be coerced (relatively cheap but not free). We could consider adding native support for FixedSizeList -- we'd also be able to avoid walking the offsets buffer to find matches. That would be slightly faster, although I think it would only be a small win.

I'm inclined to skip it to avoid the code duplication, but maybe I'm underestimating the perf win or how widespread FixedSizeList values are in real-world workloads. wdyt?

it can be done later, basically it should work fine until the function gets called without planner, like Comet for example

comphead

Thanks @neilconway

neilconway · 2026-02-26T18:42:14Z

@comphead Awesome, thanks for the review!

## Which issue does this PR close? N/A ## Rationale for this change In #20374, `array_has` with a scalar needle was optimized to reconstruct matches more efficiently. Unfortunately, that code was incorrect for sliced arrays: `values()` returns the entire value buffer (including elements outside the visible slice), so we need to skip the corresponding indexes in the result bitmap. We could fix this by just skipping indexes, but it seems more robust and efficient to arrange to not compare the needle against elements outside the visible range in the first place. `array_position` has a similar behavior (introduced in #20532): it didn't have the buggy behavior, but it still did extra work for sliced arrays by comparing against elements outside the visible range. Benchmarking the revised code, there is no performance regression for unsliced arrays. ## What changes are included in this PR? * Fix `array_has` bug for sliced arrays with scalar needle * Improve `array_has` and `array_position` to not compare against elements outside the visible range of a sliced array * Add unit test for `array_has` bug * Add unit test to increase confidence in `array_position` behavior for sliced arrays ## Are these changes tested? Yes. ## Are there any user-facing changes? No.

) ## Which issue does this PR close? N/A ## Rationale for this change In apache#20374, `array_has` with a scalar needle was optimized to reconstruct matches more efficiently. Unfortunately, that code was incorrect for sliced arrays: `values()` returns the entire value buffer (including elements outside the visible slice), so we need to skip the corresponding indexes in the result bitmap. We could fix this by just skipping indexes, but it seems more robust and efficient to arrange to not compare the needle against elements outside the visible range in the first place. `array_position` has a similar behavior (introduced in apache#20532): it didn't have the buggy behavior, but it still did extra work for sliced arrays by comparing against elements outside the visible range. Benchmarking the revised code, there is no performance regression for unsliced arrays. ## What changes are included in this PR? * Fix `array_has` bug for sliced arrays with scalar needle * Improve `array_has` and `array_position` to not compare against elements outside the visible range of a sliced array * Add unit test for `array_has` bug * Add unit test to increase confidence in `array_position` behavior for sliced arrays ## Are these changes tested? Yes. ## Are there any user-facing changes? No.

## Which issue does this PR close? - Closes #20769. ## Rationale for this change `array_positions` previously compared the needle against each row's sub-array individually. When the needle is a scalar (the common case), we can do a single bulk `arrow_ord::cmp::not_distinct` comparison against the entire flat values buffer and then walk the result bitmap, which is significantly faster: the speedup on the `array_positions()` microbenchmarks ranges from 5x to 40x, depending on the size of the array. The same pattern has already been applied to `array_position` (#20532), and previously to other array UDFs. ## What changes are included in this PR? - Add benchmarks for `array_positions`. - Implement bulk-comparison optimization - Refactor `array_position`'s existing fast path slightly for consistency - Code cleanup to use "haystack" and "needle" consistently, not vague terms like "list_array" and "element" - Add unit tests for `array_positions` with sliced ListArrays, for peace of mind - Add unit tests for sliced lists and sliced lists with nulls for the new `array_positions` fast path. ## Are these changes tested? Yes. ## Are there any user-facing changes? No. ## AI usage Multiple AI tools were used to iterate on this PR. I have reviewed and understand the resulting code. --------- Co-authored-by: Oleks V <comphead@users.noreply.github.com>

## Which issue does this PR close? - Closes apache#20530 ## Rationale for this change The previous implementation of `array_position` used `compare_element_to_list` for every input row. When the needle is a scalar (quite common), we can do much better by searching over the entire flat haystack values array with a single call to `arrow_ord::cmp::not_distinct`. We can then iterate over the resulting set bits to determine per-row results. This is ~5-10x faster than the previous implementation for typical inputs. ## What changes are included in this PR? * Implement new fast path for `array_position` with scalar needle * Improve docs for `array_position` * Don't use `internal_err` to report a user-visible error ## Are these changes tested? Yes, and benchmarked. Additional tests added in a separate PR (apache#20531) ## Are there any user-facing changes? No.

) ## Which issue does this PR close? N/A ## Rationale for this change In apache#20374, `array_has` with a scalar needle was optimized to reconstruct matches more efficiently. Unfortunately, that code was incorrect for sliced arrays: `values()` returns the entire value buffer (including elements outside the visible slice), so we need to skip the corresponding indexes in the result bitmap. We could fix this by just skipping indexes, but it seems more robust and efficient to arrange to not compare the needle against elements outside the visible range in the first place. `array_position` has a similar behavior (introduced in apache#20532): it didn't have the buggy behavior, but it still did extra work for sliced arrays by comparing against elements outside the visible range. Benchmarking the revised code, there is no performance regression for unsliced arrays. ## What changes are included in this PR? * Fix `array_has` bug for sliced arrays with scalar needle * Improve `array_has` and `array_position` to not compare against elements outside the visible range of a sliced array * Add unit test for `array_has` bug * Add unit test to increase confidence in `array_position` behavior for sliced arrays ## Are these changes tested? Yes. ## Are there any user-facing changes? No.

## Which issue does this PR close? - Closes apache#20769. ## Rationale for this change `array_positions` previously compared the needle against each row's sub-array individually. When the needle is a scalar (the common case), we can do a single bulk `arrow_ord::cmp::not_distinct` comparison against the entire flat values buffer and then walk the result bitmap, which is significantly faster: the speedup on the `array_positions()` microbenchmarks ranges from 5x to 40x, depending on the size of the array. The same pattern has already been applied to `array_position` (apache#20532), and previously to other array UDFs. ## What changes are included in this PR? - Add benchmarks for `array_positions`. - Implement bulk-comparison optimization - Refactor `array_position`'s existing fast path slightly for consistency - Code cleanup to use "haystack" and "needle" consistently, not vague terms like "list_array" and "element" - Add unit tests for `array_positions` with sliced ListArrays, for peace of mind - Add unit tests for sliced lists and sliced lists with nulls for the new `array_positions` fast path. ## Are these changes tested? Yes. ## Are there any user-facing changes? No. ## AI usage Multiple AI tools were used to iterate on this PR. I have reviewed and understand the resulting code. --------- Co-authored-by: Oleks V <comphead@users.noreply.github.com>

neilconway added 2 commits February 24, 2026 12:42

Add tests and benchmarks

265f089

Optimize array_position

74fb538

github-actions Bot added documentation Improvements or additions to documentation sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Feb 24, 2026

comphead mentioned this pull request Feb 26, 2026

feat: add support for array_position expression apache/datafusion-comet#3172

Merged

comphead reviewed Feb 26, 2026

View reviewed changes

Comment thread datafusion/functions-nested/src/position.rs

comphead reviewed Feb 26, 2026

View reviewed changes

Comment thread datafusion/functions-nested/src/position.rs Outdated

neilconway added 2 commits February 26, 2026 12:16

Optimize parsing of start_from, per review

b7b6d16

Merge start_from bounds checking into main loop

e4bb7c5

comphead approved these changes Feb 26, 2026

View reviewed changes

comphead added this pull request to the merge queue Feb 26, 2026

Merged via the queue into apache:main with commit d6fb360 Feb 26, 2026
32 checks passed

neilconway deleted the neilc/optimize-array-position branch February 26, 2026 18:42

This was referenced Feb 26, 2026

chore: Add benchmarks, tests for array_position #20531

Closed

fix: Fix bug in array_has scalar path with sliced arrays #20677

Merged

neilconway mentioned this pull request Mar 7, 2026

perf: Optimize array_positions() for scalar needle #20770

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: Optimize `array_position` for scalar needle#20532

perf: Optimize `array_position` for scalar needle#20532
comphead merged 4 commits intoapache:mainfrom
neilconway:neilc/optimize-array-position

neilconway commented Feb 24, 2026

Uh oh!

neilconway commented Feb 24, 2026

Uh oh!

neilconway commented Feb 24, 2026

Uh oh!

comphead Feb 26, 2026

Uh oh!

neilconway Feb 26, 2026 •

edited

Loading

Uh oh!

comphead Feb 26, 2026

Uh oh!

Uh oh!

Uh oh!

comphead left a comment

Uh oh!

Uh oh!

neilconway commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

neilconway commented Feb 24, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

neilconway commented Feb 24, 2026

Uh oh!

neilconway commented Feb 24, 2026

Uh oh!

comphead Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

neilconway Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

comphead Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

neilconway commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

neilconway Feb 26, 2026 •

edited

Loading