-
Notifications
You must be signed in to change notification settings - Fork 919
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
[KYUUBI #5336] Spark extension supports Spark 3.5 #5336
Conversation
...uubi-extension-spark-3-5/src/main/scala/org/apache/kyuubi/sql/KyuubiEnsureRequirements.scala
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #5336 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 590 588 -2
Lines 33493 33399 -94
Branches 4424 4387 -37
======================================
+ Misses 33493 33399 -94 see 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Seems CI does not cover Spark 3.5 |
I have added CI for spark 3.5 and it seems that there are some failed tests, I will try to fix them. |
@@ -157,7 +157,7 @@ | |||
}, { | |||
"classname" : "org.apache.spark.sql.catalyst.plans.logical.CreateTableAsSelect", | |||
"tableDescs" : [ { | |||
"fieldName" : "left", | |||
"fieldName" : "name", |
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.
There is no left
method in spark 3.5.
spark 3.4 related code:
https://github.com/apache/spark/blob/e056fc8fb4d1c1ab8786a0989adc7298cc918dae/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala#L444
spark 3.5 related code:
https://github.com/apache/spark/blob/64e2b22f6b4023197871a60eb08b055688e9fdd2/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala#L467
@pan3793 @ulysses-you The test cases have been run successfully and this is ready for review. cc @bowenliang123 |
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.
LGTM
### _Why are the changes needed?_ It is basically copied from `kyuubi-extension-spark-3-4`. ### _How was this patch tested?_ Compiled successfully: ``` build/mvn clean install -DskipTests -Pflink-provided,spark-provided,hive-provided,spark-3.5 ``` - [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [ ] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request ### _Was this patch authored or co-authored using generative AI tooling?_ No Closes #5336 from wForget/dev_spark_3_5. Closes #5336 7ba9980 [wforget] remove iceberg.version in spark-3.5 profile a18ce16 [wforget] Regenerate KyuubiEnsureRequirements based on EnsureRequirements in spark 3.5 4725c47 [wforget] fix iceberg version f5a8ea9 [wforget] Bump iceberg 1.4.0 06558dc [wforget] make kyuubi-spark-authz plugin compatible with Spark3.5 90d0e4c [wforget] make kyuubi-spark-authz plugin compatible with Spark3.5 4bc8d24 [wforget] add ci 1b3f2d9 [wforget] Make kyuubi spark extension compatible with Spark3.5 Authored-by: wforget <[email protected]> Signed-off-by: Cheng Pan <[email protected]> (cherry picked from commit d2c072b) Signed-off-by: Cheng Pan <[email protected]>
Thanks, merged to master/1.8 |
Why are the changes needed?
It is basically copied from
kyuubi-extension-spark-3-4
.How was this patch tested?
Compiled successfully:
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before make a pull request
Was this patch authored or co-authored using generative AI tooling?
No