You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/10/23 14:07:56 UTC

[GitHub] [incubator-kyuubi] zhouyifan279 commented on a diff in pull request #3678: [KYUUBI #3672][Subtask] Get table owner for DDL/DML v1 commands

zhouyifan279 commented on code in PR #3678:
URL: https://github.com/apache/incubator-kyuubi/pull/3678#discussion_r1002716465


##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilderSuite.scala:
##########
@@ -123,24 +123,34 @@ abstract class PrivilegesBuilderSuite extends AnyFunSuite
 
   test("AlterTableRenameCommand") {
     withTable(s"$reusedDb.efg") { t =>
-      sql(s"CREATE TABLE IF NOT EXISTS ${reusedTable}_old" +
-        s" (key int, value string) USING parquet")
-      // toLowerCase because of: SPARK-38587
-      val plan =
-        sql(s"ALTER TABLE ${reusedDb.toLowerCase}.${getClass.getSimpleName}_old" +
-          s" RENAME TO $t").queryExecution.analyzed
-      val operationType = OperationType(plan.nodeName)
-      assert(operationType === ALTERTABLE_RENAME)
-      val tuple = PrivilegesBuilder.build(plan, spark)
-      assert(tuple._1.isEmpty)
-      assert(tuple._2.size === 2)
-      tuple._2.foreach { po =>
-        assert(po.privilegeObjectType === PrivilegeObjectType.TABLE_OR_VIEW)
-        assert(po.dbname equalsIgnoreCase reusedDb)
-        assert(Set(reusedDb + "_old", "efg").contains(po.objectName))
-        assert(po.columns.isEmpty)
-        val accessType = ranger.AccessType(po, operationType, isInput = false)
-        assert(Set(AccessType.CREATE, AccessType.DROP).contains(accessType))
+      withTable(s"${reusedTable}_old") { oldTable =>
+        val oldTableShort = oldTable.split("\\.").last
+        val create = s"CREATE TABLE IF NOT EXISTS $oldTable" +
+          s" (key int, value string) USING parquet"
+        sql(create)
+        // toLowerCase because of: SPARK-38587
+        val plan =
+          sql(s"ALTER TABLE ${reusedDb.toLowerCase}.$oldTableShort" +
+            s" RENAME TO $t").queryExecution.analyzed
+        // re-create oldTable because we needs to get table owner from table metadata later
+        sql(create)
+
+        val operationType = OperationType(plan.nodeName)
+        assert(operationType === ALTERTABLE_RENAME)
+        val tuple = PrivilegesBuilder.build(plan, spark)

Review Comment:
   Quite a lot places need to change. I suggest to import them in a separate PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org