voonhous commented on code in PR #20390:
URL: https://github.com/apache/hudi/pull/20390#discussion_r4311425584
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/ddl/QueryBasedDDLExecutor.java:
##########
@@ -196,7 +196,7 @@ private List<String> constructAddPartitions(String
tableName, List<String> parti
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))
Review Comment:
**major:** The base path is still unescaped one step earlier:
`HiveSchemaUtil.generateCreateDDL` (line 433) appends `LOCATION '` +
`config.getAbsoluteBasePath()` raw, and `createTable` in this class runs it in
both JDBC and HiveQL modes. A base path containing `'`, like the test's
`/tmp/warehouse's\data`, fails at CREATE TABLE before these partition LOCATIONs
are reached. That literal has been raw since 388457b6b28c. Could we wrap it in
`HiveSchemaUtil.escapeSqlString` too and add a `generateCreateDDL` parse case
to this test?
--
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]
