Feat: Support arrays_overlap function #1312
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1312 +/- ##
============================================
- Coverage 34.72% 34.56% -0.16%
+ Complexity 992 965 -27
============================================
Files 117 117
Lines 45046 45077 +31
Branches 9901 9939 +38
============================================
- Hits 15640 15581 -59
- Misses 26253 26332 +79
- Partials 3153 3164 +11 ☔ View full report in Codecov by Sentry. |
This was referenced Jan 21, 2025
andygrove
reviewed
Jan 22, 2025
| @@ -2300,6 +2300,12 @@ object QueryPlanSerde extends Logging with ShimQueryPlanSerde with CometExprShim | |||
| expr.children(1), | |||
| inputs, | |||
| (builder, binaryExpr) => builder.setArrayAppend(binaryExpr)) | |||
| case ArraysOverlap(leftArrayExpr, rightArrayExpr) => | |||
There was a problem hiding this comment.
Do we support this for all input data types?
andygrove
reviewed
Jan 22, 2025
| @@ -2568,4 +2568,21 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| test("arrays_overlap") { | |||
There was a problem hiding this comment.
Could you add tests where one or both input expressions evaluate to null on some rows? This can be achieved with a CASE expression as in some of the other array tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Related to Epic: #1042
arrays_overlap:select arrays_overlap(array('hello', '-', 'world'), array('hello'))=>trueDataFusion' s array_has_any has same behavior with Spark 's arrays_overlap function
Spark:https://docs.databricks.com/en/sql/language-manual/functions/arrays_overlap.htmlDataFusion:https://datafusion.apache.org/user-guide/sql/scalar_functions.html#array-has-anyRationale for this change
Defined under Epic: #1042
What changes are included in this PR?
planner.rs:Maps Spark 'sarrays_overlapfunction to DataFusionarray_has_anyphysical expression from Spark physical expression with return type: DataType::Boolean,expr.proto:arrays_overlap message has been added,QueryPlanSerde.scala:arrays_overlap pattern matching case has been added,CometExpressionSuite.scala:A new UT has been added for arrays_overlap function.How are these changes tested?
A new UT has been added.