[KYUUBI #4474][AUTHZ] Support permission control for UPDATE statement with Iceberg 0.12 or below

[Jackhjf]

Why are the changes needed?

To close #4474. I only configure the strategy of select on ranger, but I can execute this sql： update iceberg_spark.test1 set name="3312" where id="22" successfully

How was this patch tested?

01 configure the strategy of select on ranger

02 execute this sql: update iceberg_spark.test1 set name="3312"

[yaooqinn]

can we add update iceberg_spark.test1 set name="3312" to unit tests?

[yaooqinn]

cc @bowenliang123

[bowenliang123]

I'm wondering how we miss this case of updating iceberg table, as we do have update iceberg table in unit test （IcebergCatalogRangerSparkExtensionSuite）. Can you provide more information about the issue in the description, including configs of iceberg plugin, the version of iceberg, the DDL of the target table ? And it's necessary to add the proper unit test for simulation and validation.

[Jackhjf]

I'm wondering how we miss this case of updating iceberg table, as we do have update iceberg table in unit test. Can you provide more information about the issue in the description, including configs of iceberg plugin, the DDL of the target table ? And it's necessary to add the proper unit test for simulation and validation.

OK

iceberg version:

iceberg-spark3-extensions-0.12.0.jar iceberg-spark3-runtime-0.12.0.jar

the table information:

[bowenliang123]

It seems Iceberg implements update command as ReplaceData in 0.12 and below, and UpdateIcebergTable in 0.13 and above. That's the main reason for missing this case in unit tests as Iceberg 1.x used by default.

[Jackhjf]

Seems Iceberg implements update as ReplaceData in 0.12 and below, and UpdateIcebergTable in 0.13 and above. That's the main reason for missing this case in unit tests as Iceberg 1.x used by default.

In addition, the delete operation is also used ReplaceData；This situation also needs to be considered

[bowenliang123]

Btw, you'd better update org.apache.kyuubi.plugin.spark.authz.gen.TableCommands and regenerate table_command_spec.json by running JsonSpecFileGenerator.

[Jackhjf]

Btw, you'd better update org.apache.kyuubi.plugin.spark.authz.gen.TableCommands and regenerate table_command_spec.json by running JsonSpecFileGenerator.

OK，thanks for your advice

[yaooqinn]

thanks @Jackhjf for the PR, just left some comments

[Jackhjf]

A test case shall be added to verify

A test case shall be added to verify

How to write test cases? The previous iceberg table update test case already exists

[codecov-commenter]

Codecov Report

Merging #4475 (3804885) into master (6aac3e6) will decrease coverage by 0.04%. The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #4475      +/-   ##
============================================
- Coverage     53.39%   53.35%   -0.04%     
  Complexity       13       13              
============================================
  Files           571      571              
  Lines         31350    31352       +2     
  Branches       4225     4226       +1     
============================================
- Hits          16738    16728      -10     
- Misses        13032    13046      +14     
+ Partials       1580     1578       -2     

Impacted Files	Coverage Δ
...ache/kyuubi/plugin/spark/authz/OperationType.scala	100.00% <100.00%> (ø)
.../kyuubi/plugin/spark/authz/PrivilegesBuilder.scala	90.42% <100.00%> (+0.20%)	:arrow_up:

... and 8 files with indirect coverage changes

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

[yaooqinn]

How to write test cases? The previous iceberg table update test case already exists

the test cases

• [KYUUBI #3515] UPDATE TABLE
• [KYUUBI #3515] DELETE FROM TABLE are disabled for spark <=3.1, we can enable them

[Jackhjf]

How to write test cases? The previous iceberg table update test case already exists

the test cases

• [KYUUBI #3515] UPDATE TABLE
• [KYUUBI #3515] DELETE FROM TABLE are disabled for spark <=3.1, we can enable them

The master branch has been merged into this test case

Can this PR be merged into the master branch now?

[bowenliang123]

https://github.com/apache/kyuubi/pull/4493 is already merged to master. You could merge master to your PR branch or rebase your changes to master.

[Jackhjf]

merge master to your PR branch

#4493 is already merged to master. You could merge master to your PR branch or rebase your changes to master.

I already merge master to my PR_4474 branch