Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[KYUUBI #5690][AUTHZ] Support insert into/overwrite path-based table for Delta Lake in Authz #5691

Closed
wants to merge 3 commits into from

Conversation

zml1206
Copy link
Contributor

@zml1206 zml1206 commented Nov 14, 2023

Why are the changes needed?

To close #5690 .
Support insert into/overwrite path-based table for Delta Lake in Authz plugin.

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.

@zml1206
Copy link
Contributor Author

zml1206 commented Nov 14, 2023

insert into

== Analyzed Logical Plan ==
AppendData RelationV2[id#1569, name#1570, gender#1571, birthDate#1572] spark_catalog.delta.`/var/folders/gc/c__qhntd7s502txfp0ltxh880000gn/T/kyuubi-13484fea-4c65-4acf-b3e9-1d81d3d1d16a` delta.`file:/var/folders/gc/c__qhntd7s502txfp0ltxh880000gn/T/kyuubi-13484fea-4c65-4acf-b3e9-1d81d3d1d16a`, false
+- Project [id#1565, name#1566, gender#1567, birthDate#1568]
   +- SubqueryAlias spark_catalog.delta_ns.table2_delta
      +- Relation spark_catalog.delta_ns.table2_delta[id#1565,name#1566,gender#1567,birthDate#1568] parquet

insert overwrite

== Analyzed Logical Plan ==
OverwriteByExpression RelationV2[id#1761, name#1762, gender#1763, birthDate#1764] spark_catalog.delta.`/var/folders/gc/c__qhntd7s502txfp0ltxh880000gn/T/kyuubi-13484fea-4c65-4acf-b3e9-1d81d3d1d16a` delta.`file:/var/folders/gc/c__qhntd7s502txfp0ltxh880000gn/T/kyuubi-13484fea-4c65-4acf-b3e9-1d81d3d1d16a`, true, false
+- Project [id#1757, name#1758, gender#1759, birthDate#1760]
   +- SubqueryAlias spark_catalog.delta_ns.table2_delta
      +- Relation spark_catalog.delta_ns.table2_delta[id#1757,name#1758,gender#1759,birthDate#1760] parquet

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2023

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (2568b0d) 61.46% compared to head (e1506ca) 61.40%.
Report is 5 commits behind head on master.

Files Patch % Lines
...ubi/plugin/spark/authz/serde/tableExtractors.scala 57.14% 0 Missing and 3 partials ⚠️
...yuubi/plugin/spark/authz/serde/uriExtractors.scala 62.50% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5691      +/-   ##
============================================
- Coverage     61.46%   61.40%   -0.06%     
  Complexity       23       23              
============================================
  Files           604      607       +3     
  Lines         35697    35735      +38     
  Branches       4889     4896       +7     
============================================
+ Hits          21942    21944       +2     
- Misses        11380    11402      +22     
- Partials       2375     2389      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

val maybeCatalogPlugin = invokeAs[Option[AnyRef]](v2Relation, "catalog")
val maybeCatalog = maybeCatalogPlugin.flatMap(catalogPlugin =>
plan.find(_.getClass.getSimpleName == "DataSourceV2Relation") match {
case Some(v2Relation @ DataSourceV2Relation(table, _, catalog, identifier, _))
Copy link
Member

Choose a reason for hiding this comment

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

Let‘s use v2Relation.xx instead of unapply which is easy to break

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

override def apply(spark: SparkSession, v1: AnyRef): Seq[Uri] = {
val plan = v1.asInstanceOf[LogicalPlan]
plan.find(_.getClass.getSimpleName == "DataSourceV2Relation") match {
case Some(DataSourceV2Relation(_, _, _, Some(identifier), _))
Copy link
Member

Choose a reason for hiding this comment

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

ditto

@yaooqinn yaooqinn added this to the v1.9.0 milestone Nov 16, 2023
@yaooqinn
Copy link
Member

thanks, merged to master

@yaooqinn yaooqinn closed this in 23f32cf Nov 16, 2023
@zml1206
Copy link
Contributor Author

zml1206 commented Nov 16, 2023

Thanks, @yaooqinn

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 insert into/overwrite path-based table for Delta Lake in Authz
3 participants