fix: array_sort doesnt respect array inputs for sort order/nulls first by JSOD11 · Pull Request #24502 · apache/datafusion

JSOD11 · 2026-08-19T20:33:47Z

Which issue does this PR close?

Closes array_sort doesnt respect array inputs for sort order/nulls first #24132

Rationale for this change

array_sort now evaluates its optional order and nulls_order arguments per row instead of reading only the first row of each option column.

Previously, queries like this would use the first row's option value for the whole batch:

array_sort(arr, order)

So if row 1 used 'asc' and row 2 used 'desc', row 2 could still be sorted ascending.

This PR keeps the existing primitive and non-primitive sorting paths, but threads the option arrays through the sort logic so each output row uses that row's own options.

What changes are included in this PR?

Sort order can vary per row

query ?
select array_sort(arr, order)
from values
    (make_array(5, 2, 3), 'asc'),
    (make_array(5, 2, 3), 'desc'),
    (make_array(10, 20, 30), 'desc') as t(arr, order);

Before this PR, all rows used the first row's 'asc' option:

[2, 3, 5]
[2, 3, 5]
[10, 20, 30]

Now each row uses its own option:

[2, 3, 5]
[5, 3, 2]
[30, 20, 10]

NULL option values are handled per row

query ?
select array_sort(arr, sort_order)
from values
    (make_array(2, 1), 'ASC'),
    (make_array(2, 1), NULL),
    (make_array(2, 1), 'DESC') as t(arr, sort_order);

Before this PR, the NULL and 'DESC' rows still used the first row's 'ASC' option:

[1, 2]
[1, 2]
[1, 2]

Now the NULL option only affects its own row:

[1, 2]
NULL
[2, 1]

Null placement can vary per row

query ?
select array_sort(arr, order, null_order)
from values
    (make_array(5, 2, NULL, 3, NULL), 'asc', 'NULLS FIRST'),
    (make_array(NULL, 5, NULL, 3, null), 'desc', 'NULLS LAST'),
    (make_array(10, NULL, null, 20, 30, NULL), 'desc', 'NULLS LAST') as t(arr, order, null_order);

Before this PR, later rows could incorrectly reuse the first row's sort options.

Now each row gets the correct ordering:

[NULL, NULL, 2, 3, 5]
[5, 3, NULL, NULL, NULL]
[30, 20, 10, NULL, NULL, NULL]

Additional coverage

This also covers:

• primitive arrays without element nulls
• primitive arrays with element nulls
• non-primitive arrays, such as strings
• NULL order arguments
• NULL nulls_order arguments
• preserving valid empty lists as []

Are these changes tested?

cargo test -p datafusion-sqllogictest --test sqllogictests -- array_sort

Are there any user-facing changes?

Just the bug fixes described.

codecov-commenter · 2026-08-19T20:58:32Z

Codecov Report

❌ Patch coverage is 87.23404% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.68%. Comparing base (bd86190) to head (c9881ce).

Files with missing lines	Patch %	Lines
datafusion/functions-nested/src/sort.rs	87.23%	8 Missing and 10 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24502      +/-   ##
==========================================
- Coverage   82.69%   82.68%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      447204   447300      +96     
  Branches   447204   447300      +96     
==========================================
+ Hits       369793   369865      +72     
- Misses      55002    55014      +12     
- Partials    22409    22421      +12

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

JSOD11 · 2026-10-06T12:43:35Z

cc @Jefffrey for review when you get a moment

Jefffrey · 2026-10-08T03:08:40Z

run benchmark array_sort

adriangbot · 2026-10-08T03:43:03Z

🤖 Benchmark completed (GKE) | trigger

Instance: c4a-highmem-16 (12 vCPU / 65 GiB)

Comparing jsod/array-sort-options-08-19-26 (c9881ce) to bd86190 (merge-base) diff

Run configuration

run benchmark array_sort

CPU Details (lscpu)

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

