feat[arrow-ord]: suppport REE comparisons by asubiotto · Pull Request #9621 · apache/arrow-rs

asubiotto · 2026-03-27T10:48:40Z

This commit implements native comparisons on REE-encoded arrays which are treated similarly to dictionary indirection.

This commit implements REE to scalar comparisons by operating on the physical values only then bulk expanding the boolean result.

REE-to-REE comparisons are also optimized by computing aligned physical value runs to minimize comparisons.

Mixed cases (REE vs flat) materialize a logical index mapping similar to dictionaries.

This commit also supports REE<Dict>.

For comparison, here are the benchmark results with flat arrays as a reference on my local machine:

eq Int32                time:   [14.955 µs 15.162 µs 15.396 µs]
eq scalar Int32         time:   [11.379 µs 11.418 µs 11.459 µs]

ree_comparison/eq_ree_scalar(phys=64,log=65536)     time:   [453.31 ns 454.88 ns 456.43 ns]
ree_comparison/eq_ree_scalar(phys=1024,log=65536)   time:   [4.1224 µs 4.1298 µs 4.1368 µs]
ree_comparison/eq_ree_scalar(phys=32768,log=65536)  time:   [93.506 µs 94.085 µs 94.993 µs]
ree_comparison/eq_ree_ree(phys=64,log=65536)        time:   [413.96 ns 414.82 ns 415.87 ns]
ree_comparison/eq_ree_ree(phys=1024,log=65536)      time:   [4.1597 µs 4.1660 µs 4.1749 µs]
ree_comparison/eq_ree_ree(phys=32768,log=65536)     time:   [128.74 µs 144.40 µs 161.53 µs]

As is expected, the more we take advantage of REE encoding, the faster the comparisons are.

Which issue does this PR close?

Rationale for this change

Feature enhancement for comparisons

What changes are included in this PR?

Support for REE comparisons, including tests and benchmarks.

Are these changes tested?

Yes.

Are there any user-facing changes?

REE comparisons are now supported.

asubiotto · 2026-03-27T11:01:36Z

Unfortunately did not see there is an open PR #9448 before working on this, however I think this PR is more complete since #9448 does not seem to support REE with scalar or mixed comparisons. Also, this PR is more performant:

PR #9448 (REE-vs-REE only, panics on REE-vs-scalar):
  ree_comparison/eq_ree_ree(phys=64,log=65536)      time:   [2.5053 µs 2.5281 µs 2.5516 µs]
  ree_comparison/lt_ree_ree(phys=64,log=65536)       time:   [2.5559 µs 2.5783 µs 2.6024 µs]
  ree_comparison/eq_ree_ree(phys=1024,log=65536)     time:   [9.6708 µs 9.7004 µs 9.7291 µs]
  ree_comparison/lt_ree_ree(phys=1024,log=65536)     time:   [9.4187 µs 9.4457 µs 9.4717 µs]
  ree_comparison/eq_ree_ree(phys=32768,log=65536)    time:   [225.23 µs 228.23 µs 232.46 µs]
  ree_comparison/lt_ree_ree(phys=32768,log=65536)    time:   [200.61 µs 201.16 µs 201.81 µs]

  Ours:
  ree_comparison/eq_ree_scalar(phys=64,log=65536)    time:   [449.04 ns 456.42 ns 464.89 ns]
  ree_comparison/lt_ree_scalar(phys=64,log=65536)    time:   [447.29 ns 449.15 ns 451.32 ns]
  ree_comparison/eq_ree_ree(phys=64,log=65536)       time:   [425.39 ns 426.91 ns 428.48 ns]
  ree_comparison/lt_ree_ree(phys=64,log=65536)       time:   [439.47 ns 440.59 ns 441.69 ns]
  ree_comparison/eq_ree_flat(phys=64,log=65536)      time:   [46.541 µs 46.659 µs 46.783 µs]
  ree_comparison/eq_ree_scalar(phys=1024,log=65536)  time:   [4.4344 µs 4.4435 µs 4.4543 µs]
  ree_comparison/lt_ree_scalar(phys=1024,log=65536)  time:   [5.3170 µs 5.6797 µs 6.1374 µs]
  ree_comparison/eq_ree_ree(phys=1024,log=65536)     time:   [4.7932 µs 4.8049 µs 4.8183 µs]
  ree_comparison/lt_ree_ree(phys=1024,log=65536)     time:   [5.7911 µs 6.7388 µs 7.8086 µs]
  ree_comparison/eq_ree_flat(phys=1024,log=65536)    time:   [51.404 µs 52.651 µs 54.233 µs]
  ree_comparison/eq_ree_scalar(phys=32768,log=65536) time:   [94.964 µs 95.714 µs 96.556 µs]
  ree_comparison/lt_ree_scalar(phys=32768,log=65536) time:   [95.557 µs 96.428 µs 97.527 µs]
  ree_comparison/eq_ree_ree(phys=32768,log=65536)    time:   [137.23 µs 139.33 µs 141.49 µs]
  ree_comparison/lt_ree_ree(phys=32768,log=65536)    time:   [105.79 µs 106.37 µs 107.20 µs]
  ree_comparison/eq_ree_flat(phys=32768,log=65536)   time:   [143.00 µs 215.04 µs 340.63 µs]

