aturoczy commented on code in PR #4954:
URL: https://github.com/apache/hive/pull/4954#discussion_r1430385183


##########
ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java:
##########
@@ -157,17 +165,32 @@ public boolean fillStorageFormat(ASTNode child) throws 
SemanticException {
     return true;
   }
 
-  private String processStorageHandler(String name) throws SemanticException {
-    for (StorageHandlerTypes type : StorageHandlerTypes.values()) {
-      if (type.name().equalsIgnoreCase(name)) {
-        name = type.className();
-        inputFormat = type.inputFormat();
-        outputFormat = type.outputFormat();
-        break;
+  private String processStorageHandler(ASTNode node) throws SemanticException {
+    if (node.getType() == HiveParser.StringLiteral) {
+      // e.g. STORED BY 'org.apache.iceberg.mr.hive.HiveIcebergStorageHandler'
+      return 
ensureClassExists(BaseSemanticAnalyzer.unescapeSQLString(node.getText()));
+    }
+    if (node.getType() == HiveParser.Identifier) {
+      // e.g. STORED BY ICEBERG
+      for (StorageHandlerTypes type : 
StorageHandlerTypes.SUPPORTED_BY_STORED_BY) {
+        if (type.name().equalsIgnoreCase(node.getText())) {
+          inputFormat = type.inputFormat();
+          outputFormat = type.outputFormat();
+          assert type.className != null;
+          // Should never fail
+          return 
ensureClassExists(BaseSemanticAnalyzer.unescapeSQLString(type.className));
+        }
       }
     }
-
-    return ensureClassExists(BaseSemanticAnalyzer.unescapeSQLString(name));
+    final String supportedTypes = StorageHandlerTypes
+        .SUPPORTED_BY_STORED_BY
+        .stream()
+        .map(Enum::toString)
+        .collect(Collectors.joining(", "));
+    throw  new SemanticException(String.format(
+        "Unrecognized storage handler in STORED BY clause: %s. Supported types 
= %s or FQCN of a storage handler",

Review Comment:
   I like this initiative because hive error message is too massy, and need a 
pilot certificate to understand it. So big plus for it!
   
   On the other hand, I would recommend the following message like:
   The storage handler specified in the STORED BY clause is not recognized: %s. 
Please use one of the supported types, which are %s, or provide the Fully 
Qualified Class Name (FQCN) of a valid storage handler.
   
   
   Even thinking the second half of the sentence is too technical. 



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