Skip to content

Conversation

@AngersZhuuuu
Copy link
Contributor

Why are the changes needed?

To close #5447. Kyuubi authz Support hudi DeleteHoodieTableCommand/UpdateHoodieTableCommand/MergeIntoHoodieTableCommand

How was this patch tested?

  • 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

…ateHoodieTableCommand/MergeIntoHoodieTableCommand
@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2023

Codecov Report

Merging #5482 (2598af2) into master (c4cdf18) will not change coverage.
The diff coverage is 0.00%.

@@          Coverage Diff           @@
##           master   #5482   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         588     588           
  Lines       33466   33481   +15     
  Branches     4401    4405    +4     
======================================
- Misses      33466   33481   +15     
Files Coverage Δ
...ubi/plugin/spark/authz/serde/queryExtractors.scala 0.00% <0.00%> (ø)
...ubi/plugin/spark/authz/serde/tableExtractors.scala 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@bowenliang123
Copy link
Contributor

And again, please do not forget the positive cases.

@yaooqinn
Copy link
Member

The current implementation looks to break the original intention of plan serde layer.

@AngersZhuuuu
Copy link
Contributor Author

The current implementation looks to break the original intention of plan serde layer.

How about current? match all case.

Copy link
Contributor

@bowenliang123 bowenliang123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Minor comment.

@HudiTest
class HudiCatalogRangerSparkExtensionSuite extends RangerSparkExtensionSuite {
override protected val catalogImpl: String = "hive"
override protected val catalogImpl: String = "in-memory"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we have to change this?

Copy link
Contributor Author

@AngersZhuuuu AngersZhuuuu Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we have to change this?

To trigger a positive case, if not change this, will trigger some hive hms-related derby error and can' run. I have checked that this changing to in-memory the plan is also Hudi's command.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

@bowenliang123 bowenliang123 changed the title [KYUUBI #5447][AUTHZ] Support hudi DeleteHoodieTableCommand/UpdateHoo…dieTableCommand/MergeIntoHoodieTableCommand [KYUUBI #5447][AUTHZ] Support Hudi DeleteHoodieTableCommand/UpdateHoodieTableCommand/MergeIntoHoodieTableCommand Oct 20, 2023
@bowenliang123 bowenliang123 added this to the v1.9.0 milestone Oct 21, 2023
@bowenliang123
Copy link
Contributor

Thanks, merged to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TASK][EASY] Support Hudi Delete/Update/MergeInto Table Command

4 participants