[KYUUBI #5780][AUTHZ] Treating PermanentViewMarker as LeafNode make code simple and got correct privilege object

[AngersZhuuuu]

🔍 Description

Issue References 🔗

This pull request fixes #5780

Describe Your Solution 🔧

Currently, we convert persist view to PermanentViewMaker, but after optimizer, it changed its child, making it hard to do column prune and get the right column privilege object of persist view.

In this pr, we change PVM as a LeafNode, then we can directly treat it as a HiveRelation since we won't change its internal plan to make code simpler.
But we need to re-optimize the child plan after do privilege check.

Types of changes 🔖

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

For sql such as below table and view

CREATE TABLE IF NOT EXISTS $db1.$table1(id int, scope int);

CREATE TABLE IF NOT EXISTS $db1.$table2(
id int,
name string,
age int,
scope int)
.stripMargin);

CREATE VIEW $db1.$view1
AS
WITH temp AS (
    SELECT max(scope) max_scope
    FROM $db1.$table1)
SELECT id, name, max(scope) as max_scope, sum(age) sum_age
FROM $db1.$table2
WHERE scope in (SELECT max_scope FROM temp)
GROUP BY id, name

When we execute query on $db1.$view1

SELECT id as new_id, name, max_scope FROM $db1.$view1

It will first execute the subquery in the query, then got a un-correct column privilege

Behavior With This Pull Request 🎉

After this change, since PVM is a LeafNode, we won't execute the subquery under PVM, and we directly got the correct column privilege.

Related Unit Tests

Existed UT

---

Checklists

📝 Author Self Checklist

• My code follows the style guidelines of this project
• I have performed a self-review
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• This patch was not authored or co-authored using Generative Tooling

📝 Committer Pre-Merge Checklist

• Pull request title is okay.
• No license issues.
• Milestone correctly set?
• Test coverage is ok
• Assignees are selected.
• Minimum number of approvals
• No changes are requested

Be nice. Be informative.

[AngersZhuuuu]

@yaooqinn WDYT about this solution?

[codecov-commenter]

Codecov Report

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

Comparison is base (b920836) 61.42% compared to head (3c18bb7) 61.39%.

Files	Patch %	Lines
.../authz/rule/RuleEliminatePermanentViewMarker.scala	70.00%	0 Missing and 3 partials ⚠️
...kyuubi/plugin/spark/authz/rule/Authorization.scala	66.66%	0 Missing and 1 partial ⚠️
...authz/rule/permanentview/PermanentViewMarker.scala	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #5781      +/-   ##
============================================
- Coverage     61.42%   61.39%   -0.03%     
  Complexity       23       23              
============================================
  Files           607      607              
  Lines         35944    35933      -11     
  Branches       4936     4935       -1     
============================================
- Hits          22079    22062      -17     
- Misses        11478    11486       +8     
+ Partials       2387     2385       -2     

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

[AngersZhuuuu]

Any more suggestion?

[yaooqinn]

please take care of the tests

[AngersZhuuuu]

please take care of the tests

Fixed

[AngersZhuuuu]

ping @yaooqinn GA passed and conflict resolved.

[yaooqinn]

Let's fulfill the PR desc

[yaooqinn]

The PR title is for refactoring, but actually what we do here is a bugfix, can we make it more precisely.

[AngersZhuuuu]

The PR title is for refactoring, but actually what we do here is a bugfix, can we make it more precisely.

How about current, desc also full fill.

[AngersZhuuuu]

I don't know why this pr needs to change this...But without this change, GA failed.

[AngersZhuuuu]

cc @yaooqinn

[yaooqinn]

Thank you, merged to master