-
Notifications
You must be signed in to change notification settings - Fork 435
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
[GLUTEN-5341][VL][TEST] Fix SPARK-42782: Hive compatibility check for get_json_object #5467
Conversation
Run Gluten Clickhouse CI |
1 similar comment
Run Gluten Clickhouse CI |
@zhli1142015 Could you please review? Thanks |
@@ -126,7 +126,7 @@ class VeloxTestSettings extends BackendTestSettings { | |||
enableSuite[GlutenHashExpressionsSuite] | |||
enableSuite[GlutenIntervalExpressionsSuite] | |||
enableSuite[GlutenJsonFunctionsSuite] | |||
// Disable for Spark3.5. | |||
// * in get_json_object expression not supported in velox |
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.
Is there any issue to track this, if not please open one, thanks.
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.
Sure, I will open one, this is the PR where get_json_object support was added facebookincubator/velox@20e4678
CC @PHILO-HE |
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenJsonFunctionsSuite.scala
Outdated
Show resolved
Hide resolved
runTest(json, "$.store.bicycle", "{\"price\":19.95,\"color\":\"red\"}") | ||
runTest(json, "$.store.book", book) | ||
runTest(json, "$.store.book[0]", book0) | ||
runTest(json, "$.store.book[*]", null) // not supported in velox |
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.
As null is not the expected result for Spark user, I think we should comment or just remove such test cases. And it would be nice to document this limitation here: https://github.com/apache/incubator-gluten/blob/main/docs/velox-backend-limitations.md#json-functions
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.
Added, thanks for the suggestion @PHILO-HE
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenJsonFunctionsSuite.scala
Outdated
Show resolved
Hide resolved
bd8de58
to
7aa8827
Compare
Run Gluten Clickhouse CI |
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.
Only one nit.
Run Gluten Clickhouse CI |
I have addressed it, thanks. |
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.
Thanks!
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
What changes were proposed in this pull request?
Fixes get_json_object test introduced in spark 3.5 to check compatibility with hive. * character is not supported in velox in json queries.
(Fixes: #5341)
How was this patch tested?
Ran in local