From 4d49b9aa4835427af13ffe01bdb6946e9617c04b Mon Sep 17 00:00:00 2001 From: architjainjain Date: Tue, 21 Jul 2026 00:32:11 +0530 Subject: [PATCH] HIVE-29749: parsing special chars in column identifiers --- .../iceberg_partition_special_chars.q | 31 ++++++ .../iceberg_partition_special_chars.q.out | 97 +++++++++++++++++++ .../hive/serde2/typeinfo/TypeInfoUtils.java | 46 +++++++-- .../serde2/typeinfo/TestTypeInfoUtils.java | 48 ++++++++- 4 files changed, 213 insertions(+), 9 deletions(-) create mode 100644 iceberg/iceberg-handler/src/test/queries/positive/iceberg_partition_special_chars.q create mode 100644 iceberg/iceberg-handler/src/test/results/positive/iceberg_partition_special_chars.q.out diff --git a/iceberg/iceberg-handler/src/test/queries/positive/iceberg_partition_special_chars.q b/iceberg/iceberg-handler/src/test/queries/positive/iceberg_partition_special_chars.q new file mode 100644 index 000000000000..f117eb1f97c2 --- /dev/null +++ b/iceberg/iceberg-handler/src/test/queries/positive/iceberg_partition_special_chars.q @@ -0,0 +1,31 @@ +CREATE TABLE studenttab10k( + name string, + age int, + gpa double +) +ROW FORMAT DELIMITED FIELDS TERMINATED BY '\t' +STORED AS TEXTFILE; + +LOAD DATA LOCAL INPATH '../../data/files/studenttab10k' OVERWRITE INTO TABLE studenttab10k; + + +set hive.support.quoted.identifiers=column; + +drop table if exists quoted_identifier_2_1; + +create table quoted_identifier_2_1(`-+={[<_name_>]}?/|` string, `!@#$%^&*()_age` int) partitioned by(`gpa_!@#$%^&*()` double) stored by iceberg; + +insert into table quoted_identifier_2_1 select name,age,gpa from studenttab10k where gpa=3.5; + +drop view if exists quoted_identifier_2_2; + +create view quoted_identifier_2_2 as select `!@#$%^&*()_age`,count(*) from quoted_identifier_2_1 group by `!@#$%^&*()_age`; + +-- Two partition columns: one plain (age), one with special characters (gpa_!@#$%^&*()) +drop table if exists quoted_identifier_2_3; + +create table quoted_identifier_2_3(`name_col` string) partitioned by(age int, `gpa_!@#$%^&*()` double) stored by iceberg; + +insert into table quoted_identifier_2_3 select name,age,gpa from studenttab10k where gpa=4.5; + +select count(*) from quoted_identifier_2_3; diff --git a/iceberg/iceberg-handler/src/test/results/positive/iceberg_partition_special_chars.q.out b/iceberg/iceberg-handler/src/test/results/positive/iceberg_partition_special_chars.q.out new file mode 100644 index 000000000000..c8f2da301e40 --- /dev/null +++ b/iceberg/iceberg-handler/src/test/results/positive/iceberg_partition_special_chars.q.out @@ -0,0 +1,97 @@ +PREHOOK: query: CREATE TABLE studenttab10k( + name string, + age int, + gpa double +) +ROW FORMAT DELIMITED FIELDS TERMINATED BY '\t' +STORED AS TEXTFILE +PREHOOK: type: CREATETABLE +PREHOOK: Output: database:default +PREHOOK: Output: default@studenttab10k +POSTHOOK: query: CREATE TABLE studenttab10k( + name string, + age int, + gpa double +) +ROW FORMAT DELIMITED FIELDS TERMINATED BY '\t' +STORED AS TEXTFILE +POSTHOOK: type: CREATETABLE +POSTHOOK: Output: database:default +POSTHOOK: Output: default@studenttab10k +PREHOOK: query: LOAD DATA LOCAL INPATH '../../data/files/studenttab10k' OVERWRITE INTO TABLE studenttab10k +PREHOOK: type: LOAD +#### A masked pattern was here #### +PREHOOK: Output: default@studenttab10k +POSTHOOK: query: LOAD DATA LOCAL INPATH '../../data/files/studenttab10k' OVERWRITE INTO TABLE studenttab10k +POSTHOOK: type: LOAD +#### A masked pattern was here #### +POSTHOOK: Output: default@studenttab10k +PREHOOK: query: drop table if exists quoted_identifier_2_1 +PREHOOK: type: DROPTABLE +PREHOOK: Output: database:default +POSTHOOK: query: drop table if exists quoted_identifier_2_1 +POSTHOOK: type: DROPTABLE +POSTHOOK: Output: database:default +PREHOOK: query: create table quoted_identifier_2_1(`-+={[<_name_>]}?/|` string, `!@#$%^&*()_age` int) partitioned by(`gpa_!@#$%^&*()` double) stored by iceberg +PREHOOK: type: CREATETABLE +PREHOOK: Output: database:default +PREHOOK: Output: default@quoted_identifier_2_1 +POSTHOOK: query: create table quoted_identifier_2_1(`-+={[<_name_>]}?/|` string, `!@#$%^&*()_age` int) partitioned by(`gpa_!@#$%^&*()` double) stored by iceberg +POSTHOOK: type: CREATETABLE +POSTHOOK: Output: database:default +POSTHOOK: Output: default@quoted_identifier_2_1 +PREHOOK: query: insert into table quoted_identifier_2_1 select name,age,gpa from studenttab10k where gpa=3.5 +PREHOOK: type: QUERY +PREHOOK: Input: default@studenttab10k +PREHOOK: Output: default@quoted_identifier_2_1 +POSTHOOK: query: insert into table quoted_identifier_2_1 select name,age,gpa from studenttab10k where gpa=3.5 +POSTHOOK: type: QUERY +POSTHOOK: Input: default@studenttab10k +POSTHOOK: Output: default@quoted_identifier_2_1 +PREHOOK: query: drop view if exists quoted_identifier_2_2 +PREHOOK: type: DROPVIEW +POSTHOOK: query: drop view if exists quoted_identifier_2_2 +POSTHOOK: type: DROPVIEW +PREHOOK: query: create view quoted_identifier_2_2 as select `!@#$%^&*()_age`,count(*) from quoted_identifier_2_1 group by `!@#$%^&*()_age` +PREHOOK: type: CREATEVIEW +PREHOOK: Input: default@quoted_identifier_2_1 +PREHOOK: Output: database:default +PREHOOK: Output: default@quoted_identifier_2_2 +POSTHOOK: query: create view quoted_identifier_2_2 as select `!@#$%^&*()_age`,count(*) from quoted_identifier_2_1 group by `!@#$%^&*()_age` +POSTHOOK: type: CREATEVIEW +POSTHOOK: Input: default@quoted_identifier_2_1 +POSTHOOK: Output: database:default +POSTHOOK: Output: default@quoted_identifier_2_2 +POSTHOOK: Lineage: quoted_identifier_2_2.!@#$%^&*()_age SIMPLE [(quoted_identifier_2_1)quoted_identifier_2_1.FieldSchema(name:!@#$%^&*()_age, type:int, comment:null), ] +POSTHOOK: Lineage: quoted_identifier_2_2._c1 EXPRESSION [(quoted_identifier_2_1)quoted_identifier_2_1.null, ] +PREHOOK: query: drop table if exists quoted_identifier_2_3 +PREHOOK: type: DROPTABLE +PREHOOK: Output: database:default +POSTHOOK: query: drop table if exists quoted_identifier_2_3 +POSTHOOK: type: DROPTABLE +POSTHOOK: Output: database:default +PREHOOK: query: create table quoted_identifier_2_3(`name_col` string) partitioned by(age int, `gpa_!@#$%^&*()` double) stored by iceberg +PREHOOK: type: CREATETABLE +PREHOOK: Output: database:default +PREHOOK: Output: default@quoted_identifier_2_3 +POSTHOOK: query: create table quoted_identifier_2_3(`name_col` string) partitioned by(age int, `gpa_!@#$%^&*()` double) stored by iceberg +POSTHOOK: type: CREATETABLE +POSTHOOK: Output: database:default +POSTHOOK: Output: default@quoted_identifier_2_3 +PREHOOK: query: insert into table quoted_identifier_2_3 select name,age,gpa from studenttab10k where gpa=4.5 +PREHOOK: type: QUERY +PREHOOK: Input: default@studenttab10k +PREHOOK: Output: default@quoted_identifier_2_3 +POSTHOOK: query: insert into table quoted_identifier_2_3 select name,age,gpa from studenttab10k where gpa=4.5 +POSTHOOK: type: QUERY +POSTHOOK: Input: default@studenttab10k +POSTHOOK: Output: default@quoted_identifier_2_3 +PREHOOK: query: select count(*) from quoted_identifier_2_3 +PREHOOK: type: QUERY +PREHOOK: Input: default@quoted_identifier_2_3 +PREHOOK: Output: hdfs://### HDFS PATH ### +POSTHOOK: query: select count(*) from quoted_identifier_2_3 +POSTHOOK: type: QUERY +POSTHOOK: Input: default@quoted_identifier_2_3 +POSTHOOK: Output: hdfs://### HDFS PATH ### +0 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 index b25d35868df7..54f310864037 100644 --- a/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java +++ b/serde/src/java/org/apache/hadoop/hive/serde2/typeinfo/TypeInfoUtils.java @@ -296,6 +296,14 @@ private static boolean isTypeChar(char c) { 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 @@ private static ArrayList tokenize(String typeInfoString) { 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++; @@ -345,6 +349,7 @@ private static ArrayList tokenize(String typeInfoString) { return tokens; } + public TypeInfoParser(String typeInfoString) { this.typeInfoString = typeInfoString; typeInfoTokens = tokenize(typeInfoString); @@ -445,6 +450,31 @@ private String[] parseParams() { 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_!@#$%^&*()" 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.isEmpty()) { + expect(">"); + return null; + } + return fieldName.toString(); + } + private TypeInfo parseType() { Token t = expect("type"); @@ -542,11 +572,11 @@ private TypeInfo parseType() { 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); diff --git a/serde/src/test/org/apache/hadoop/hive/serde2/typeinfo/TestTypeInfoUtils.java b/serde/src/test/org/apache/hadoop/hive/serde2/typeinfo/TestTypeInfoUtils.java index a29f5d1fe66b..be8494ebc94d 100644 --- a/serde/src/test/org/apache/hadoop/hive/serde2/typeinfo/TestTypeInfoUtils.java +++ b/serde/src/test/org/apache/hadoop/hive/serde2/typeinfo/TestTypeInfoUtils.java @@ -20,6 +20,7 @@ +import java.util.Arrays; import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo; import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoUtils; import static org.junit.Assert.assertEquals; @@ -52,7 +53,13 @@ public void testTypeInfoParser() { "decimal(10, 2)", "decimal(10, 2 )", "decimal( 10, 2 )", - "struct" + "struct", + // struct field names with special characters (Iceberg partition column names) + "struct", + "struct", + "struct", + "struct", + "struct" }; String[] invalidTypeStrings = { @@ -125,4 +132,43 @@ public void testDecimal() { assertEquals("Failed for " + testCase.typeString, testCase.expectedScale, decimalType.getScale()); } } + + @Test + public void testStructFieldNameWithSpecialChars() { + TypeInfo typeInfo = TypeInfoUtils.getTypeInfoFromTypeString("struct"); + StructTypeInfo structTypeInfo = (StructTypeInfo) typeInfo; + assertEquals(1, structTypeInfo.getAllStructFieldNames().size()); + assertEquals("gpa_!@#$%^&*()", structTypeInfo.getAllStructFieldNames().get(0)); + assertEquals(TypeInfoFactory.doubleTypeInfo, structTypeInfo.getAllStructFieldTypeInfos().get(0)); + } + + @Test + public void testStructFieldNameStartingWithSpecialChar() { + TypeInfo typeInfo = TypeInfoUtils.getTypeInfoFromTypeString("struct"); + StructTypeInfo structTypeInfo = (StructTypeInfo) typeInfo; + assertEquals(Arrays.asList("!@#$%^&*()_age"), structTypeInfo.getAllStructFieldNames()); + } + + @Test + public void testStructMultipleFieldsWithSpecialChars() { + TypeInfo typeInfo = TypeInfoUtils.getTypeInfoFromTypeString( + "struct"); + StructTypeInfo structTypeInfo = (StructTypeInfo) typeInfo; + assertEquals(Arrays.asList("name", "!@#$%^&*()_age", "gpa_!@#$%^&*()"), + structTypeInfo.getAllStructFieldNames()); + } + + @Test + public void testEmptyStruct() { + TypeInfo typeInfo = TypeInfoUtils.getTypeInfoFromTypeString("struct<>"); + StructTypeInfo structTypeInfo = (StructTypeInfo) typeInfo; + assertEquals(0, structTypeInfo.getAllStructFieldNames().size()); + } + + @Test + public void testStructTrailingComma() { + TypeInfo typeInfo = TypeInfoUtils.getTypeInfoFromTypeString("struct"); + StructTypeInfo structTypeInfo = (StructTypeInfo) typeInfo; + assertEquals(Arrays.asList("a"), structTypeInfo.getAllStructFieldNames()); + } }