Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/docs/concepts/rest/rest-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ order of `partitionSpecs`; use `{}` when a partition has no options. Custom loca
`path` option. Before registering custom locations, ensure that the REST server supports partition
options and all readers support custom locations.

For an existing Format Table partition, omitting `path` keeps its location. Naming the partition's
own default directory under the table asks the server to put it back there: the stored location is
dropped, no data is deleted, and the request needs `replaceStatistics=true` with a
`partitionStatistics` entry for the same spec. Any other path under the table location stays
invalid, so a server that does not implement this rejects the request rather than storing it. A
server also rejects additive statistics for a partition that already has a custom location.

<body>
<iframe src="/docs/master/rest-catalog-open-api.yaml" width="100%" height="800px" />
</body>
7 changes: 5 additions & 2 deletions docs/scripts/validate-rest-openapi.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,11 @@ function validateCatalogOpenApi() {
requestOptions.description.includes('partitionSpecs') &&
requestOptions.description.toLowerCase().includes('position') &&
requestOptions.description.toLowerCase().includes('same length') &&
requestOptions.description.toLowerCase().includes('empty object'),
'CreatePartitionsRequest.partitionOptions must document positional alignment and empty options',
requestOptions.description.toLowerCase().includes('empty object') &&
requestOptions.description.includes('replaceStatistics=true') &&
requestOptions.description.includes('partitionStatistics') &&
requestOptions.description.toLowerCase().includes('default directory'),
'CreatePartitionsRequest.partitionOptions must document alignment and returns to the default directory',
);
contract.requireProperties('Partition', ['options']);
contract.requireProperties('ConfigResponse', ['defaults', 'overrides']);
Expand Down
4 changes: 2 additions & 2 deletions docs/static/rest-catalog-open-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2506,10 +2506,10 @@ components:
items:
$ref: '#/components/schemas/PartitionStatistics'
replaceStatistics:
description: Whether partitionStatistics replace the stored values rather than add to them; required whenever partitionStatistics is present, and absent otherwise. Replacing overwrites recordCount, fileSizeInBytes, fileCount and lastFileCreationTime; adding sums the three counts and keeps the later lastFileCreationTime, since two timestamps do not add. A field reported as unknown leaves the stored one alone either way, and totalBuckets is never combined. A client that reports only the files it just wrote adds; one that reports a whole partition, such as an overwrite or a directory rescan, replaces.
description: Whether partitionStatistics replace the stored values rather than add to them; required whenever partitionStatistics is present, and absent otherwise. Replacing overwrites recordCount, fileSizeInBytes, fileCount and lastFileCreationTime; adding sums the three counts and keeps the later lastFileCreationTime, since two timestamps do not add. A field reported as unknown leaves the stored one alone either way, and totalBuckets is never combined. A client that reports only the files it just wrote adds; one that reports a whole partition, such as an overwrite or a directory rescan, replaces. An additive report is rejected for a Format Table partition that already has a custom path; registering a new custom path together with its first statistics is allowed.
type: [ boolean, "null" ]
partitionOptions:
description: Optional options aligned with partitionSpecs by position. The list must have the same length as partitionSpecs; use an empty object when a partition has no options. A custom location is stored under the path key.
description: Optional options aligned with partitionSpecs by position. The list must have the same length as partitionSpecs; use an empty object when a partition has no options. A custom location is stored under the path key. Omitting path keeps the stored location; a path outside the table location registers a custom location for a new partition and must equal the stored path for an existing one; the partition's own default directory under the table location returns the partition there, dropping the stored path without deleting data. Returning a partition to its default directory requires replaceStatistics=true and a partitionStatistics entry with the same spec, applied in the same request. Any other path under the table location is invalid, so a provider that does not implement this rejects the request instead of storing it.
type: [ array, "null" ]
items:
type: object
Expand Down
8 changes: 7 additions & 1 deletion paimon-api/src/main/java/org/apache/paimon/rest/RESTApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,11 @@ public void markDonePartitions(Identifier identifier, List<Map<String, String>>
* as unknown leaves the stored one alone either way, and a report never creates or removes a
* partition row.
*
* <p>For an existing partition, omitting {@code path} keeps its location, and naming the
* partition's own default directory returns it there without deleting data, which needs
* replacement statistics for that partition. Additive statistics are rejected for a Format
* Table partition that already has a custom location.
*
* @param identifier database name and table name
* @param partitions partitions to be created
* @param ignoreIfExists if false, fail when any partition already exists and apply none of the
Expand All @@ -1027,7 +1032,8 @@ public void markDonePartitions(Identifier identifier, List<Map<String, String>>
* PartitionStatistics#spec()} rather than by position, or null to report none
* @param replaceStatistics whether the report replaces the stored values rather than adding to
* them; ignored when {@code statistics} is null, and not sent at all in that case
* @param partitionOptions options aligned with {@code partitions} by position, or null
* @param partitionOptions options aligned with {@code partitions} by position, or null; a
* {@code path} naming the partition's default directory returns it there
* @return the partitions the server created and the ones it already held
*/
public CreatePartitionsResponse createPartitions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@
*
* <p>Statistics ride along optionally, matched to {@code partitionSpecs} by {@link
* PartitionStatistics#spec()} rather than by position, so they may cover only some of them. Both
* statistics fields are absent unless the client reports.
* statistics fields are absent unless the client reports. Partition options align with {@code
* partitionSpecs} by position; naming the partition's own default directory in {@code path} asks
* the catalog to put it back there, which needs replacement statistics for that partition.
*/
@JsonIgnoreProperties(ignoreUnknown = true)
public class CreatePartitionsRequest implements RESTRequest {
Expand Down Expand Up @@ -148,7 +150,10 @@ public Boolean replaceStatistics() {
return replaceStatistics;
}

/** Options aligned with partition specs; a null list omits the field. */
/**
* Options aligned with partition specs; a {@code path} naming the partition's default directory
* returns it there. A null list omits the field.
*/
@JsonGetter(FIELD_PARTITION_OPTIONS)
@Nullable
public List<Map<String, String>> getPartitionOptions() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1106,6 +1106,11 @@ default void createPartitions(Identifier identifier, List<Map<String, String>> p
/**
* Create partitions atomically unless existing entries are ignored, with optional statistics
* and position-aligned options.
*
* <p>For an existing partition, omitting {@code path} keeps its location, and naming the
* partition's own default directory returns it there without deleting data, which needs
* replacement statistics for that partition. Additive statistics are rejected for a Format
* Table partition that already has a custom location. Each call is atomic on its own.
*/
default void createPartitions(
Identifier identifier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

.toString());
} catch (IllegalArgumentException e) {
throw invalidPartitionLocation(identifier, partition, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ public void createPartitions(
@Nullable List<PartitionStatistics> statistics,
boolean replaceStatistics,
@Nullable List<Map<String, String>> partitionOptions) {
validatePartitionOptions(partitionOptions, partitions);
Map<Map<String, String>, PartitionStatistics> statisticsBySpec =
validateAndIndexStatistics(statistics, partitions);
validatePartitionOptions(partitionOptions, partitions);
if (partitions.isEmpty()) {
return;
}
Expand Down
Loading
Loading