RussellSpitzer commented on a change in pull request #1890:
URL: https://github.com/apache/iceberg/pull/1890#discussion_r538624413



##########
File path: 
spark3/src/main/java/org/apache/iceberg/spark/procedures/BaseProcedure.java
##########
@@ -75,25 +71,15 @@ protected BaseProcedure(TableCatalog tableCatalog) {
   }
 
   // we have to parse both namespace and name as they may be quoted
-  protected Identifier toIdentifier(String namespaceAsString, String name) {
-    String[] namespaceParts = parseMultipartIdentifier(namespaceAsString);
-
-    String[] nameParts = parseMultipartIdentifier(name);
-    Preconditions.checkArgument(nameParts.length == 1, "Name must consist of 
one part: %s", name);
-
-    return Identifier.of(namespaceParts, nameParts[0]);
-  }
-
-  private String[] parseMultipartIdentifier(String identifierAsString) {
+  protected Identifier toIdentifier(String identifier) {
+    Spark3Util.CatalogAndIdentifier catalogAndIdentifier;
     try {
-      ParserInterface parser = spark.sessionState().sqlParser();
-      Seq<String> namePartsSeq = 
parser.parseMultipartIdentifier(identifierAsString);
-      String[] nameParts = new String[namePartsSeq.size()];
-      namePartsSeq.copyToArray(nameParts);
-      return nameParts;
+      catalogAndIdentifier = Spark3Util.catalogAndIdentifier(spark, 
identifier, tableCatalog);

Review comment:
       I did this based on some of the discussion me and Ryan were having
   
   ```
   We know we have a catalog if there is a registered catalog with the name
   
   
   
   
   
   11:45
   In Spark, the rules are:
   1. If the identifier is a single part, it is a catalog name. Fill in current 
catalog and current namespace
   2. If the first part of the identifier is a known catalog, it is a fully 
qualified name
   3. If the first part is not a known catalog, fill in the current catalog 
(not namespace because it is already there)
   11:46
   The only difference here would be that we consider the current catalog to be 
the procedure catalog because this is in the procedure's context
   ```
   
   I thought this was reasonable but we could force the catalog as well




----------------------------------------------------------------
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.

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