Skip to content

Commit

Permalink
[KYUUBI #5557][AUTHZ] Refactor code about handle PermanentViewMarker
Browse files Browse the repository at this point in the history
### _Why are the changes needed?_
To close #5557
Refactor code about handle PermanentViewMarker

### _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

- [x] [Run test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests) locally before make a pull request

### _Was this patch authored or co-authored using generative AI tooling?_
No

Closes #5558 from AngersZhuuuu/KYUUBI-5557.

Closes #5557

ff55e50 [Angerszhuuuu] [KYUUBI #5557][AUTHZ] Refactor code about handle PermanentViewMarker

Authored-by: Angerszhuuuu <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
  • Loading branch information
AngersZhuuuu authored and pan3793 committed Oct 30, 2023
1 parent c290f20 commit e707bbc
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,15 @@ object PrivilegesBuilder {
conditionList: Seq[NamedExpression] = Nil,
spark: SparkSession): Unit = {

def getOutputColumnNames(plan: LogicalPlan): Seq[String] = {
plan match {
case pvm: PermanentViewMarker
if pvm.isSubqueryExpressionPlaceHolder || pvm.output.isEmpty =>
pvm.visitColNames
case _ =>
plan.output.map(_.name)
}
}

def mergeProjection(table: Table, plan: LogicalPlan): Unit = {
if (projectionList.isEmpty) {
privilegeObjects += PrivilegeObject(table, getOutputColumnNames(plan))
plan match {
case pvm: PermanentViewMarker
if pvm.isSubqueryExpressionPlaceHolder || pvm.output.isEmpty =>
privilegeObjects += PrivilegeObject(table, pvm.outputColNames)
case _ =>
privilegeObjects += PrivilegeObject(table, plan.output.map(_.name))
}
} else {
val cols = (projectionList ++ conditionList).flatMap(collectLeaves)
.filter(plan.outputSet.contains).map(_.name).distinct
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import org.apache.kyuubi.plugin.spark.authz.util.WithInternalChild
case class PermanentViewMarker(
child: LogicalPlan,
catalogTable: CatalogTable,
visitColNames: Seq[String],
outputColNames: Seq[String],
isSubqueryExpressionPlaceHolder: Boolean = false) extends UnaryNode
with WithInternalChild {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class RuleApplyPermanentViewMarker extends Rule[LogicalPlan] {
plan mapChildren {
case p: PermanentViewMarker => p
case permanentView: View if hasResolvedPermanentView(permanentView) =>
val resolvedSubquery = permanentView.transformAllExpressions {
val resolved = permanentView.transformAllExpressions {
case subquery: SubqueryExpression =>
subquery.withNewPlan(plan =
PermanentViewMarker(
Expand All @@ -45,10 +45,7 @@ class RuleApplyPermanentViewMarker extends Rule[LogicalPlan] {
permanentView.output.map(_.name),
true))
}
PermanentViewMarker(
resolvedSubquery,
resolvedSubquery.desc,
resolvedSubquery.output.map(_.name))
PermanentViewMarker(resolved, resolved.desc, resolved.output.map(_.name))
case other => apply(other)
}
}
Expand Down

0 comments on commit e707bbc

Please sign in to comment.