-
Notifications
You must be signed in to change notification settings - Fork 265
feat: add support for array_remove expression #1179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
d477f3b
988e021
1dbb3d3
ec89c3a
04bee72
68ba5ad
866423b
1ed00e1
10a4b47
de3d599
f8485c5
3594ded
4966d7c
cefc7f8
3caf95c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2529,4 +2529,21 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper { | |||||||||||
| spark.sql("SELECT array_contains((CASE WHEN _2 =_3 THEN array(_4) END), _4) FROM t1")); | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
| test("array_remove") { | ||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: there should be a blank line between tests:
Suggested change
|
||||||||||||
| Seq(true, false).foreach { dictionaryEnabled => | ||||||||||||
| withTempDir { dir => | ||||||||||||
| val path = new Path(dir.toURI.toString, "test.parquet") | ||||||||||||
| makeParquetFileAllTypes(path, dictionaryEnabled, 10000) | ||||||||||||
| spark.read.parquet(path.toString).createOrReplaceTempView("t1"); | ||||||||||||
|
||||||||||||
| spark.read.parquet(path.toString).createOrReplaceTempView("t1"); | |
| spark.read.parquet(path.toString).createOrReplaceTempView("t1") |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting error for empty array()
- array_remove *** FAILED *** (5 seconds, 932 milliseconds)
Expected only Comet native operators, but found Project.
plan: *(1) Project [array_remove(CASE WHEN (cast(_2#1 as smallint) = _3#2) THEN array(cast(_2#1 as int), cast(_3#2 as int), _4#3) ELSE [] END, cast(_3#2 as int)) AS array_remove(CASE WHEN (_2 = _3) THEN array(_2, _3, _4) ELSE array() END, _3)#85]
+- *(1) CometColumnarToRow
+- CometScan parquet [_2#1,_3#2,_4#3] Batched: true, DataFilters: [], Format: CometParquet, Location: InMemoryFileIndex(1 paths)[file:/Users/jatin/Documents/open-source/datafusion-comet/spark/target/..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<_2:tinyint,_3:smallint,_4:int> (CometTestBase.scala:184)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into this and the issue is that Comet does not support literal arrays, so does not support array(). I think we will need to skip the empty array test.
@parthchandra Do you have any other ideas for testing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have an idea for how to test this and how to get more coverage on all array expression tests. I filed #1269 to track
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #1270
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This can be inlined