[incubator-kyuubi] branch master updated: [KYUUBI #2414] [KYUUBI apache#2413 ] Fix InsertIntoHiveTableCommand case in PrivilegesBuilder#buildCommand()

[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 b88773237 [KYUUBI #2414] [KYUUBI apache#2413  ] Fix InsertIntoHiveTableCommand case in PrivilegesBuilder#buildCommand()
b88773237 is described below

commit b887732372f81193883ef3e92c9c99e711335da0
Author: packyan <pa...@gmail.com>
AuthorDate: Tue Apr 19 11:20:32 2022 +0800

    [KYUUBI #2414] [KYUUBI apache#2413  ] Fix InsertIntoHiveTableCommand case in PrivilegesBuilder#buildCommand()
    
    …gesBuilder#buildCommand()
    
    ### _Why are the changes needed?_
    
    1. To close #2413
    2. To move unit test case `AlterTableAddColumnsCommand` from the end of `HiveCatalogPrivilegeBuilderSuite` to the end of `PrivilegesBuilderSuite`. It is my mistake, this case should be tested both in InmemoryCatalog and HiveCatalog.
    
    ### _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 #2414 from packyan/bugfix-insert-command-should-return-insert-action-type.
    
    Closes #2414
    
    13dd1c60 [packyan] [KYUUBI apache#2413  ] Fix style violations
    589882f3 [packyan] [KYUUBI apache#2413  ] Fix InsertIntoHiveTableCommand case in PrivilegesBuilder#buildCommand()
    
    Authored-by: packyan <pa...@gmail.com>
    Signed-off-by: Kent Yao <ya...@apache.org>
---
 .../plugin/spark/authz/PrivilegesBuilder.scala     | 10 +++-
 .../spark/authz/PrivilegesBuilderSuite.scala       | 68 ++++++++++++++++------
 2 files changed, 57 insertions(+), 21 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 a17187432..956a18ba3 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
@@ -303,8 +303,7 @@ object PrivilegesBuilder {
         outputObjs += tablePrivileges(table)
 
       case "CreateDataSourceTableAsSelectCommand" |
-          "OptimizedCreateHiveTableAsSelectCommand" |
-          "InsertIntoHiveTable" =>
+          "OptimizedCreateHiveTableAsSelectCommand" =>
         val table = getPlanField[CatalogTable]("table").identifier
         outputObjs += tablePrivileges(table)
         buildQuery(getQuery, inputObjs)
@@ -383,6 +382,13 @@ object PrivilegesBuilder {
         // TODO: Should get the table via datasource options?
         buildQuery(getQuery, inputObjs)
 
+      case "InsertIntoHiveTable" =>
+        val table = getPlanField[CatalogTable]("table").identifier
+        val overwrite = getPlanField[Boolean]("overwrite")
+        val actionType = if (overwrite) INSERT_OVERWRITE else INSERT
+        outputObjs += tablePrivileges(table, actionType = actionType)
+        buildQuery(getQuery, inputObjs)
+
       case "LoadDataCommand" =>
         val table = getPlanField[TableIdentifier]("table")
         val overwrite = getPlanField[Boolean]("isOverwrite")
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 ee9ce44fc..12b762418 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
@@ -1079,6 +1079,26 @@ abstract class PrivilegesBuilderSuite extends KyuubiFunSuite with SparkSessionPr
       s"SELECT CASE WHEN key > 0 THEN 'big' ELSE 'small' END FROM $reusedTable",
       Seq("key"))
   }
+
+  // ALTER TABLE ADD COLUMNS It should be run at end
+  // to prevent affecting the cases that rely on the $reusedTable's table structure
+  test("AlterTableAddColumnsCommand") {
+    val plan = sql(s"ALTER TABLE $reusedTable" +
+      s" ADD COLUMNS (a int)").queryExecution.analyzed
+    val operationType = OperationType(plan.nodeName)
+    assert(operationType === ALTERTABLE_ADDCOLS)
+    val tuple = PrivilegesBuilder.build(plan)
+    assert(tuple._1.isEmpty)
+    assert(tuple._2.size === 1)
+    val po = tuple._2.head
+    assert(po.actionType === PrivilegeObjectActionType.OTHER)
+    assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
+    assert(po.dbname equalsIgnoreCase reusedDb)
+    assert(po.objectName === getClass.getSimpleName)
+    assert(po.columns.head === "a")
+    val accessType = ranger.AccessType(po, operationType, isInput = false)
+    assert(accessType === AccessType.ALTER)
+  }
 }
 
 class InMemoryPrivilegeBuilderSuite extends PrivilegesBuilderSuite {
@@ -1218,6 +1238,35 @@ class HiveCatalogPrivilegeBuilderSuite extends PrivilegesBuilderSuite {
     assert(accessType === AccessType.CREATE)
   }
 
+  test("InsertIntoHiveTableCommand") {
+    assume(!isSparkV2)
+    val tableName = "InsertIntoHiveTable"
+    withTable(tableName) { _ =>
+      sql(s"CREATE TABLE $tableName (a int, b string) USING hive")
+      val sqlStr =
+        s"""
+           |INSERT INTO $tableName VALUES (1, "KYUUBI")
+           |""".stripMargin
+      val plan = sql(sqlStr).queryExecution.analyzed
+      val operationType = OperationType(plan.nodeName)
+      assert(operationType === QUERY)
+      val (inputs, outputs) = PrivilegesBuilder.build(plan)
+
+      assert(inputs.isEmpty)
+
+      assert(outputs.size === 1)
+      outputs.foreach { po =>
+        assert(po.actionType === PrivilegeObjectActionType.INSERT)
+        assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
+        assert(po.dbname equalsIgnoreCase "default")
+        assert(po.objectName equalsIgnoreCase tableName)
+        assert(po.columns.isEmpty)
+        val accessType = ranger.AccessType(po, operationType, isInput = false)
+        assert(accessType === AccessType.UPDATE)
+      }
+    }
+  }
+
   test("ShowCreateTableAsSerdeCommand") {
     assume(!isSparkV2)
     withTable("ShowCreateTableAsSerdeCommand") { t =>
@@ -1239,23 +1288,4 @@ class HiveCatalogPrivilegeBuilderSuite extends PrivilegesBuilderSuite {
       assert(tuple._2.size === 0)
     }
   }
-// ALTER TABLE ADD COLUMNS It should be run at end
-// to prevent affecting the cases that rely on the $reusedTable's table structure
-  test("AlterTableAddColumnsCommand") {
-    val plan = sql(s"ALTER TABLE $reusedTable" +
-      s" ADD COLUMNS (a int)").queryExecution.analyzed
-    val operationType = OperationType(plan.nodeName)
-    assert(operationType === ALTERTABLE_ADDCOLS)
-    val tuple = PrivilegesBuilder.build(plan)
-    assert(tuple._1.isEmpty)
-    assert(tuple._2.size === 1)
-    val po = tuple._2.head
-    assert(po.actionType === PrivilegeObjectActionType.OTHER)
-    assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
-    assert(po.dbname equalsIgnoreCase reusedDb)
-    assert(po.objectName === getClass.getSimpleName)
-    assert(po.columns.head === "a")
-    val accessType = ranger.AccessType(po, operationType, isInput = false)
-    assert(accessType === AccessType.ALTER)
-  }
 }