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/kyuubi.git


The following commit(s) were added to refs/heads/master by this push:
     new 8f6b15c91 [KYUUBI #5417] should not check in-subquery in permanent view
8f6b15c91 is described below

commit 8f6b15c91774264fcafe71b38d29fb81807e4df7
Author: Angerszhuuuu <[email protected]>
AuthorDate: Tue Oct 17 22:30:58 2023 +0800

    [KYUUBI #5417] should not check in-subquery in permanent view
    
    ### _Why are the changes needed?_
    Fix #5417
    If there is is a view with in-subquery, authz will still request this 
in-subquery's interval privilege, it's not we want.
    For view
    ```
    CREATE VIEW db.view1
    AS
    WITH temp AS (
        SELECT max(scope) max_scope
        FROM db1.table1)
    SELECT id as new_id FROM db1.table2
    WHERE scope in (SELECT max_scope FROM temp)
    ```
    
    When we query the view
    ```
    SEELCT * FROM db.view1
    ```
    
    Before this pr, since spark will first execute subquery, it will first 
request `[default/table1/scope]` then request `[default/view1/new_id]`
    
    after this pr, it only request  `[default/view1/new_id]`
    
    ### _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
    
    - [ ] [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 #5442 from AngersZhuuuu/KYUUBI-5417-FOLLOWUP.
    
    Closes #5417
    
    6919903cb [Angerszhuuuu] Update RuleApplyPermanentViewMarker.scala
    5097d8059 [Angerszhuuuu] [KYUUBI #5417] should not check in-subquery in 
permanent view
    
    Authored-by: Angerszhuuuu <[email protected]>
    Signed-off-by: Kent Yao <[email protected]>
---
 .../ranger/RuleApplyPermanentViewMarker.scala      |  6 +--
 .../authz/ranger/RangerSparkExtensionSuite.scala   | 46 +++++++++++++++++++++-
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git 
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
 
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
index 917410807..679b5d65d 100644
--- 
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
+++ 
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
@@ -17,7 +17,7 @@
 
 package org.apache.kyuubi.plugin.spark.authz.ranger
 
-import org.apache.spark.sql.catalyst.expressions.ScalarSubquery
+import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
 import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, View}
 import org.apache.spark.sql.catalyst.rules.Rule
 
@@ -38,11 +38,11 @@ class RuleApplyPermanentViewMarker extends 
Rule[LogicalPlan] {
       case p: PermanentViewMarker => p
       case permanentView: View if hasResolvedPermanentView(permanentView) =>
         val resolvedSubquery = permanentView.transformAllExpressions {
-          case scalarSubquery: ScalarSubquery =>
+          case subquery: SubqueryExpression =>
             // TODO: Currently, we do not do an auth check in the subquery
             //  as the main query part also secures it. But for performance 
consideration,
             //  we also pre-check it in subqueries and fail fast with negative 
privileges.
-            scalarSubquery.copy(plan = 
PermanentViewMarker(scalarSubquery.plan, null))
+            subquery.withNewPlan(plan = PermanentViewMarker(subquery.plan, 
null))
         }
         PermanentViewMarker(resolvedSubquery, resolvedSubquery.desc)
       case other => apply(other)
diff --git 
a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
 
b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
index d109a7f2b..8e1fe0587 100644
--- 
a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
+++ 
b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
@@ -748,7 +748,7 @@ class HiveCatalogRangerSparkExtensionSuite extends 
RangerSparkExtensionSuite {
     }
   }
 
-  test("[KYUUBI #5417] should not check dependent subquery plan privilege") {
+  test("[KYUUBI #5417] should not check scalar-subquery in permanent view") {
     val db1 = defaultDb
     val table1 = "table1"
     val table2 = "table2"
@@ -791,4 +791,48 @@ class HiveCatalogRangerSparkExtensionSuite extends 
RangerSparkExtensionSuite {
       assert(e2.getMessage.contains(s"does not have [select] privilege on 
[$db1/$view1/new_id]"))
     }
   }
+
+  test("[KYUUBI #5417] should not check in-subquery in permanent view") {
+    val db1 = defaultDb
+    val table1 = "table1"
+    val table2 = "table2"
+    val view1 = "view1"
+    withCleanTmpResources(
+      Seq((s"$db1.$table1", "table"), (s"$db1.$table2", "table"), 
(s"$db1.$view1", "view"))) {
+      doAs(admin, sql(s"CREATE TABLE IF NOT EXISTS $db1.$table1 (id int, scope 
int)"))
+      doAs(admin, sql(s"CREATE TABLE IF NOT EXISTS $db1.$table2 (id int, scope 
int)"))
+
+      val e1 = intercept[AccessControlException] {
+        doAs(
+          someone,
+          sql(
+            s"""
+               |WITH temp AS (
+               |    SELECT max(scope) max_scope
+               |    FROM $db1.$table1)
+               |SELECT id as new_id FROM $db1.$table2
+               |WHERE scope in (SELECT max_scope FROM temp)
+               |""".stripMargin).show())
+      }
+      // Will first check subquery privilege.
+      assert(e1.getMessage.contains(s"does not have [select] privilege on 
[$db1/$table1/scope]"))
+
+      doAs(
+        admin,
+        sql(
+          s"""
+             |CREATE VIEW $db1.$view1
+             |AS
+             |WITH temp AS (
+             |    SELECT max(scope) max_scope
+             |    FROM $db1.$table1)
+             |SELECT id as new_id FROM $db1.$table2
+             |WHERE scope in (SELECT max_scope FROM temp)
+             |""".stripMargin))
+      // Will just check permanent view privilege.
+      val e2 = intercept[AccessControlException](
+        doAs(someone, sql(s"SELECT * FROM $db1.$view1".stripMargin).show()))
+      assert(e2.getMessage.contains(s"does not have [select] privilege on 
[$db1/$view1/new_id]"))
+    }
+  }
 }

Reply via email to