yabola commented on code in PR #5248:
URL: https://github.com/apache/kyuubi/pull/5248#discussion_r1315367863


##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/serde/tableExtractors.scala:
##########
@@ -132,6 +134,34 @@ class IdentifierTableExtractor extends TableExtractor {
   }
 }
 
+class IcebergCallCommandExtractor extends TableExtractor {
+  override def apply(spark: SparkSession, callStatementArgs: AnyRef): 
Option[Table] = {
+    val literals = callStatementArgs.asInstanceOf[Seq[Literal]]
+    if (literals.isEmpty) {
+      throw new IllegalArgumentException(
+        s"Unknown parameters in iceberg call command: $callStatementArgs")
+    }
+    // iceberg will rearrange the parameters according to the parameter order
+    // defined in the procedure, where the table parameters are currently 
always the first.
+    lookupExtractor[IcebergTableExtractor].apply(spark, literals.head)

Review Comment:
   iceberg will rearrange the parameters according to the parameter order 
defined in the procedure, where the table parameters are currently always the 
first.
   For example `RewriteDataFilesProcedure#PARAMETERS` and in 
`RewriteDataFilesProcedure#call` take the first parameter.



##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/serde/tableExtractors.scala:
##########
@@ -145,14 +175,37 @@ class DataSourceV2RelationTableExtractor extends 
TableExtractor {
         val maybeCatalogPlugin = invokeAs[Option[AnyRef]](v2Relation, 
"catalog")
         val maybeCatalog = maybeCatalogPlugin.flatMap(catalogPlugin =>
           lookupExtractor[CatalogPluginCatalogExtractor].apply(catalogPlugin))
-        val maybeIdentifier = invokeAs[Option[AnyRef]](v2Relation, 
"identifier")

Review Comment:
   The `identifier` in `DataSourceV2Relation` is not reliable enough. In fact, 
datasourceV2 will parse out the table based on identifier, so table is more 
appropriate here.
   For example, in iceberg `rewrite_data_files` command, `identifier` is 
rewrite task id, iceberg catalog will load table by it ( can see in 
`SparkCatalog#load` the `isPathIdentifier` function)



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to