[incubator-kyuubi] branch master updated: [KYUUBI #2391] Fix privileges builder return wrong result when there is no project but has filter/join

[ya...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

yao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-kyuubi.git


The following commit(s) were added to refs/heads/master by this push:
     new 88f168c67 [KYUUBI #2391] Fix privileges builder return wrong result when there is no project but has filter/join
88f168c67 is described below

commit 88f168c6722dc5529c5c028b4b4294b9681dbf79
Author: packyan <pa...@gmail.com>
AuthorDate: Sat Apr 16 22:30:35 2022 +0800

    [KYUUBI #2391] Fix privileges builder return wrong result when there is no project but has filter/join
    
    ### _Why are the changes needed?_
    
    To close #2391
    Fixed an issue where PrivilegesBuilder would generate incorrect PrivilegeObjects when there is no Project operator in the LogicalPlan but there has Filter or Join operators.
    
    ### _How was this patch tested?_
    - [x] 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.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
    
    Closes #2393 from packyan/branch-fix-privileges-build-wrong-result.
    
    Closes #2391
    
    361c6148 [packyan] fix privileges builder return wrong result when there is no project op
    
    Authored-by: packyan <pa...@gmail.com>
    Signed-off-by: Kent Yao <ya...@apache.org>
---
 .../plugin/spark/authz/PrivilegesBuilder.scala     | 19 ++++----
 .../spark/authz/PrivilegesBuilderSuite.scala       | 57 ++++++++++++++++++++++
 2 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
index 3e7d503d8..b0ea52b49 100644
--- a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
+++ b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
@@ -76,7 +76,8 @@ object PrivilegesBuilder {
   private def buildQuery(
       plan: LogicalPlan,
       privilegeObjects: ArrayBuffer[PrivilegeObject],
-      projectionList: Seq[NamedExpression] = Nil): Unit = {
+      projectionList: Seq[NamedExpression] = Nil,
+      conditionList: Seq[NamedExpression] = Nil): Unit = {
 
     def mergeProjection(table: CatalogTable, plan: LogicalPlan): Unit = {
       if (projectionList.isEmpty) {
@@ -84,24 +85,24 @@ object PrivilegesBuilder {
           table.identifier,
           table.schema.fieldNames)
       } else {
-        val cols = projectionList.flatMap(collectLeaves)
+        val cols = (projectionList ++ conditionList).flatMap(collectLeaves)
           .filter(plan.outputSet.contains).map(_.name).distinct
         privilegeObjects += tablePrivileges(table.identifier, cols)
       }
     }
 
     plan match {
-      case p: Project => buildQuery(p.child, privilegeObjects, p.projectList)
+      case p: Project => buildQuery(p.child, privilegeObjects, p.projectList, conditionList)
 
       case j: Join =>
         val cols =
-          projectionList ++ j.condition.map(expr => collectLeaves(expr)).getOrElse(Nil)
-        buildQuery(j.left, privilegeObjects, cols)
-        buildQuery(j.right, privilegeObjects, cols)
+          conditionList ++ j.condition.map(expr => collectLeaves(expr)).getOrElse(Nil)
+        buildQuery(j.left, privilegeObjects, projectionList, cols)
+        buildQuery(j.right, privilegeObjects, projectionList, cols)
 
       case f: Filter =>
-        val cols = projectionList ++ collectLeaves(f.condition)
-        buildQuery(f.child, privilegeObjects, cols)
+        val cols = conditionList ++ collectLeaves(f.condition)
+        buildQuery(f.child, privilegeObjects, projectionList, cols)
 
       case hiveTableRelation if hasResolvedHiveTable(hiveTableRelation) =>
         mergeProjection(getHiveTable(hiveTableRelation), hiveTableRelation)
@@ -119,7 +120,7 @@ object PrivilegesBuilder {
 
       case p =>
         for (child <- p.children) {
-          buildQuery(child, privilegeObjects, projectionList)
+          buildQuery(child, privilegeObjects, projectionList, conditionList)
         }
     }
   }
diff --git a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilderSuite.scala b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilderSuite.scala
index 252332e4e..fda9e2814 100644
--- a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilderSuite.scala
+++ b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilderSuite.scala
@@ -920,6 +920,63 @@ abstract class PrivilegesBuilderSuite extends KyuubiFunSuite with SparkSessionPr
     assert(tuple._2.size === 0)
   }
 
+  test("Query: WHERE Without Project") {
+    val sqlStr =
+      s"""
+         |SELECT t1.key, t1.value
+         |    FROM $reusedTable t1
+         |    WHERE key < 1
+         |    """.stripMargin
+
+    val plan = sql(sqlStr).queryExecution.optimizedPlan
+    val operationType = OperationType(plan.nodeName)
+    assert(operationType === QUERY)
+    val tuple = PrivilegesBuilder.build(plan)
+
+    assert(tuple._1.size === 1)
+    tuple._1.foreach { po =>
+      assert(po.actionType === PrivilegeObjectActionType.OTHER)
+      assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
+      assert(po.dbname equalsIgnoreCase reusedDb)
+      assert(po.objectName startsWith reusedTableShort.toLowerCase)
+      assert(
+        po.columns === Seq("key", "value"),
+        s"$reusedPartTable both 'key' and 'value' should be authenticated")
+      val accessType = ranger.AccessType(po, operationType, isInput = true)
+      assert(accessType === AccessType.SELECT)
+    }
+    assert(tuple._2.size === 0)
+  }
+
+  test("Query: JOIN Without Project") {
+    val sqlStr =
+      s"""
+         |SELECT t1.key, t1.value, t2.key, t2.value
+         |    FROM $reusedTable t1
+         |    INNER JOIN $reusedPartTable t2
+         |    ON t1.key = t2.key
+         |    """.stripMargin
+
+    val plan = sql(sqlStr).queryExecution.optimizedPlan
+    val operationType = OperationType(plan.nodeName)
+    assert(operationType === QUERY)
+    val tuple = PrivilegesBuilder.build(plan)
+
+    assert(tuple._1.size === 2)
+    tuple._1.foreach { po =>
+      assert(po.actionType === PrivilegeObjectActionType.OTHER)
+      assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
+      assert(po.dbname equalsIgnoreCase reusedDb)
+      assert(po.objectName startsWith reusedTableShort.toLowerCase)
+      assert(
+        po.columns === Seq("key", "value"),
+        s"$reusedPartTable both 'key' and 'value' should be authenticated")
+      val accessType = ranger.AccessType(po, operationType, isInput = true)
+      assert(accessType === AccessType.SELECT)
+    }
+    assert(tuple._2.size === 0)
+  }
+
   test("Query: Union") {
     val plan = sql(
       s"""