[KYUUBI #5529][AUTHZ] Support create table command for Delta Lake #5530

zml1206 · 2023-10-26T06:45:00Z

Why are the changes needed?

To close #5529.
Support create table command for Delta Lake in Authz.
https://docs.delta.io/latest/delta-batch.html#create-a-table

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.

pan3793 · 2023-10-26T06:47:36Z

Nice! cc @bowenliang123 and @AngersZhuuuu

bowenliang123 · 2023-10-26T07:27:56Z

Cool, let's bring Ranger authorisation to Delta in Kyuubi Authz !

bowenliang123

LGTM.
Would be more clare if we reorder the test cases in the order for the non-partitioned table, the partitioned table, and then the create/replace table. Or just separated into separated ut.

cfmcgrady · 2023-10-26T07:44:13Z

delta lake also supports creating a table at a path

-- Create or replace table with path
CREATE OR REPLACE TABLE delta.`/tmp/delta/people10m` (
  id INT,
  firstName STRING,
  middleName STRING,
  lastName STRING,
  gender STRING,
  birthDate TIMESTAMP,
  ssn STRING,
  salary INT
) USING DELTA

Shall we also add UTs for this case?

bowenliang123 · 2023-10-26T07:46:50Z

Path-based tables are supported in Authz now. No path based policies (eg. HDFS service def) of Ranger are supported.

zml1206 · 2023-10-26T07:50:06Z

LGTM. Would be more clare if we reorder the test cases in the order for the non-partitioned table, the partitioned table, and then the create/replace table. Or just separated into separated ut.

Thanks, updated.

cfmcgrady · 2023-10-26T08:07:07Z

Path-based tables are supported in Authz now. No path based policies (eg. HDFS service def) of Ranger are supported.

Does Delta Lake have an option for the globally disabled path-based table, similar to how spark.sql.runSQLOnFiles works? If not, I think we can introduce a configuration in Kyuubi AuthZ to disable this behavior temporarily, prior to the completion of the path-based table feature. WDYT? cc @yaooqinn

yaooqinn · 2023-10-26T09:52:18Z

@AngersZhuuuu is currently working one the path/uri authorization

What's the plan of CREATE OR REPLACE TABLE delta./tmp/delta/people10m

zml1206 · 2023-10-26T10:36:22Z

Path-based tables are supported in Authz now. No path based policies (eg. HDFS service def) of Ranger are supported.

Does Delta Lake have an option for the globally disabled path-based table, similar to how spark.sql.runSQLOnFiles works? If not, I think we can introduce a configuration in Kyuubi AuthZ to disable this behavior temporarily, prior to the completion of the path-based table feature. WDYT? cc @yaooqinn

https://github.com/delta-io/delta/blob/8639c411890a5c77386f04e2282fcf4caa401eff/spark/src/test/scala/org/apache/spark/sql/delta/DeltaDDLUsingPathSuite.scala#L125
Delta Lake is also controlled by spark.sql.runSQLOnFiles.

zml1206 · 2023-10-26T11:07:24Z

@AngersZhuuuu is currently working one the path/uri authorization

What's the plan of CREATE OR REPLACE TABLE delta./tmp/delta/people10m

== Analyzed Logical Plan ==
ReplaceTable [StructField(id,IntegerType,true)], TableSpec(Map(),Some(DELTA),Map(),None,None,None,false), true
+- ResolvedDBObjectName org.apache.spark.sql.delta.catalog.DeltaCatalog@2f9addd4, [delta, /tmp/delta/people10m]

== Optimized Logical Plan ==
CommandResult AtomicReplaceTable org.apache.spark.sql.delta.catalog.DeltaCatalog@2f9addd4, delta.`/tmp/delta/people10m`, [StructField(id,IntegerType,true)], TableSpec(Map(),Some(DELTA),Map(),None,None,None,false), true, org.apache.spark.sql.execution.datasources.v2.DataSourceV2Strategy$$Lambda$1627/2047805101@71b3ba5a
   +- ReplaceTable [StructField(id,IntegerType,true)], TableSpec(Map(),Some(DELTA),Map(),None,None,None,false), true
      +- ResolvedDBObjectName org.apache.spark.sql.delta.catalog.DeltaCatalog@2f9addd4, [delta, /tmp/delta/people10m]

cfmcgrady · 2023-10-27T02:01:42Z

Delta Lake is also controlled by spark.sql.runSQLOnFiles.

thank you for your input. @zml1206

codecov-commenter · 2023-10-27T02:28:40Z

Codecov Report

Merging #5530 (facd8f7) into master (322cb48) will decrease coverage by 0.03%.
Report is 2 commits behind head on master.
The diff coverage is n/a.

❗ Current head facd8f7 differs from pull request most recent head b8ed2a4. Consider uploading reports for the commit b8ed2a4 to get more accurate results

@@             Coverage Diff              @@
##             master    #5530      +/-   ##
============================================
- Coverage     61.35%   61.32%   -0.03%     
  Complexity       23       23              
============================================
  Files           598      598              
  Lines         34260    34254       -6     
  Branches       4489     4488       -1     
============================================
- Hits          21019    21007      -12     
- Misses        11113    11118       +5     
- Partials       2128     2129       +1

see 15 files with indirect coverage changes

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

cfmcgrady · 2023-10-27T08:37:41Z

thanks, merging to master(v1.9.0).

yaooqinn · 2023-11-01T05:45:14Z

...cala/org/apache/kyuubi/plugin/spark/authz/ranger/DeltaCatalogRangerSparkExtensionSuite.scala

+      interceptContains[AccessControlException] {
+        doAs(someone, sql(createNonPartitionTableSql))
+      }(s"does not have [create] privilege on [$namespace1/$table1]")
+      doAs(admin, createNonPartitionTableSql)


Hi @zml1206, this line has an issue that it does not trigger a sql operation.

Yes, I'll fix it right away, thanks.

Fix in #5597

yaooqinn · 2023-11-01T05:45:22Z

...cala/org/apache/kyuubi/plugin/spark/authz/ranger/DeltaCatalogRangerSparkExtensionSuite.scala

+      interceptContains[AccessControlException] {
+        doAs(someone, sql(createPartitionTableSql))
+      }(s"does not have [create] privilege on [$namespace1/$table2]")
+      doAs(admin, createPartitionTableSql)


yaooqinn · 2023-11-01T05:45:28Z

...cala/org/apache/kyuubi/plugin/spark/authz/ranger/DeltaCatalogRangerSparkExtensionSuite.scala

+      interceptContains[AccessControlException] {
+        doAs(someone, sql(createOrReplaceTableSql))
+      }(s"does not have [create] privilege on [$namespace1/$table1]")
+      doAs(admin, createOrReplaceTableSql)


Support create table command for Delta Lake

23f6b81

github-actions bot added module:spark kind:build module:extensions module:authz labels Oct 26, 2023

bowenliang123 approved these changes Oct 26, 2023

View reviewed changes

update ut

b2a9543

fix show databases ut

facd8f7

Merge branch 'master' into KYUUBI-5529

10138d8

zml1206 and others added 3 commits October 27, 2023 12:05

fix check spec json files

fb4223b

Merge branch 'master' into KYUUBI-5529

036154b

resolve conflicts

c025230

yaooqinn approved these changes Oct 27, 2023

View reviewed changes

zml1206 added 2 commits October 27, 2023 14:22

Merge branch 'master' into KYUUBI-5529

f12ff8f

update

b8ed2a4

yaooqinn assigned zml1206 Oct 27, 2023

yaooqinn modified the milestones: v1.8.0, v1.9.0 Oct 27, 2023

cfmcgrady approved these changes Oct 27, 2023

View reviewed changes

cfmcgrady closed this in cfd90e0 Oct 27, 2023

yaooqinn reviewed Nov 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[KYUUBI #5529][AUTHZ] Support create table command for Delta Lake #5530

[KYUUBI #5529][AUTHZ] Support create table command for Delta Lake #5530

zml1206 commented Oct 26, 2023

pan3793 commented Oct 26, 2023

bowenliang123 commented Oct 26, 2023

bowenliang123 left a comment

cfmcgrady commented Oct 26, 2023

bowenliang123 commented Oct 26, 2023

zml1206 commented Oct 26, 2023 •

edited

Loading

cfmcgrady commented Oct 26, 2023

yaooqinn commented Oct 26, 2023

zml1206 commented Oct 26, 2023

zml1206 commented Oct 26, 2023

cfmcgrady commented Oct 27, 2023

codecov-commenter commented Oct 27, 2023 •

edited

Loading

cfmcgrady commented Oct 27, 2023

yaooqinn Nov 1, 2023

zml1206 Nov 1, 2023

zml1206 Nov 1, 2023

yaooqinn Nov 1, 2023

yaooqinn Nov 1, 2023

[KYUUBI #5529][AUTHZ] Support create table command for Delta Lake #5530

[KYUUBI #5529][AUTHZ] Support create table command for Delta Lake #5530

Conversation

zml1206 commented Oct 26, 2023

Why are the changes needed?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

pan3793 commented Oct 26, 2023

bowenliang123 commented Oct 26, 2023

bowenliang123 left a comment

Choose a reason for hiding this comment

cfmcgrady commented Oct 26, 2023

bowenliang123 commented Oct 26, 2023

zml1206 commented Oct 26, 2023 • edited Loading

cfmcgrady commented Oct 26, 2023

yaooqinn commented Oct 26, 2023

zml1206 commented Oct 26, 2023

zml1206 commented Oct 26, 2023

cfmcgrady commented Oct 27, 2023

codecov-commenter commented Oct 27, 2023 • edited Loading

Codecov Report

cfmcgrady commented Oct 27, 2023

yaooqinn Nov 1, 2023

Choose a reason for hiding this comment

zml1206 Nov 1, 2023

Choose a reason for hiding this comment

zml1206 Nov 1, 2023

Choose a reason for hiding this comment

yaooqinn Nov 1, 2023

Choose a reason for hiding this comment

yaooqinn Nov 1, 2023

Choose a reason for hiding this comment

zml1206 commented Oct 26, 2023 •

edited

Loading

codecov-commenter commented Oct 27, 2023 •

edited

Loading