JingsongLi commented on PR #9295:
URL: https://github.com/apache/paimon/pull/9295#issuecomment-5341087965
I suggest simplifying this API by moving the merge semantics into each
partition statistics report, instead of having a request-wide `statisticsMode`.
A compatible public API could look like this:
```java
// Keep the existing overloads unchanged.
default void createPartitions(
Identifier identifier,
List<Map<String, String>> partitions,
boolean ignoreIfExists,
@Nullable List<PartitionStatisticsReport> statistics)
throws TableNotExistException {
// Preserve old Catalog implementations which override only the
// existing three-argument method.
createPartitions(identifier, partitions, ignoreIfExists);
}
```
The delegation direction is important: the new overload should call the old
three-argument overload. `RESTApi`, `RESTCatalog`, `DelegateCatalog`, and
`CachingCatalog` should also retain their existing overloads.
Each report would carry its own interpretation:
```java
class PartitionStatisticsReport {
Map<String, String> spec;
Long recordCount;
Long fileSizeInBytes;
Long fileCount;
Long lastFileCreationTime;
boolean replaceStatistics;
}
```
Semantics:
* No report for a partition: leave its statistics unchanged.
* `replaceStatistics == false`: the values describe only files added by this
write. Add counts/sizes and take the max creation time.
* `replaceStatistics == true`: the values describe the complete resulting
partition. Replace all reported statistics.
* For an incremental report, if either the stored aggregate or this write's
contribution is unknown, the result must remain unknown. For example, `UNKNOWN
+ 3 = UNKNOWN`.
* For a replacement report, an unknown field must replace the old value with
unknown; it must not preserve a value describing overwritten data.
* `totalBuckets` should not be part of an incremental report; it can remain
separate or only be accepted for a replacement report.
This also allows one batch to contain different report types per partition.
Retry safety follows directly: a request with no reports, or only
`replaceStatistics == true` reports, is retry-safe; a request containing any
incremental report is not retry-safe. I would not add an operation/idempotency
ID in this PR—an ambiguous incremental failure should instead be repaired later
by a complete replacement report.
For REST compatibility, keep `partitionSpecs` and `ignoreIfExists` unchanged
and add `partitionStatistics` as an optional field. Old clients continue to
work with a new server, and an old server can ignore the new field while still
registering partitions.
--
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]
