-
Notifications
You must be signed in to change notification settings - Fork 338
chore: Respect to legacySizeOfNull option for size function #3036
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 14 commits
768b3e9
c68c342
d887555
231aa90
9500bbb
9577481
3791557
7c2f082
3177912
350e046
0971e88
609a605
a151b2c
8c19385
ce60509
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 |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ import org.apache.spark.sql.CometTestBase | |
| import org.apache.spark.sql.catalyst.expressions.{ArrayAppend, ArrayDistinct, ArrayExcept, ArrayInsert, ArrayIntersect, ArrayJoin, ArrayRepeat, ArraysOverlap, ArrayUnion} | ||
| import org.apache.spark.sql.execution.adaptive.AdaptiveSparkPlanHelper | ||
| import org.apache.spark.sql.functions._ | ||
| import org.apache.spark.sql.internal.SQLConf | ||
| import org.apache.spark.sql.types.ArrayType | ||
|
|
||
| import org.apache.comet.CometSparkSessionExtensions.{isSpark35Plus, isSpark40Plus} | ||
|
|
@@ -871,4 +872,20 @@ class CometArrayExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelp | |
| } | ||
| } | ||
| } | ||
|
|
||
| test("size - respect to legacySizeOfNull") { | ||
| val table = "t1" | ||
| withSQLConf(CometConf.COMET_NATIVE_SCAN_IMPL.key -> CometConf.SCAN_NATIVE_ICEBERG_COMPAT) { | ||
|
Contributor
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. we need to make sure the ansi mode is off here. According to Spark
Contributor
Author
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. Thanks @comphead. ANSI mode is disabled by default, but for safety, I've explicitly added the setting. |
||
| withTable(table) { | ||
| sql(s"create table $table(col array<string>) using parquet") | ||
| sql(s"insert into $table values(null)") | ||
| withSQLConf(SQLConf.LEGACY_SIZE_OF_NULL.key -> "false") { | ||
| checkSparkAnswerAndOperator(sql(s"select size(col) from $table")) | ||
| } | ||
| withSQLConf(SQLConf.LEGACY_SIZE_OF_NULL.key -> "true") { | ||
| checkSparkAnswerAndOperator(sql(s"select size(col) from $table")) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
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.
should we also refer to ansi mode?
Uh oh!
There was an error while loading. Please reload this page.
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.
For more safety, I used SQLConf.get.legacySizeOfNull.