cc @alamb @Jefffrey @brunal

brunal · 2026-03-27T11:30:47Z

Super interesting. I suspect that the perf gain might go lower as # of phys indices grow as I'm avoiding copying them, but this definitely seems like a better implementation.

arrow-ord/src/cmp.rs

brunal · 2026-03-27T11:33:28Z

arrow-ord/src/cmp.rs

 fn apply<T: ArrayOrd>(
    op: Op,
    l: T,
    l_s: bool,


Unless I'm mistaken, you can have only one of l_s == true / l_v.is_some() / l_ree.is_some() at a time. How about introducing an enum? It will reduce the arg # and make them clearer

I don't think that's right. Scalars can have ree and/or dictionary types and l_v and l_ree are both Some for Ree

Ah, right! Still, I think that the too_many_arguments link is legit, and I'd consider groupin the args for each side -- either anonymous 3-tuple or a real struct. It would make the call site much cleaner. e.g.

struct ArrayInfo<'a> { is_scalar: bool, dict_values: &'a dyn AnyDictionyArray, run_array: Option<&'a ReeInfo>, }

arrow-ord/src/cmp.rs

asubiotto · 2026-03-27T13:54:58Z

Thanks for the review @brunal. I addressed all your comments.

brunal · 2026-03-28T08:38:15Z

arrow-ord/src/cmp.rs

 fn apply<T: ArrayOrd>(
    op: Op,
    l: T,
    l_s: bool,


Ah, right! Still, I think that the too_many_arguments link is legit, and I'd consider groupin the args for each side -- either anonymous 3-tuple or a real struct. It would make the call site much cleaner. e.g.

struct ArrayInfo<'a> { is_scalar: bool, dict_values: &'a dyn AnyDictionyArray, run_array: Option<&'a ReeInfo>, }

brunal · 2026-03-28T21:06:36Z

arrow-ord/src/cmp.rs

+    }
+}
+
+fn ree_physical_indices(info: &ReeInfo) -> Vec<usize> {


you can turn this into a .skip().map_while(...) to return an Iterator<Item = usize>. Then in logical_indices's Some-Some branch, you can directly map on it. This avoid materializing the intermediate physical indices vector.

This is very nitpicky, as the one branch that benefits from this, REE<dict>, seems like quite a niche use case :)

I think I'm going to keep this as is currently written to mirror the dict normalized_keys materialization. We can do this as a follow up improvement if we see it's worth it.

arrow-ord/src/cmp.rs

asubiotto · 2026-04-02T21:33:20Z

Updated, apologies for the delay.

brunal

Looks excellent to me, now you just need someone with repo rights :-)

