You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "kazantsev-maksim (via GitHub)" <gi...@apache.org> on 2026/02/01 16:46:51 UTC
[PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
kazantsev-maksim opened a new pull request, #3356:
URL: https://github.com/apache/datafusion-comet/pull/3356
## Which issue does this PR close?
Closes https://github.com/apache/datafusion-comet/issues/3327
## Rationale for this change
## What changes are included in this PR?
## How are these changes tested?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
Posted by "kazantsev-maksim (via GitHub)" <gi...@apache.org>.
kazantsev-maksim commented on code in PR #3356:
URL: https://github.com/apache/datafusion-comet/pull/3356#discussion_r2834081766
##########
spark/src/test/resources/sql-tests/expressions/map/map_from_arrays.sql:
##########
@@ -26,9 +26,7 @@ INSERT INTO test_map_from_arrays VALUES (array('a', 'b', 'c'), array(1, 2, 3)),
query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NOT NULL
--- Comet bug: map_from_arrays(NULL, NULL) causes native crash "map key cannot be null"
--- https://github.com/apache/datafusion-comet/issues/3327
-query ignore(https://github.com/apache/datafusion-comet/issues/3327)
+query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NULL
Review Comment:
Tanks @andygrove, fixed this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #3356:
URL: https://github.com/apache/datafusion-comet/pull/3356#discussion_r2759595775
##########
spark/src/test/resources/sql-tests/expressions/map/map_from_arrays.sql:
##########
@@ -26,9 +26,7 @@ INSERT INTO test_map_from_arrays VALUES (array('a', 'b', 'c'), array(1, 2, 3)),
query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NOT NULL
--- Comet bug: map_from_arrays(NULL, NULL) causes native crash "map key cannot be null"
--- https://github.com/apache/datafusion-comet/issues/3327
-query ignore(https://github.com/apache/datafusion-comet/issues/3327)
+query spark_answer_only
Review Comment:
This should be fully native?
```suggestion
query
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #3356:
URL: https://github.com/apache/datafusion-comet/pull/3356#discussion_r2827043744
##########
spark/src/test/resources/sql-tests/expressions/map/map_from_arrays.sql:
##########
@@ -26,9 +26,7 @@ INSERT INTO test_map_from_arrays VALUES (array('a', 'b', 'c'), array(1, 2, 3)),
query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NOT NULL
--- Comet bug: map_from_arrays(NULL, NULL) causes native crash "map key cannot be null"
--- https://github.com/apache/datafusion-comet/issues/3327
-query ignore(https://github.com/apache/datafusion-comet/issues/3327)
+query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NULL
Review Comment:
I ran some additional tests locally and found that the null check needs to cover both inputs, not just the keys. The Spark docs say `map_from_arrays` "returns null if either the keys array or values array is null", but the current `keyIsNotNullExpr` only checks `expr.left`.
I added the row `(array('x'), NULL)` to the test data and a query `SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NOT NULL AND v IS NULL`, which crashes with `CometNativeException: Compute error: concat requires input of at least one array`.
Here's a suggested updated test file that covers the missing cases. Note that I did use AI to generate these tests, so it is possible that there are mistakes.
```sql
-- ConfigMatrix: parquet.enable.dictionary=false,true
statement
CREATE TABLE test_map_from_arrays(k array<string>, v array<int>) USING parquet
statement
INSERT INTO test_map_from_arrays VALUES
(array('a', 'b', 'c'), array(1, 2, 3)),
(array(), array()),
(NULL, NULL),
(array('x'), NULL),
(NULL, array(99))
-- basic functionality
query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NOT NULL AND v IS NOT NULL
-- both inputs NULL should return NULL
query
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NULL AND v IS NULL
-- keys not null but values null should return NULL (Spark behavior)
query
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NOT NULL AND v IS NULL
-- keys null but values not null should return NULL (Spark behavior)
query
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NULL AND v IS NOT NULL
-- all rows including nulls
query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays
-- literal arguments
query spark_answer_only
SELECT map_from_arrays(array('a', 'b'), array(1, 2))
-- literal null arguments
query
SELECT map_from_arrays(NULL, array(1, 2))
query
SELECT map_from_arrays(array('a'), NULL)
query
SELECT map_from_arrays(NULL, NULL)
```
To fix the serde, the condition should check both inputs for null. Something like `IsNotNull(expr.left) AND IsNotNull(expr.right)` instead of just `IsNotNull(expr.left)`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
Posted by "kazantsev-maksim (via GitHub)" <gi...@apache.org>.
kazantsev-maksim commented on PR #3356:
URL: https://github.com/apache/datafusion-comet/pull/3356#issuecomment-4055777363
Thanks @andygrove, 1 TEST Failed: Failed to CreateArtifact: Received non-retryable error: Failed request: (409) Conflict: an artifact with this name already exists on the workflow run
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #3356:
URL: https://github.com/apache/datafusion-comet/pull/3356#discussion_r2759597210
##########
spark/src/test/resources/sql-tests/expressions/map/map_from_arrays.sql:
##########
@@ -26,9 +26,7 @@ INSERT INTO test_map_from_arrays VALUES (array('a', 'b', 'c'), array(1, 2, 3)),
query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NOT NULL
--- Comet bug: map_from_arrays(NULL, NULL) causes native crash "map key cannot be null"
--- https://github.com/apache/datafusion-comet/issues/3327
-query ignore(https://github.com/apache/datafusion-comet/issues/3327)
+query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NULL
Review Comment:
Could you also add tests where `v` is NULL?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
Posted by "kazantsev-maksim (via GitHub)" <gi...@apache.org>.
kazantsev-maksim commented on code in PR #3356:
URL: https://github.com/apache/datafusion-comet/pull/3356#discussion_r2759737324
##########
spark/src/test/resources/sql-tests/expressions/map/map_from_arrays.sql:
##########
@@ -26,9 +26,7 @@ INSERT INTO test_map_from_arrays VALUES (array('a', 'b', 'c'), array(1, 2, 3)),
query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NOT NULL
--- Comet bug: map_from_arrays(NULL, NULL) causes native crash "map key cannot be null"
--- https://github.com/apache/datafusion-comet/issues/3327
-query ignore(https://github.com/apache/datafusion-comet/issues/3327)
+query spark_answer_only
SELECT map_from_arrays(k, v) FROM test_map_from_arrays WHERE k IS NULL
Review Comment:
I think it will be the same behavior: in the original data, value == null when key == null.
```sql
statement
INSERT INTO test_map_from_arrays VALUES (array('a', 'b', 'c'), array(1, 2, 3)), (array(), array()), (NULL, NULL)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3356:
URL: https://github.com/apache/datafusion-comet/pull/3356#issuecomment-3842013710
## [Codecov](https://app.codecov.io/gh/apache/datafusion-comet/pull/3356?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `15 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 19.96%. Comparing base ([`f09f8af`](https://app.codecov.io/gh/apache/datafusion-comet/commit/f09f8af64c6599255e116a376f4f008f2fd63b43?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`faffcbc`](https://app.codecov.io/gh/apache/datafusion-comet/commit/faffcbc1394dc4fff7894f3c7aa0121ddc1e56e1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 918 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion-comet/pull/3356?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...k/src/main/scala/org/apache/comet/serde/maps.scala](https://app.codecov.io/gh/apache/datafusion-comet/pull/3356?src=pr&el=tree&filepath=spark%2Fsrc%2Fmain%2Fscala%2Forg%2Fapache%2Fcomet%2Fserde%2Fmaps.scala&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9jb21ldC9zZXJkZS9tYXBzLnNjYWxh) | 0.00% | [15 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion-comet/pull/3356?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #3356 +/- ##
=============================================
- Coverage 56.12% 19.96% -36.17%
+ Complexity 976 461 -515
=============================================
Files 119 175 +56
Lines 11743 16164 +4421
Branches 2251 2655 +404
=============================================
- Hits 6591 3227 -3364
- Misses 4012 12422 +8410
+ Partials 1140 515 -625
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/datafusion-comet/pull/3356?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: map_from_arrays() with NULL inputs causes native crash [datafusion-comet]
Posted by "kazantsev-maksim (via GitHub)" <gi...@apache.org>.
kazantsev-maksim closed pull request #3356: Fix: map_from_arrays() with NULL inputs causes native crash
URL: https://github.com/apache/datafusion-comet/pull/3356
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org