[api][core] Reset Format Table partition locations on overwrite - #9710
[api][core] Reset Format Table partition locations on overwrite#9710sundapeng wants to merge 3 commits into
Conversation
5aeae8b to
606f96d
Compare
A null path in partitionOptions resets an existing Format Table partition to its default location, together with replacement statistics for that partition; non-null paths and statistics-only requests keep their behavior.
606f96d to
aa5cadf
Compare
Resolve append, overwrite, and truncate targets through the narrowest registry API. Reset partition path metadata with replacement statistics while preserving external data.
|
Could we pass the concrete target partition path instead of using The writer already knows its output directory. The REST server could derive and normalize the default partition path from the table metadata and partition spec, then remove the stored path override when the requested path equals that default. Whether the server stores this as NULL should be an implementation detail. The API semantics would be:
For this PR, we could limit changes to existing locations to switching back to the default directory. Such a change should still require replacement statistics for the same partition and apply both atomically; an ordinary This would also require recognizing the default path before the custom-location overlap/conflict checks. Passing a non-null path alone does not provide compatibility with older servers, so the updated server contract would still need to be explicit. |
|
Makes sense — I'll switch to naming the default path. The preconditions you list are already what the PR enforces. |
Follow-up on review. partitionOptions goes back to a plain string map: a request returns a partition to its default location by naming that directory, and the catalog recognizes it, so nothing on the wire has to be null. The preconditions are unchanged. It counts only with replaceStatistics=true and a statistics entry for the same spec, both applied in the same request, and an ordinary registration still never relocates an existing partition. A path under the table location that is not the partition's own default directory stays invalid, so a server that has not implemented this rejects the request instead of storing it. FormatTablePartitionPathResolver gains the two things both sides need to agree on: where a partition lives by default, and whether a requested location is that directory after canonicalization. It also separates canonicalizing a location from judging what a partition may own, because a partition location may now be the table's own directory, whose scheme can address storage without an authority; the callers that know the table keep the stricter rule.
7cfd26e to
55381ee
Compare
JingsongLi
left a comment
There was a problem hiding this comment.
Two path-normalization issues found while reviewing the explicit default-path contract.
| PATH.key(), | ||
| FormatTablePartitionPathResolver.canonicalizeCustomLocation( | ||
| location, context) | ||
| FormatTablePartitionPathResolver.canonicalizeLocation(location, context) |
There was a problem hiding this 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.
| Path requested; | ||
| try { | ||
| PartitionPathUtils.validatePartitionSpecForPath(spec, onlyValueInPath); | ||
| requested = canonicalizeCustomLocation(requestedLocation, catalogContext); |
There was a problem hiding this 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.
Purpose
Follow-up to #9540. Every Format Table write commit currently reads the whole partition registry to find out whether it touches a partition registered at a custom location, and it rejects overwrite and truncate of such partitions. On a table with hundreds of thousands of partitions that is hundreds of REST calls per commit, for every writer.
This PR changes both sides:
partitionOptions[i].path = nullresets a partition to its default location. It is accepted only withreplaceStatistics=trueand apartitionStatisticsentry for the same spec, both applied in the same request. Omittingpathkeeps the stored location; non-null paths, other options and statistics-only requests behave as before. Additive statistics for a partition that already has a custom location are rejected. Servers without the reset reject the request, since a null option value already fails their validation.FormatTableCommit: appends look up only the partitions they wrote, static overwrites list only their prefix, and overwrite and truncate reset the target partitions to the default location together with replacement statistics instead of failing. External data is left untouched. Whole-table overwrite and truncate still list the table, which they need for directory cleanup.Tests
RESTApiJsonTest,MockRESTCatalogTest,CatalogFormatTablePartitionManagerTestFormatTableCommitTest,FormatTableCommitStatisticsTest,FormatTableCommitRegistryValidationTestnode scripts/validate-rest-openapi.js