kyuubi icon indicating copy to clipboard operation
kyuubi copied to clipboard

[KYUUBI #4149] Refactor FilteredShowObjectCommands based on Authz Pla…

Open zhouyifan279 opened this issue 2 years ago • 2 comments

…n Serde Layer

Why are the changes needed?

  1. Refactor FilteredShowObjectCommands based on Authz Plan Serde Layer
  2. #3687 can reuse the framework implemented in this PR

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

  • [x] Run test locally before make a pull request

zhouyifan279 avatar Jan 12 '23 08:01 zhouyifan279

Codecov Report

Merging #4154 (25e34e1) into master (892a44d) will increase coverage by 0.01%. The diff coverage is 89.09%.

@@             Coverage Diff              @@
##             master    #4154      +/-   ##
============================================
+ Coverage     53.01%   53.03%   +0.01%     
  Complexity       13       13              
============================================
  Files           548      551       +3     
  Lines         29946    29967      +21     
  Branches       4025     4021       -4     
============================================
+ Hits          15877    15894      +17     
- Misses        12593    12602       +9     
+ Partials       1476     1471       -5     
Impacted Files Coverage Δ
.../kyuubi/plugin/spark/authz/ranger/AccessType.scala 15.78% <0.00%> (-0.43%) :arrow_down:
...spark/authz/ranger/ReplaceShowObjectCommands.scala 68.75% <ø> (ø)
.../plugin/spark/authz/ranger/RuleAuthorization.scala 87.50% <ø> (+6.10%) :arrow_up:
...ache/kyuubi/plugin/spark/authz/serde/package.scala 85.29% <66.66%> (+0.91%) :arrow_up:
.../plugin/spark/authz/serde/sparkPlanRewriters.scala 85.71% <85.71%> (ø)
...plugin/spark/authz/ranger/RuleRewriteCommand.scala 88.88% <88.88%> (ø)
...ugin/spark/authz/ranger/RangerSparkExtension.scala 100.00% <100.00%> (ø)
...in/spark/authz/ranger/RewriteCommandStrategy.scala 100.00% <100.00%> (ø)
.../kyuubi/plugin/spark/authz/serde/CommandSpec.scala 82.35% <100.00%> (+2.35%) :arrow_up:
...che/kyuubi/plugin/spark/authz/serde/Rewriter.scala 100.00% <100.00%> (ø)
... and 9 more

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

codecov-commenter avatar Jan 16 '23 15:01 codecov-commenter

I agree with this PR overall, but can you summarize the changes and refactoring design in the description for clarification ? Including but not limited to, the purpose and goals of the Rewriter to Serde layer, what new optimizer rule and planner strategy introduced or refactored and their relationship, what privilege changed of which type of source. And most importantly, the key reason or consideration to these changes. @zhouyifan279

bowenliang123 avatar Feb 04 '23 14:02 bowenliang123