This is an automated email from the ASF dual-hosted git repository.

danny0405 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git


The following commit(s) were added to refs/heads/master by this push:
     new 6aeb9a27ae98 fix: escape partition values and locations in Hive sync 
DDL (#20390)
6aeb9a27ae98 is described below

commit 6aeb9a27ae989f3f8b955447e2db8bd3acaabc1a
Author: Danny Chan <[email protected]>
AuthorDate: Tue Oct 20 09:55:45 2026 +0800

    fix: escape partition values and locations in Hive sync DDL (#20390)
    
    * fix: escape partition values and locations in Hive sync DDL
    * fix: escape Hive CREATE TABLE base locations
---
 .../hudi/hive/ddl/QueryBasedDDLExecutor.java       |   6 +-
 .../org/apache/hudi/hive/util/HiveSchemaUtil.java  |   2 +-
 .../ddl/TestQueryBasedDDLExecutorEscaping.java     | 163 +++++++++++++++++++++
 3 files changed, 167 insertions(+), 4 deletions(-)

diff --git 
a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/QueryBasedDDLExecutor.java
 
b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/QueryBasedDDLExecutor.java
index 3bcbbe1841df..76cb3b5376a2 100644
--- 
a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/QueryBasedDDLExecutor.java
+++ 
b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/QueryBasedDDLExecutor.java
@@ -196,7 +196,7 @@ public abstract class QueryBasedDDLExecutor implements 
DDLExecutor {
       String partitionClause = getPartitionClause(partitions.get(i));
       String fullPartitionPath =
           FSUtils.constructAbsolutePath(config.getString(META_SYNC_BASE_PATH), 
partitions.get(i)).toString();
-      alterSQL.append("  PARTITION (").append(partitionClause).append(") 
LOCATION '").append(fullPartitionPath)
+      alterSQL.append("  PARTITION (").append(partitionClause).append(") 
LOCATION '").append(HiveSchemaUtil.escapeSqlString(fullPartitionPath))
           .append("' ");
       if ((i + 1) % batchSyncPartitionNum == 0) {
         result.add(alterSQL.toString());
@@ -231,7 +231,7 @@ public abstract class QueryBasedDDLExecutor implements 
DDLExecutor {
         // This is a decode operator for encode in 
KeyGenUtils#getRecordPartitionPath
         partitionValue = 
PartitionPathEncodeUtils.unescapePathName(partitionValue);
       }
-      partBuilder.add("`" + 
config.getSplitStrings(META_SYNC_PARTITION_FIELDS).get(i) + "`='" + 
partitionValue + "'");
+      partBuilder.add("`" + 
config.getSplitStrings(META_SYNC_PARTITION_FIELDS).get(i) + "`='" + 
HiveSchemaUtil.escapeSqlString(partitionValue) + "'");
     }
     return String.join(",", partBuilder);
   }
@@ -289,7 +289,7 @@ public abstract class QueryBasedDDLExecutor implements 
DDLExecutor {
           String partitionScheme = partitionPath.toUri().getScheme();
           String fullPartitionPath = 
StorageSchemes.HDFS.getScheme().equals(partitionScheme)
               ? 
HadoopFSUtils.getDFSFullPartitionPath(config.getHadoopFileSystem(), 
partitionPath) : partitionPath.toString();
-          result.add(alterTablePrefix + " PARTITION (" + partitionClause + ") 
SET LOCATION '" + fullPartitionPath + "'");
+          result.add(alterTablePrefix + " PARTITION (" + partitionClause + ") 
SET LOCATION '" + HiveSchemaUtil.escapeSqlString(fullPartitionPath) + "'");
         }
         break;
       default:
diff --git 
a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java
 
b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java
index bbc9ef94546f..23516c319a54 100644
--- 
a/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java
+++ 
b/hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/util/HiveSchemaUtil.java
@@ -430,7 +430,7 @@ public class HiveSchemaUtil {
       sb.append(" WITH SERDEPROPERTIES 
(").append(propertyToString(serdeProperties)).append(")");
     }
     sb.append(" STORED AS INPUTFORMAT '").append(inputFormatClass).append("'");
-    sb.append(" OUTPUTFORMAT '").append(outputFormatClass).append("' LOCATION 
'").append(config.getAbsoluteBasePath()).append("'");
+    sb.append(" OUTPUTFORMAT '").append(outputFormatClass).append("' LOCATION 
'").append(escapeSqlString(config.getAbsoluteBasePath())).append("'");
 
     if (tableProperties != null && !tableProperties.isEmpty()) {
       sb.append(" 
TBLPROPERTIES(").append(propertyToString(tableProperties)).append(")");
diff --git 
a/hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/ddl/TestQueryBasedDDLExecutorEscaping.java
 
b/hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/ddl/TestQueryBasedDDLExecutorEscaping.java
new file mode 100644
index 000000000000..85aa6b9e730d
--- /dev/null
+++ 
b/hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/ddl/TestQueryBasedDDLExecutorEscaping.java
@@ -0,0 +1,163 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.hive.ddl;
+
+import org.apache.hudi.common.config.TypedProperties;
+import org.apache.hudi.common.schema.HoodieSchema;
+import org.apache.hudi.common.schema.HoodieSchemaField;
+import org.apache.hudi.common.schema.HoodieSchemaType;
+import org.apache.hudi.hive.HiveSyncConfig;
+import org.apache.hudi.hive.MultiPartKeysValueExtractor;
+
+import org.apache.hadoop.hive.ql.parse.ASTNode;
+import org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer;
+import org.apache.hadoop.hive.ql.parse.HiveParser;
+import org.apache.hadoop.hive.ql.parse.ParseDriver;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Stream;
+
+import static org.apache.hudi.sync.common.HoodieSyncConfig.META_SYNC_BASE_PATH;
+import static 
org.apache.hudi.sync.common.HoodieSyncConfig.META_SYNC_DATABASE_NAME;
+import static 
org.apache.hudi.sync.common.HoodieSyncConfig.META_SYNC_DECODE_PARTITION;
+import static 
org.apache.hudi.sync.common.HoodieSyncConfig.META_SYNC_PARTITION_EXTRACTOR_CLASS;
+import static 
org.apache.hudi.sync.common.HoodieSyncConfig.META_SYNC_PARTITION_FIELDS;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class TestQueryBasedDDLExecutorEscaping {
+
+  static Stream<Arguments> partitionValues() {
+    return Stream.of(
+        Arguments.of("plain", false, "plain"),
+        Arguments.of("O'Reilly", false, "O'Reilly"),
+        Arguments.of("back\\slash", false, "back\\slash"),
+        Arguments.of("trailing\\", false, "trailing\\"),
+        Arguments.of("back\\'quote", false, "back\\'quote"),
+        Arguments.of("O%27Reilly%5Ctail", true, "O'Reilly\\tail"),
+        Arguments.of("O%27Reilly%5Ctail", false, "O%27Reilly%5Ctail"),
+        Arguments.of("x') LOCATION 'other' --", false, "x') LOCATION 'other' 
--"));
+  }
+
+  @ParameterizedTest
+  @MethodSource("partitionValues")
+  void partitionDdlPreservesLiteralValues(String value, boolean decode, String 
expectedValue) throws Exception {
+    // Quotes and backslashes in the base path must also survive Hive's 
literal decoding.
+    String basePath = "/tmp/warehouse's\\data";
+    TypedProperties props = new TypedProperties();
+    props.setProperty(META_SYNC_DATABASE_NAME.key(), "db");
+    props.setProperty(META_SYNC_BASE_PATH.key(), basePath);
+    props.setProperty(META_SYNC_PARTITION_FIELDS.key(), "p");
+    props.setProperty(META_SYNC_PARTITION_EXTRACTOR_CLASS.key(), 
MultiPartKeysValueExtractor.class.getName());
+    props.setProperty(META_SYNC_DECODE_PARTITION.key(), 
Boolean.toString(decode));
+    RecordingExecutor executor = new RecordingExecutor(new 
HiveSyncConfig(props));
+    String partition = "p=" + value;
+    List<String> partitions = Collections.singletonList(partition);
+    // Decoding the partition value must not decode the physical directory 
location.
+    List<String> expectedWithLocation = Arrays.asList(expectedValue, basePath 
+ "/" + partition);
+
+    executor.addPartitionsToTable("tbl", partitions);
+    assertEquals(1, executor.executed.size());
+    assertEquals(expectedWithLocation, 
parseLiterals(executor.executed.get(0)));
+
+    executor.executed.clear();
+    executor.updatePartitionsToTable("tbl", partitions);
+    assertEquals(2, executor.executed.size());
+    // Hive's SET LOCATION AST visits the location before the partition 
specification.
+    assertEquals(Arrays.asList(basePath + "/" + partition, expectedValue), 
parseLiterals(executor.executed.get(1)));
+
+    executor.executed.clear();
+    executor.touchPartitionsToTable("tbl", partitions);
+    assertEquals(2, executor.executed.size());
+    assertEquals(Collections.singletonList(expectedValue), 
parseLiterals(executor.executed.get(1)));
+  }
+
+  @ParameterizedTest
+  @ValueSource(strings = {"/tmp/warehouse", "/tmp/warehouse's", 
"/tmp/warehouse\\data",
+      "/tmp/warehouse's\\data", "/tmp/warehouse\\'data", "/tmp/warehouse\\"})
+  void createTableDdlPreservesBasePath(String basePath) throws Exception {
+    TypedProperties props = new TypedProperties();
+    props.setProperty(META_SYNC_DATABASE_NAME.key(), "db");
+    props.setProperty(META_SYNC_BASE_PATH.key(), basePath);
+    props.setProperty(META_SYNC_PARTITION_FIELDS.key(), "p");
+    RecordingExecutor executor = new RecordingExecutor(new 
HiveSyncConfig(props));
+    HoodieSchema schema = HoodieSchema.createRecord("record", null, null,
+        Collections.singletonList(HoodieSchemaField.of("id", 
HoodieSchema.create(HoodieSchemaType.STRING))));
+    String serde = 
"org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe";
+    String inputFormat = 
"org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat";
+    String outputFormat = 
"org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat";
+
+    // Exercise generateCreateDDL through the same createTable path used by 
JDBC and HiveQL.
+    executor.createTable("tbl", schema, inputFormat, outputFormat, serde,
+        Collections.emptyMap(), Collections.emptyMap());
+
+    assertEquals(1, executor.executed.size());
+    assertEquals(Arrays.asList(serde, inputFormat, outputFormat, basePath), 
parseLiterals(executor.executed.get(0)));
+  }
+
+  private static List<String> parseLiterals(String sql) throws Exception {
+    List<String> literals = new ArrayList<>();
+    collectLiterals(new ParseDriver().parse(sql), literals);
+    return literals;
+  }
+
+  private static void collectLiterals(ASTNode node, List<String> literals) {
+    if (node.getType() == HiveParser.StringLiteral) {
+      literals.add(BaseSemanticAnalyzer.unescapeSQLString(node.getText()));
+    }
+    for (int i = 0; i < node.getChildCount(); i++) {
+      collectLiterals((ASTNode) node.getChild(i), literals);
+    }
+  }
+
+  private static class RecordingExecutor extends QueryBasedDDLExecutor {
+    private final List<String> executed = new ArrayList<>();
+
+    RecordingExecutor(HiveSyncConfig config) {
+      super(config);
+    }
+
+    @Override
+    public void runSQL(String sql) {
+      executed.add(sql);
+    }
+
+    @Override
+    public Map<String, String> getTableSchema(String tableName) {
+      return Collections.emptyMap();
+    }
+
+    @Override
+    public void dropPartitionsToTable(String tableName, List<String> 
partitionsToDrop) {
+      // Not used by these tests.
+    }
+
+    @Override
+    public void close() {
+      // No resources to close.
+    }
+  }
+}

Reply via email to