This commit implements native comparisons on REE-encoded arrays which are treated similarly to dictionary indirection. This commit implements REE to scalar comparisons by operating on the physical values only then bulk expanding the boolean result. REE-to-REE comparisons are also optimized by computing aligned physical value runs to minimize comparisons. Mixed cases (REE vs flat) materialize a logical index mapping similar to dictionaries. This commit also supports REE<Dict>. For comparison, here are the benchmark results with flat arrays as a reference on my local machine: ``` eq Int32 time: [14.955 µs 15.162 µs 15.396 µs] eq scalar Int32 time: [11.379 µs 11.418 µs 11.459 µs] ree_comparison/eq_ree_scalar(phys=64,log=65536) time: [453.31 ns 454.88 ns 456.43 ns] ree_comparison/eq_ree_scalar(phys=1024,log=65536) time: [4.1224 µs 4.1298 µs 4.1368 µs] ree_comparison/eq_ree_scalar(phys=32768,log=65536) time: [93.506 µs 94.085 µs 94.993 µs] ree_comparison/eq_ree_ree(phys=64,log=65536) time: [413.96 ns 414.82 ns 415.87 ns] ree_comparison/eq_ree_ree(phys=1024,log=65536) time: [4.1597 µs 4.1660 µs 4.1749 µs] ree_comparison/eq_ree_ree(phys=32768,log=65536) time: [128.74 µs 144.40 µs 161.53 µs] ``` As is expected, the more we take advantage of REE encoding, the faster the comparisons are. Signed-off-by: Alfonso Subiotto Marques <alfonso.subiotto@polarsignals.com>

asubiotto · 2026-04-03T21:43:13Z

Thanks again for your review. cc @alamb @Jefffrey this PR is ready for a final review

asubiotto · 2026-04-13T07:09:55Z

Friendly ping for a review or triage @alamb (not sure if there's another code owner)

alamb · 2026-04-13T20:24:40Z

Thanks for the ping @asubiotto -- I'll try and find time to review this, but I am having a hard time finding time

alamb · 2026-04-13T20:24:57Z

But nwo I see @brunal has already done it so that will make it easier

asubiotto · 2026-04-14T07:21:48Z

Thanks! Appreciate the time. Yes, @brunal conducted an in-depth review so I only need a final look from a maintainer.

github-actions bot added the arrow Changes to the arrow crate label Mar 27, 2026

asubiotto force-pushed the asubiotto/reecmp branch from 79d81af to eee179c Compare March 27, 2026 11:06

brunal reviewed Mar 27, 2026

View reviewed changes

asubiotto force-pushed the asubiotto/reecmp branch 2 times, most recently from 85034d0 to 791ec8c Compare March 27, 2026 13:48

brunal reviewed Mar 28, 2026

View reviewed changes

brunal mentioned this pull request Mar 31, 2026

Implement comparisons for RunArray. #9448

Closed

asubiotto force-pushed the asubiotto/reecmp branch from 791ec8c to a3669d9 Compare April 2, 2026 21:18

brunal approved these changes Apr 3, 2026

View reviewed changes

asubiotto force-pushed the asubiotto/reecmp branch from a3669d9 to 08e2e39 Compare April 3, 2026 21:42

Conversation

asubiotto commented Mar 27, 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?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

asubiotto commented Mar 27, 2026

Uh oh!

brunal commented Mar 27, 2026

Uh oh!

Uh oh!

Uh oh!

brunal Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

asubiotto Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

brunal Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

asubiotto Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

asubiotto commented Mar 27, 2026

Uh oh!

brunal Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

brunal Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

asubiotto Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

asubiotto commented Apr 2, 2026

Uh oh!

brunal left a comment

Choose a reason for hiding this comment

Uh oh!

asubiotto commented Apr 3, 2026

Uh oh!

asubiotto commented Apr 13, 2026

Uh oh!

alamb commented Apr 13, 2026

Uh oh!

alamb commented Apr 13, 2026

Uh oh!

asubiotto commented Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

asubiotto commented Mar 27, 2026 •

edited

Loading