lasdf1234 commented on code in PR #13532:
URL: https://github.com/apache/gravitino/pull/13532#discussion_r4228521522
##########
server/src/main/java/org/apache/gravitino/server/web/rest/StatisticOperations.java:
##########
@@ -494,6 +479,61 @@ private static String getFormattedToPartitionName(String
toPartitionName, boolea
}
}
+ private Response writeStatistics(
+ String metalake,
+ String type,
+ String fullName,
+ StatisticsUpdateRequest request,
+ boolean merge) {
Review Comment:
Nit: prefer an enum over `boolean merge` here (e.g. `WriteMode.REPLACE` /
`WriteMode.MERGE`). Call sites like `writeStatistics(..., true)` are a bit hard
to read. Public `updateStatistics` / `mergeStatistics` can stay as they are.
Please apply the same change in `StatisticManager.writeStatistics` as well
for consistency.
Optional: the shared error `"Update statistics is only supported for tables
now."` below is also used on the merge path—consider wording it by mode.
##########
core/src/main/java/org/apache/gravitino/stats/StatisticManager.java:
##########
@@ -424,4 +387,76 @@ public Statistic[] statistics() {
return statistics;
}
}
+
+ private void writeStatistics(
+ String metalake,
+ MetadataObject metadataObject,
+ Map<String, StatisticValue<?>> statistics,
+ boolean merge) {
Review Comment:
Same nit as in `StatisticOperations`: replace `boolean merge` with an enum
(e.g. `WriteMode.REPLACE` / `MERGE`) so call sites don't rely on `true`/`false`.
--
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]
