Skip to content

[Authz] Serde for queries#4044

Closed
yaooqinn wants to merge 7 commits intoapache:masterfrom
yaooqinn:scan
Closed

[Authz] Serde for queries#4044
yaooqinn wants to merge 7 commits intoapache:masterfrom
yaooqinn:scan

Conversation

@yaooqinn
Copy link
Copy Markdown
Member

@yaooqinn yaooqinn commented Dec 30, 2022

Why are the changes needed?

Add serde layer for queries, which extracts table information from a scan relation, such as HiveTableRelation, logical relation, etc.

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

@bowenliang123
Copy link
Copy Markdown
Contributor

We should also consider ScanSpec for building function privileges of perm function usages in queries, as in pending PR 3633 (#3633). The core logic to this is scanning logical plan by transformAllExpressions and build function privileges for UDFs.

@yaooqinn
Copy link
Copy Markdown
Member Author

yaooqinn commented Jan 3, 2023

We should also consider ScanSpec for building function privileges of perm function usages in queries

How can ScanSpec configure functions?

@bowenliang123
Copy link
Copy Markdown
Contributor

bowenliang123 commented Jan 3, 2023

How can ScanSpec configure functions?

If ScanSpec is used for extracting all related resources, functions could be considered one type among them.
Or if it is limited to scanned resources, the used function support in queries can be introduced later.

@yaooqinn yaooqinn changed the title [WIP][Authz] Serde for queries [Authz] Serde for queries Jan 4, 2023
@bowenliang123
Copy link
Copy Markdown
Contributor

bowenliang123 commented Jan 4, 2023

LGTM. Just some minor comments. Let's merge it after passing the CI.

Glad to see this PR reaching a milestone for Serde refactoring in Authz module, by completely removing hardcoded rules and extraction of v2Commands and IcebergCommands, and using spec generation instead.

Big applause to @yaooqinn for your elegant redesign and all the hard work. 👏🏻 🍻

@yaooqinn yaooqinn closed this in c3741ae Jan 4, 2023
@yaooqinn yaooqinn added this to the v1.7.0 milestone Jan 4, 2023
@yaooqinn yaooqinn self-assigned this Jan 4, 2023
@yaooqinn
Copy link
Copy Markdown
Member Author

yaooqinn commented Jan 4, 2023

thanks @bowenliang123 for the check, merged to master

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.

2 participants