Copilot commented on code in PR #3630:
URL: https://github.com/apache/fluss/pull/3630#discussion_r3556242744
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorService.java:
##########
@@ -723,16 +725,34 @@ public CompletableFuture<CreatePartitionResponse>
createPartition(
// first, validate the partition spec, and get resolved partition spec.
PartitionSpec partitionSpec =
getPartitionSpec(request.getPartitionSpec());
- validatePartitionSpec(tablePath, table.partitionKeys, partitionSpec,
true);
+ ResolvedPartitionSpec partitionToCreate;
+ if (isHistoricalPartitionCreate(table, partitionSpec)) {
+ // Historical system partitions are lookup metadata, so creating
one requires the same
+ // permission as reading the table instead of writing table data.
+ authorizeTable(OperationType.READ, tablePath);
+ if (!request.isIgnoreIfNotExists()) {
+ throw new InvalidPartitionException(
+ "Creating historical system partition requires
ignoreIfExists=true.");
+ }
Review Comment:
Error message references `ignoreIfExists=true`, but the request flag checked
here is `ignoreIfNotExists`. This can mislead users when they hit this
validation path for historical system partitions.
##########
fluss-client/src/main/java/org/apache/fluss/client/lookup/PrimaryKeyLookuper.java:
##########
@@ -115,15 +130,57 @@ public CompletableFuture<LookupResult> lookup(InternalRow
lookupKey) {
tableInfo.getTablePath(),
metadataUpdater);
} catch (PartitionNotExistException e) {
+ if (isHistoricalLookupCandidatePartition(
+ tableInfo, originalPartitionName, Instant.now())) {
+ if (insertIfNotExists) {
+ return completedExceptionally(
+ new UnsupportedOperationException(
+ "Lookup with insertIfNotExists is not
supported for historical partition lookup."));
+ }
Review Comment:
This branch returns a failed future with `UnsupportedOperationException`,
which is not a Fluss `ApiException` and can lead to inconsistent error handling
for callers (compared to the server-side `InvalidTableException` used for the
same constraint). Prefer returning an `InvalidTableException` so the error is
classified as a user-facing, non-retriable request validation failure.
--
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]
