mchades commented on code in PR #14318:
URL: https://github.com/apache/gravitino/pull/14318#discussion_r4410114654
##########
common/src/main/java/org/apache/gravitino/dto/requests/TableUpdatesRequest.java:
##########
@@ -58,6 +58,19 @@ public TableUpdatesRequest() {
public void validate() throws IllegalArgumentException {
Preconditions.checkArgument(updates != null, "updates must not be null");
Preconditions.checkArgument(!updates.isEmpty(), "updates must not be
empty");
- updates.forEach(RESTRequest::validate);
+ for (int i = 0; i < updates.size(); i++) {
+ TableUpdateRequest update = updates.get(i);
+ String fieldPrefix = "updates[" + i + "].";
+ if (update instanceof TableUpdateRequest.AddTableColumnRequest) {
Review Comment:
Replaced the `instanceof` dispatch with `validateWithPrefix` on both update
interfaces. The default delegates to `validate()`, and only the existing
type/expression-aware requests override it. Other validators retain their
messages; the PR description now limits indexed paths to the new checks. The
distinct method name also preserves existing `request::validate` method
references.
##########
common/src/main/java/org/apache/gravitino/dto/requests/ViewUpdateRequest.java:
##########
@@ -222,6 +223,20 @@ public ReplaceViewRequest() {
@Override
public void validate() throws IllegalArgumentException {
+ validateWithPrefix("");
+ }
+
+ @Override
+ public ViewChange viewChange() {
+ return ViewChange.replaceView(
+ columns == null ? new ColumnDTO[0] : columns,
+ DTOConverters.fromDTOs(representations),
+ defaultCatalog,
+ defaultSchema,
+ comment);
+ }
+
+ void validateWithPrefix(String fieldPrefix) {
Review Comment:
Those checks and their messages predate this PR. The prefix applies to the
new writable-type and expression checks; existing validation messages remain
unchanged. The method documentation and PR description now state this scope.
##########
core/src/main/java/org/apache/gravitino/catalog/PartitionOperationDispatcher.java:
##########
@@ -68,6 +69,7 @@ public Partition getPartition(NameIdentifier tableIdent,
String partitionName)
@Override
public Partition addPartition(NameIdentifier tableIdent, Partition partition)
throws PartitionAlreadyExistsException {
+ WritableDataTypeValidator.validatePartition(partition, "partitions[0]");
Review Comment:
Changed the core path to `partition`, since this method accepts one
partition. REST still reports `partitions[0]`. Both checks remain to cover REST
and direct core calls using the same validator; other batch paths retain their
indices.
--
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]
