Skip to content

Commit 5746048

Browse files
compheadcomphead
authored
[branch-53]: fix array_remove_* with NULLS (apache#21013) (apache#21016)
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#21011 . ## Rationale for this change Handle correctly `array_remove_*` functions if NULL is a value to delete <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. --> (cherry picked from commit 6ab16cc) ## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes #. ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
1 parent b83c0e0 commit 5746048

2 files changed

Lines changed: 97 additions & 5 deletions

File tree

‎datafusion/functions-nested/src/remove.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ fn general_remove<OffsetSize: OffsetSizeTrait>(
390390
let mut valid = NullBufferBuilder::new(list_array.len());
391391

392392
for (row_index, offset_window) in list_array.offsets().windows(2).enumerate() {
393-
if list_array.is_null(row_index) {
393+
if list_array.is_null(row_index) || element_array.is_null(row_index) {
394394
offsets.push(offsets[row_index]);
395395
valid.append_null();
396396
continue;

‎datafusion/sqllogictest/test_files/array.slt‎

Lines changed: 96 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5512,21 +5512,47 @@ select
55125512
array_remove(make_array(1, null, 2), null),
55135513
array_remove(make_array(1, null, 2, null), null);
55145514
----
5515-
[1, 2] [1, 2, NULL]
5515+
NULL NULL
55165516

55175517
query ??
55185518
select
55195519
array_remove(arrow_cast(make_array(1, null, 2), 'LargeList(Int64)'), null),
55205520
array_remove(arrow_cast(make_array(1, null, 2, null), 'LargeList(Int64)'), null);
55215521
----
5522-
[1, 2] [1, 2, NULL]
5522+
NULL NULL
55235523

55245524
query ??
55255525
select
55265526
array_remove(arrow_cast(make_array(1, null, 2), 'FixedSizeList(3, Int64)'), null),
55275527
array_remove(arrow_cast(make_array(1, null, 2, null), 'FixedSizeList(4, Int64)'), null);
55285528
----
5529-
[1, 2] [1, 2, NULL]
5529+
NULL NULL
5530+
5531+
# array_remove with null element from column
5532+
query ?
5533+
select array_remove(column1, column2) from (values
5534+
(make_array(1, 2, 3), 2),
5535+
(make_array(4, 5, 6), null),
5536+
(make_array(7, 8, 9), 8),
5537+
(null, 1)
5538+
) as t(column1, column2);
5539+
----
5540+
[1, 3]
5541+
NULL
5542+
[7, 9]
5543+
NULL
5544+
5545+
# array_remove with null element from column (LargeList)
5546+
query ?
5547+
select array_remove(column1, column2) from (values
5548+
(arrow_cast(make_array(1, 2, 3), 'LargeList(Int64)'), 2),
5549+
(arrow_cast(make_array(4, 5, 6), 'LargeList(Int64)'), null),
5550+
(arrow_cast(make_array(7, 8, 9), 'LargeList(Int64)'), 8)
5551+
) as t(column1, column2);
5552+
----
5553+
[1, 3]
5554+
NULL
5555+
[7, 9]
55305556

55315557
# array_remove scalar function #2 (element is list)
55325558
query ??
@@ -5669,6 +5695,46 @@ select array_remove(make_array([1, 2, 3], [4, 5, 6], [4, 5, 6], [10, 11, 12], [1
56695695

56705696
## array_remove_n (aliases: `list_remove_n`)
56715697

5698+
# array_remove_n with null element scalar
5699+
query ??
5700+
select array_remove_n(make_array(1, 2, 2, 1, 1), NULL, 2),
5701+
array_remove_n(make_array(1, 2, 2, 1, 1), 2, 2);
5702+
----
5703+
NULL [1, 1, 1]
5704+
5705+
# array_remove_n with null element scalar (LargeList)
5706+
query ??
5707+
select array_remove_n(arrow_cast(make_array(1, 2, 2, 1, 1), 'LargeList(Int64)'), NULL, 2),
5708+
array_remove_n(arrow_cast(make_array(1, 2, 2, 1, 1), 'LargeList(Int64)'), 2, 2);
5709+
----
5710+
NULL [1, 1, 1]
5711+
5712+
# array_remove_n with null element from column
5713+
query ?
5714+
select array_remove_n(column1, column2, column3) from (values
5715+
(make_array(1, 2, 2, 1, 1), 2, 2),
5716+
(make_array(3, 4, 4, 3, 3), null, 2),
5717+
(make_array(5, 6, 6, 5, 5), 6, 1),
5718+
(null, 1, 1)
5719+
) as t(column1, column2, column3);
5720+
----
5721+
[1, 1, 1]
5722+
NULL
5723+
[5, 6, 5, 5]
5724+
NULL
5725+
5726+
# array_remove_n with null element from column (LargeList)
5727+
query ?
5728+
select array_remove_n(column1, column2, column3) from (values
5729+
(arrow_cast(make_array(1, 2, 2, 1, 1), 'LargeList(Int64)'), 2, 2),
5730+
(arrow_cast(make_array(3, 4, 4, 3, 3), 'LargeList(Int64)'), null, 2),
5731+
(arrow_cast(make_array(5, 6, 6, 5, 5), 'LargeList(Int64)'), 6, 1)
5732+
) as t(column1, column2, column3);
5733+
----
5734+
[1, 1, 1]
5735+
NULL
5736+
[5, 6, 5, 5]
5737+
56725738
# array_remove_n scalar function #1
56735739
query ???
56745740
select array_remove_n(make_array(1, 2, 2, 1, 1), 2, 2), array_remove_n(make_array(1.0, 2.0, 2.0, 1.0, 1.0), 1.0, 2), array_remove_n(make_array('h', 'e', 'l', 'l', 'o'), 'l', 3);
@@ -5761,7 +5827,33 @@ select array_remove_n(make_array([1, 2, 3], [4, 5, 6], [4, 5, 6], [10, 11, 12],
57615827
query ?
57625828
select array_remove_all(make_array(1, 2, 2, 1, 1), NULL);
57635829
----
5764-
[1, 2, 2, 1, 1]
5830+
NULL
5831+
5832+
# array_remove_all with null element from column
5833+
query ?
5834+
select array_remove_all(column1, column2) from (values
5835+
(make_array(1, 2, 2, 1, 1), 2),
5836+
(make_array(3, 4, 4, 3, 3), null),
5837+
(make_array(5, 6, 6, 5, 5), 6),
5838+
(null, 1)
5839+
) as t(column1, column2);
5840+
----
5841+
[1, 1, 1]
5842+
NULL
5843+
[5, 5, 5]
5844+
NULL
5845+
5846+
# array_remove_all with null element from column (LargeList)
5847+
query ?
5848+
select array_remove_all(column1, column2) from (values
5849+
(arrow_cast(make_array(1, 2, 2, 1, 1), 'LargeList(Int64)'), 2),
5850+
(arrow_cast(make_array(3, 4, 4, 3, 3), 'LargeList(Int64)'), null),
5851+
(arrow_cast(make_array(5, 6, 6, 5, 5), 'LargeList(Int64)'), 6)
5852+
) as t(column1, column2);
5853+
----
5854+
[1, 1, 1]
5855+
NULL
5856+
[5, 5, 5]
57655857

57665858
# array_remove_all scalar function #1
57675859
query ???

0 commit comments

Comments
 (0)