diqiu50 commented on code in PR #14561:
URL: https://github.com/apache/gravitino/pull/14561#discussion_r4465694417
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueTableOperations.java:
##########
@@ -235,20 +271,22 @@ public boolean dropPartition(String partitionName) {
/**
* Builds a Hive-style partition name (e.g. {@code
dt=2024-01-01/country=us}) from an ordered list
- * of Glue partition values.
+ * of Glue partition values. Values are escaped as Hive does, so the name
parses back to them.
*/
private String buildPartitionName(List<String> values) {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < partitionColNames.length && i < values.size(); i++) {
if (i > 0) sb.append('/');
- sb.append(partitionColNames[i]).append('=').append(values.get(i));
+ sb.append(escapePathName(partitionColNames[i]))
Review Comment:
The key is lowercased before escaping now (f6685bc5d), and the comment at
the add path is corrected.
I kept the default partition mapping out. As far as I know, Hive and Spark
store such a partition with the literal value `__HIVE_DEFAULT_PARTITION__`, and
that name already round-trips. Mapping it back to an empty value on read would
make get and drop look up `""` and miss those partitions. I have not checked
this against partitions written by those engines on Glue, so please tell me if
you see otherwise. A default partition can still be added by passing the
literal value.
--
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]