group                                       HEAD                                   jsod_array-sort-options-08-19-26
-----                                       ----                                   --------------------------------
array_sort/int32/100                        1.00      4.4±0.00ms        ? ?/sec    1.01      4.5±0.02ms        ? ?/sec
array_sort/int32/1000                       1.00     65.5±0.11ms        ? ?/sec    1.00     65.7±0.07ms        ? ?/sec
array_sort/int32/20                         1.00   1135.8±1.23µs        ? ?/sec    1.03   1165.2±4.56µs        ? ?/sec
array_sort/int32/5                          1.00    137.2±1.09µs        ? ?/sec    1.23    169.2±0.72µs        ? ?/sec
array_sort/int32_desc/100                   1.00      4.5±0.00ms        ? ?/sec    1.06      4.8±0.01ms        ? ?/sec
array_sort/int32_desc/1000                  1.00     66.4±0.10ms        ? ?/sec    1.01     66.7±0.06ms        ? ?/sec
array_sort/int32_desc/20                    1.00   1167.4±2.02µs        ? ?/sec    1.17   1363.8±7.21µs        ? ?/sec
array_sort/int32_desc/5                     1.00    157.1±0.52µs        ? ?/sec    2.22    349.4±0.94µs        ? ?/sec
array_sort/int32_null_elements/100          1.00      4.8±0.01ms        ? ?/sec    1.02      4.9±0.04ms        ? ?/sec
array_sort/int32_null_elements/1000         1.00     64.7±0.12ms        ? ?/sec    1.00     64.6±0.07ms        ? ?/sec
array_sort/int32_null_elements/20           1.00   1393.8±6.91µs        ? ?/sec    1.01   1407.9±5.58µs        ? ?/sec
array_sort/int32_null_elements/5            1.00    395.9±4.10µs        ? ?/sec    1.05    416.3±3.57µs        ? ?/sec
array_sort/int32_null_elements_desc/100     1.00      4.9±0.01ms        ? ?/sec    1.05      5.1±0.01ms        ? ?/sec
array_sort/int32_null_elements_desc/1000    1.00     65.5±0.12ms        ? ?/sec    1.00     65.3±0.18ms        ? ?/sec
array_sort/int32_null_elements_desc/20      1.00   1431.7±7.99µs        ? ?/sec    1.12  1603.8±39.70µs        ? ?/sec
array_sort/int32_null_elements_desc/5       1.00    418.1±5.55µs        ? ?/sec    1.47    613.8±1.21µs        ? ?/sec
array_sort/int32_with_nulls                 1.00   1678.1±1.82µs        ? ?/sec    1.02   1712.4±2.00µs        ? ?/sec
array_sort/string/100                       1.00     39.4±0.06ms        ? ?/sec    2.09     82.4±0.67ms        ? ?/sec
array_sort/string/1000                      1.00    551.3±2.20ms        ? ?/sec    1.84   1014.1±6.42ms        ? ?/sec
array_sort/string/20                        1.00      6.3±0.01ms        ? ?/sec    1.72     10.9±0.13ms        ? ?/sec
array_sort/string/5                         1.00    957.0±2.24µs        ? ?/sec    2.23      2.1±0.03ms        ? ?/sec

Resource Usage

array_sort — base (merge-base)

Metric	Value
Wall time	530.1s
Peak memory	592.9 MiB
Avg memory	88.8 MiB
CPU user	324.5s
CPU sys	5.2s
Peak spill	0 B

array_sort — branch

Metric	Value
Wall time	570.1s
Peak memory	1.2 GiB
Avg memory	198.4 MiB
CPU user	349.1s
CPU sys	32.2s
Peak spill	0 B

File an issue against this benchmark runner

Jefffrey

impacts on benchmarks are unfortunate; we could probably tune things a bit to optimize, but perhaps the best way would be to have a separate scalar & array path. i.e. have a fast scalar path for when we have scalar inputs for sort order & nulls order, otherwise fallback to slower array path when all inputs are arrays. though this likely we increase required code 🙁

Jefffrey · 2026-10-08T08:01:15Z

+    let row_count = list_array.len();
    let list_nulls = list_array.nulls();
    let offsets = list_array.offsets();
+    let mut list_validity = BooleanBufferBuilder::new(row_count);


instead of using a builder here we can consider using something like NullBuffer::union to construct the null buffer upfront from all the input arrays

Add sqllogic test showing incorrect behavior.

69d6257

github-actions Bot added the sqllogictest SQL Logic Tests (.slt) label Aug 19, 2026

Sort order works for primitive arrays.

b85f175

github-actions Bot added the functions Changes to functions implementation label Aug 28, 2026

JSOD11 added 6 commits August 31, 2026 13:58

Sort order works for non-primitive arrays.

7119715

Null order works for primitive and non-primitive arrays.

9f686de

Evalute NULL arguments per-row rather than just using the first row.

3002c6e

clippy

9ff5b5e

Add other cases.

36a27b3

Merge branch 'main' into jsod/array-sort-options-08-19-26

c9881ce

JSOD11 marked this pull request as ready for review October 5, 2026 21:54

This comment was marked as duplicate.

Sign in to view

Jefffrey reviewed Oct 8, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix: array_sort doesnt respect array inputs for sort order/nulls first - #24502

fix: array_sort doesnt respect array inputs for sort order/nulls first#24502
JSOD11 wants to merge 8 commits into
apache:mainfrom
JSOD11:jsod/array-sort-options-08-19-26

JSOD11 commented Aug 19, 2026 •

edited

Loading

Uh oh!

codecov-commenter commented Aug 19, 2026 •

edited

Loading

Uh oh!

JSOD11 commented Oct 6, 2026

Uh oh!

Jefffrey commented Oct 8, 2026

Uh oh!

This comment was marked as duplicate.

adriangbot commented Oct 8, 2026

Uh oh!

Jefffrey left a comment

Uh oh!

Jefffrey Oct 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

JSOD11 commented Aug 19, 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?

Sort order can vary per row

NULL option values are handled per row

Null placement can vary per row

Additional coverage

Are these changes tested?

Are there any user-facing changes?

Uh oh!

codecov-commenter commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JSOD11 commented Oct 6, 2026

Uh oh!

Jefffrey commented Oct 8, 2026

Uh oh!

This comment was marked as duplicate.

adriangbot commented Oct 8, 2026

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Jefffrey Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JSOD11 commented Aug 19, 2026 •

edited

Loading

codecov-commenter commented Aug 19, 2026 •

edited

Loading