Skip to content

Conversation

@zml1206
Copy link
Contributor

@zml1206 zml1206 commented Oct 30, 2023

Why are the changes needed?

To close #5565.

  • Support Delete from table command for Delta Lake in Authz.
  • Support Insert table command for Delta Lake in Authz.
  • Support Update table command for Delta Lake in Authz.
  • Remove useless org.apache.spark.sql.delta.commands.CreateDeltaTableCommand. Because the logical plans for Delta Lake create table and replace table are org.apache.spark.sql.catalyst.plans.logical.CreateTable, org.apache.spark.sql.catalyst.plans.logical.CreateV2Table and org. apache.spark.sql.catalyst.plans.logical.ReplaceTable.
  • Reduce the fields of createTableSql.

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 zml1206 changed the title Support Delete/Insert/Update table command for Delta Lake [KYUUBI #5565][AUTHZ] Support Delete/Insert/Update table command for Delta Lake Oct 30, 2023
@pan3793
Copy link
Member

pan3793 commented Oct 30, 2023

Why CreateDeltaTableCommand is useless?

Nit: for PR description and code comments, explain WHY is more important than WHAT, since the code usually is clear enough for reviewer to understand what you are doing.

@zml1206
Copy link
Contributor Author

zml1206 commented Oct 30, 2023

Why CreateDeltaTableCommand is useless?

Nit: for PR description and code comments, explain WHY is more important than WHAT, since the code usually is clear enough for reviewer to understand what you are doing.

Before, I misunderstood the role of table_command_spec.json in #5529 ,The logical plans for Delta Lake create table and replace table are org.apache.spark.sql.catalyst.plans.logical.CreateTable and org.apache.spark.sql.catalyst.plans.logical.CreateV2Table and org. apache.spark.sql.catalyst.plans.logical.ReplaceTable.

@pan3793
Copy link
Member

pan3793 commented Oct 30, 2023

@zml1206 for such case, reverting #5529 independent is preferred, to gain more clear commit history

@zml1206
Copy link
Contributor Author

zml1206 commented Oct 31, 2023

Reverting #5529 has conflict, how about create a followup PR for #5529 first to remove useless org.apache.spark.sql.delta.commands.CreateDeltaTableCommand first, and then resubmit #5565? @pan3793

@pan3793
Copy link
Member

pan3793 commented Oct 31, 2023

@zml1206 creating a logic revert PR SGTM, please go ahead

@zml1206
Copy link
Contributor Author

zml1206 commented Nov 1, 2023

New PR #5596

@zml1206 zml1206 closed this Nov 3, 2023
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 Delete/Insert/Update table command for Delta Lake

2 participants