kyuubi icon indicating copy to clipboard operation
kyuubi copied to clipboard

[KYUUBI #3515] [Authz] support checking rewritten Iceberg commands and skip apply Row-filter to output tables

Open bowenliang123 opened this issue 2 years ago • 3 comments

Why are the changes needed?

to close #3515.

By replacing mapChildren in RuleApplyRowFilterAndDataMaskingto skip head of children query as insterted by iceberg in IcebergSparkSessionExtensions .

How was this patch tested?

  • [x] 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

bowenliang123 avatar Sep 19 '22 15:09 bowenliang123

  1. Introduced a IcebergTableCatalogRangerSparkExtensionSuite with iceberg's IcebergSparkSessionExtensions for unit test.
  2. Replacing mapChildren in RuleApplyRowFilterAndDataMaskingto skip head of children query as insterted by iceberg in IcebergSparkSessionExtensions .

Currently it passed the UPDATE TABLE in IcebergTableCatalogRangerSparkExtensionSuite

bowenliang123 avatar Sep 19 '22 15:09 bowenliang123

Codecov Report

Merging #3520 (ae2df99) into master (2cb34c4) will increase coverage by 0.06%. The diff coverage is 92.30%.

:exclamation: Current head ae2df99 differs from pull request most recent head 5eb7f84. Consider uploading reports for the commit 5eb7f84 to get more accurate results

@@             Coverage Diff              @@
##             master    #3520      +/-   ##
============================================
+ Coverage     51.75%   51.81%   +0.06%     
  Complexity       13       13              
============================================
  Files           483      483              
  Lines         27023    27023              
  Branches       3777     3766      -11     
============================================
+ Hits          13985    14003      +18     
+ Misses        11672    11661      -11     
+ Partials       1366     1359       -7     
Impacted Files Coverage Δ
...he/kyuubi/plugin/spark/authz/util/AuthZUtils.scala 59.09% <50.00%> (+6.59%) :arrow_up:
...uthz/ranger/RuleApplyRowFilterAndDataMasking.scala 89.58% <92.30%> (-0.90%) :arrow_down:
...he/kyuubi/plugin/spark/authz/IcebergCommands.scala 96.87% <96.87%> (ø)
.../kyuubi/plugin/spark/authz/PrivilegesBuilder.scala 69.51% <100.00%> (+0.18%) :arrow_up:
.../apache/kyuubi/plugin/spark/authz/v2Commands.scala 90.62% <100.00%> (+0.93%) :arrow_up:
...la/org/apache/kyuubi/ctl/ControlCliException.scala 0.00% <0.00%> (-50.00%) :arrow_down:
.../apache/kyuubi/jdbc/hive/JdbcConnectionParams.java 48.14% <0.00%> (-3.58%) :arrow_down:
...main/scala/org/apache/kyuubi/ctl/cmd/Command.scala 80.00% <0.00%> (-3.34%) :arrow_down:
...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala 78.00% <0.00%> (-2.00%) :arrow_down:
...c/main/java/org/apache/kyuubi/jdbc/hive/Utils.java 35.95% <0.00%> (-1.82%) :arrow_down:
... and 14 more

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

codecov-commenter avatar Sep 20 '22 05:09 codecov-commenter

cc @yaooqinn , please have a check if some time available.

bowenliang123 avatar Sep 23 '22 14:09 bowenliang123

merged to master

yaooqinn avatar Oct 11 '22 01:10 yaooqinn

late lgtm

ulysses-you avatar Oct 11 '22 01:10 ulysses-you

Thanks very much for the guidance and patience from all of you.

And now with this PR with Iceberg catalog/extension integrated, it also helps us to test more features and checks on Spark V2 catalogs. The previously introduced V2JDBCcatalog is not fully functioned with all V2 catalog features.

bowenliang123 avatar Oct 11 '22 02:10 bowenliang123