JingsongLi commented on code in PR #9710:
URL: https://github.com/apache/paimon/pull/9710#discussion_r3976430718
##########
paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalog.java:
##########
@@ -873,10 +873,12 @@ private List<Map<String, String>>
canonicalizePartitionOptions(
String location = copied.get(PATH.key());
if (location != null) {
try {
+ // A partition location may be the table's own directory,
which is how a
+ // request returns a partition there, so what a partition
may own is judged
+ // where the table is known rather than here.
copied.put(
PATH.key(),
-
FormatTablePartitionPathResolver.canonicalizeCustomLocation(
- location, context)
+
FormatTablePartitionPathResolver.canonicalizeLocation(location, context)
Review Comment:
[P1] Preserve escaped partition directory names when normalizing default
paths
`FormatTableCommit` now sends generated default partition paths through this
code, but `canonicalizeLocation()` percent-decodes the Hive-escaped directory
name. For a valid partition value `dt=a%b`, the generated directory is
`dt=a%25b`; decoding leaves `%`, which causes the client to reject the path.
Values such as `a/b` and `a=b` are also decoded into different directory names,
so they no longer match the server's generated default path.
I reproduced an overwrite where the old file had already been deleted and
the replacement published before this failure, leaving the operation failed and
the old statistics unchanged. The same cases succeed with the baseline
committer.
Please preserve the actual escaped directory identity throughout request
normalization and default-path comparison, and add overwrite/truncate coverage
for these partition values.
##########
paimon-core/src/main/java/org/apache/paimon/table/format/FormatTablePartitionPathResolver.java:
##########
@@ -96,6 +92,39 @@ Path resolve(LinkedHashMap<String, String> spec, @Nullable
String customLocation
}
}
+ /** Where a partition lives when it carries no location of its own. */
+ public static Path defaultPartitionPath(
+ Path tablePath, LinkedHashMap<String, String> spec, boolean
onlyValueInPath) {
+ return new Path(
+ tablePath, PartitionPathUtils.generatePartitionPathUtil(spec,
onlyValueInPath));
+ }
+
+ /**
+ * Whether a requested location names the partition's own default
directory. That is how a
+ * request asks for a partition to go back to it, so both sides are
compared after
+ * canonicalization and do not have to spell the same directory the same
way. A location that
+ * does not parse is not the default one; the caller rejects it on its own
terms.
+ */
+ public static boolean isDefaultPartitionPath(
+ Path tablePath,
+ LinkedHashMap<String, String> spec,
+ boolean onlyValueInPath,
+ String requestedLocation,
+ @Nullable CatalogContext catalogContext) {
+ Path requested;
+ try {
+ PartitionPathUtils.validatePartitionSpecForPath(spec,
onlyValueInPath);
+ requested = canonicalizeCustomLocation(requestedLocation,
catalogContext);
Review Comment:
[P2] Use table-location rules when recognizing a default partition path
This still calls `canonicalizeCustomLocation()`, which rejects locations
without an authority for schemes other than `file` and `hdfs`. A supported
table location such as `viewfs:/warehouse/table` therefore fails the basic
round trip: generate its default directory for `dt=2026`, pass that exact path
to `isDefaultPartitionPath()`, and the result is `false`.
The reference REST server's request preprocessing applies the same
custom-location restriction. With a real ViewFS default mount and that server
validation, I reproduced truncate deleting the old file before the reset
request was rejected; the baseline succeeds.
Please use table-location canonicalization for default-path detection and
request preprocessing, applying the stricter custom-location rules only after
determining that a path is not the partition's own default directory.
--
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]
