deniskuzZ commented on code in PR #6619:
URL: https://github.com/apache/hive/pull/6619#discussion_r3622440365


##########
serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java:
##########
@@ -543,11 +543,35 @@ private TypeInfo parseType() {
               break;
             }
           }
-          Token name = expect("name",">");
-          if (name.text.equals(">")) {
+          // Collect all consecutive tokens until ":" or ">" or "," as the 
field name.
+          // This handles struct field names that:
+          //   (a) start with a valid type-char token, e.g. "gpa_!@#$%^&*()" 
where "gpa_" is
+          //       the first token and "!", "@", ... are appended by the loop 
below, and
+          //   (b) start with a special character, e.g. "!@#$%^&*()_age" where 
"!" is first.
+          // Both cases arise when Iceberg partition column names contain 
special characters
+          // stored as bare (unquoted) strings in Iceberg metadata.
+          //
+          // NOTE: The following characters are NOT supported in bare 
(unquoted) struct field
+          // names as they are structural delimiters in the type grammar:
+          //   ":"  - separates field name from field type
+          //   ">"  - closes the struct definition
+          //   ","  - separates fields
+          // Field names containing these characters would require a quoting 
convention
+          // which is not currently implemented.
+          StringBuilder fieldNameBuilder = new StringBuilder();

Review Comment:
   maybe extract to helper method like `parseStructFieldName`? + need to handle 
empty `struct<>`
   
   ````
   Subject: [PATCH] patch
   ---
   Index: 
serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java
   IDEA additional info:
   Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
   <+>UTF-8
   ===================================================================
   diff --git 
a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java 
b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java
   --- 
a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java      
 (revision ef821c2a22ffdddc56e0280a33356878abbce04c)
   +++ 
b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java      
 (date 1784639578954)
   @@ -296,6 +296,14 @@
          return Character.isLetterOrDigit(c) || c == '_' || c == '.' || c == ' 
' || c == '$';
        }
    
   +    private static Token createToken(String typeInfoString, int start, int 
end) {
   +      Token t = new Token();
   +      t.position = start;
   +      t.text = typeInfoString.substring(start, end).trim();
   +      t.isType = isTypeChar(typeInfoString.charAt(start));
   +      return t;
   +    }
   +
        /**
         * Tokenize the typeInfoString. The rule is simple: all consecutive
         * alphadigits and '_', '.' are in one token, and all other characters 
are
   @@ -333,11 +341,7 @@
            if (end == typeInfoString.length()
                || !isTypeChar(typeInfoString.charAt(end - 1))
                || !isTypeChar(typeInfoString.charAt(end))) {
   -          Token t = new Token();
   -          t.position = begin;
   -          t.text = typeInfoString.substring(begin, end).trim();
   -          t.isType = isTypeChar(typeInfoString.charAt(begin));
   -          tokens.add(t);
   +          tokens.add(createToken(typeInfoString, begin, end));
              begin = end;
            }
            end++;
   @@ -446,6 +450,31 @@
          return params.toArray(new String[params.size()]);
        }
    
   +    private static boolean isStructFieldNameEnd(Token t) {
   +      return t == null || t.text.equals(":") || t.text.equals(">") || 
t.text.equals(",");
   +    }
   +
   +    /**
   +     * A struct field name can be split across multiple tokens, e.g. an 
Iceberg
   +     * partition column name like "gpa_!@#$%^&amp;*()" is tokenized as 
"gpa_", "!",
   +     * "@", ... since only letters/digits/'_'/'.'/' '/'$' form a single 
token.
   +     * Consume tokens until the next structural delimiter: ":" ends the 
name,
   +     * while ">" or "," with nothing consumed means there was no name (end 
of
   +     * struct, or a trailing separator before the close).
   +     */
   +    private String parseStructFieldName() {
   +      StringBuilder fieldName = new StringBuilder();
   +      for (Token next = peek(); !isStructFieldNameEnd(next); next = peek()) 
{
   +        fieldName.append(next.text);
   +        iToken++;
   +      }
   +      if (fieldName.length() == 0) {
   +        expect(">");
   +        return null;
   +      }
   +      return fieldName.toString();
   +    }
   +
        private TypeInfo parseType() {
    
          Token t = expect("type");
   @@ -543,11 +572,11 @@
                  break;
                }
              }
   -          Token name = expect("name",">");
   -          if (name.text.equals(">")) {
   +          String fieldName = parseStructFieldName();
   +          if (fieldName == null) {
                break;
              }
   -          fieldNames.add(name.text);
   +          fieldNames.add(fieldName);
              expect(":");
              fieldTypeInfos.add(parseType());
            } while (true);
   
   ````



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