diqiu50 commented on code in PR #14318:
URL: https://github.com/apache/gravitino/pull/14318#discussion_r4409646302
##########
common/src/main/java/org/apache/gravitino/dto/requests/FunctionDataTypeValidator.java:
##########
@@ -40,12 +38,15 @@ static void validateDefinition(FunctionDefinitionDTO
definition, String fieldPat
String parameterPath = fieldPath + ".parameters[" + i + "]";
Preconditions.checkArgument(
parameters[i] != null, "\"%s\" field is required and cannot be
null", parameterPath);
- validateDataType(parameters[i].getDataType(), parameterPath +
".dataType");
+ WritableDataTypeValidator.validate(
Review Comment:
This now largely duplicates `WritableDataTypeValidator.validateDefinition`
(params dataType + defaultValue, returnType, returnColumns), and the two have
already diverged on null checks. Once the shared one is null-safe, could this
delegate to it directly?
##########
core/src/main/java/org/apache/gravitino/catalog/ViewOperationDispatcher.java:
##########
@@ -220,6 +224,12 @@ public View createView(
@Override
public View alterView(NameIdentifier ident, ViewChange... changes)
throws NoSuchViewException, IllegalArgumentException {
+ for (int i = 0; i < changes.length; i++) {
+ if (changes[i] instanceof ViewChange.ReplaceView) {
+ WritableDataTypeValidator.validateColumns(
Review Comment:
`ReplaceView` requires the full column list (`columns must not be null`), so
a view whose stored columns are already `UnparsedType` can no longer be
replaced: the client must resend those columns, and the new check rejects them.
The PR description says existing metadata "remains manageable". Is this case
intended? If not, we could skip validation for columns that are unchanged from
the current view, or document that replacing such a view requires supplying
resolved types.
##########
common/src/main/java/org/apache/gravitino/utils/WritableDataTypeValidator.java:
##########
@@ -0,0 +1,294 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.utils;
+
+import com.google.common.base.Preconditions;
+import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionParam;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.TableChange;
+import org.apache.gravitino.rel.expressions.Expression;
+import org.apache.gravitino.rel.expressions.FunctionExpression;
+import org.apache.gravitino.rel.expressions.distributions.Distribution;
+import org.apache.gravitino.rel.expressions.literals.Literal;
+import org.apache.gravitino.rel.expressions.sorts.SortOrder;
+import org.apache.gravitino.rel.expressions.transforms.Transform;
+import org.apache.gravitino.rel.partitions.IdentityPartition;
+import org.apache.gravitino.rel.partitions.ListPartition;
+import org.apache.gravitino.rel.partitions.Partition;
+import org.apache.gravitino.rel.partitions.RangePartition;
+import org.apache.gravitino.rel.types.Type;
+import org.apache.gravitino.rel.types.Types;
+
+/**
+ * Validates data types in newly written metadata. Read and import paths, and
selectors identifying
+ * existing metadata, must retain support for {@link Types.UnparsedType}.
+ *
+ * <p>This checks the type representation, not whether a connector supports
the type or whether a
+ * literal value matches its declared type.
+ */
+public final class WritableDataTypeValidator {
+
+ private WritableDataTypeValidator() {}
+
+ /**
+ * Validates a type, including all nested types.
+ *
+ * @param dataType The type to validate.
+ * @param fieldPath The request field path used in validation errors.
+ * @throws IllegalArgumentException If the type is unresolved or an external
type is blank.
+ */
+ public static void validate(Type dataType, String fieldPath) {
+ Preconditions.checkArgument(
+ dataType != null, "\"%s\" field is required and cannot be null",
fieldPath);
+ Preconditions.checkArgument(
+ !(dataType instanceof Types.UnparsedType),
+ "\"%s\" must be a Gravitino-recognized data type or an explicit
ExternalType; "
+ + "UnparsedType is not allowed for new metadata",
+ fieldPath);
+
+ if (dataType instanceof Types.ExternalType) {
+ Types.ExternalType externalType = (Types.ExternalType) dataType;
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(externalType.catalogString()),
+ "\"%s.catalogString\" field is required and cannot be blank",
+ fieldPath);
+ return;
+ }
+
+ if (dataType instanceof Types.StructType) {
+ Types.StructType.Field[] fields = ((Types.StructType) dataType).fields();
+ Preconditions.checkArgument(
+ fields != null, "\"%s.fields\" field is required and cannot be
null", fieldPath);
+ for (int i = 0; i < fields.length; i++) {
+ Preconditions.checkArgument(
+ fields[i] != null,
+ "\"%s.fields[%s]\" field is required and cannot be null",
+ fieldPath,
+ i);
+ validate(fields[i].type(), fieldPath + ".fields[" + i + "].type");
+ }
+ return;
+ }
+
+ if (dataType instanceof Types.ListType) {
+ validate(((Types.ListType) dataType).elementType(), fieldPath +
".elementType");
+ return;
+ }
+
+ if (dataType instanceof Types.MapType) {
+ Types.MapType mapType = (Types.MapType) dataType;
+ validate(mapType.keyType(), fieldPath + ".keyType");
+ validate(mapType.valueType(), fieldPath + ".valueType");
+ return;
+ }
+
+ if (dataType instanceof Types.UnionType) {
+ Type[] types = ((Types.UnionType) dataType).types();
+ Preconditions.checkArgument(
+ types != null, "\"%s.types\" field is required and cannot be null",
fieldPath);
+ for (int i = 0; i < types.length; i++) {
+ validate(types[i], fieldPath + ".types[" + i + "]");
+ }
+ return;
+ }
+
+ Preconditions.checkArgument(
+ dataType instanceof Type.PrimitiveType || dataType instanceof
Types.NullType,
+ "\"%s\" must be a Gravitino-recognized data type or an explicit
ExternalType",
+ fieldPath);
+ }
+
+ /**
+ * Validates literal types recursively within an expression and any
partition assignments.
+ *
+ * @param expression The expression to validate, or null when absent.
+ * @param fieldPath The request field path used in validation errors.
+ * @throws IllegalArgumentException If a literal contains a non-writable
type, a traversal array
+ * is null, or a partition assignment is null or unsupported.
+ */
+ public static void validateExpression(@Nullable Expression expression,
String fieldPath) {
+ if (expression == null) {
+ return;
+ }
+ if (expression instanceof Literal) {
+ validate(((Literal<?>) expression).dataType(), fieldPath + ".dataType");
+ }
+ if (expression instanceof Transform) {
+ Partition[] assignments = ((Transform) expression).assignments();
+ Preconditions.checkArgument(
+ assignments != null,
+ "\"%s.assignments\" field is required and cannot be null",
+ fieldPath);
+ for (int i = 0; i < assignments.length; i++) {
+ validatePartition(assignments[i], fieldPath + ".assignments[" + i +
"]");
+ }
+ }
+ Expression[] children = expression.children();
+ String childField =
+ expression instanceof FunctionExpression || expression instanceof
Transform
+ ? ".funcArgs"
+ : ".children";
+ Preconditions.checkArgument(
+ children != null, "\"%s\" field is required and cannot be null",
fieldPath + childField);
+ for (int i = 0; i < children.length; i++) {
+ validateExpression(children[i], fieldPath + childField + "[" + i + "]");
+ }
+ }
+
+ /**
+ * Validates the literal types in a newly added partition.
+ *
+ * @param partition The partition to validate.
+ * @param fieldPath The request field path used in validation errors.
+ * @throws IllegalArgumentException If a literal contains a non-writable
type, a traversal array
+ * is null, or the partition is null or unsupported.
+ */
+ public static void validatePartition(Partition partition, String fieldPath) {
+ Preconditions.checkArgument(
+ partition != null, "\"%s\" field is required and cannot be null",
fieldPath);
+ if (partition instanceof IdentityPartition) {
+ Literal<?>[] values = ((IdentityPartition) partition).values();
+ Preconditions.checkArgument(
+ values != null, "\"%s.values\" field is required and cannot be
null", fieldPath);
+ for (int i = 0; i < values.length; i++) {
+ validateExpression(values[i], fieldPath + ".values[" + i + "]");
+ }
+ } else if (partition instanceof ListPartition) {
+ Literal<?>[][] lists = ((ListPartition) partition).lists();
+ Preconditions.checkArgument(
+ lists != null, "\"%s.lists\" field is required and cannot be null",
fieldPath);
+ for (int i = 0; i < lists.length; i++) {
+ Preconditions.checkArgument(
+ lists[i] != null,
+ "\"%s.lists[%s]\" field is required and cannot be null",
+ fieldPath,
+ i);
+ for (int j = 0; j < lists[i].length; j++) {
+ validateExpression(lists[i][j], fieldPath + ".lists[" + i + "][" + j
+ "]");
+ }
+ }
+ } else if (partition instanceof RangePartition) {
+ RangePartition range = (RangePartition) partition;
+ validateExpression(range.lower(), fieldPath + ".lower");
+ validateExpression(range.upper(), fieldPath + ".upper");
+ } else {
+ throw new IllegalArgumentException(
+ String.format(
+ "\"%s\" must be an IdentityPartition, ListPartition or
RangePartition", fieldPath));
+ }
+ }
+
+ /**
+ * Validates the types and defaults of newly supplied table or view columns.
+ *
+ * @param columns The new columns.
+ * @param fieldPath The request field path of the columns array.
+ * @throws IllegalArgumentException If a new type or literal type is not
writable.
+ */
+ public static void validateColumns(Column[] columns, String fieldPath) {
+ for (int i = 0; i < columns.length; i++) {
+ String columnPath = fieldPath + "[" + i + "]";
+ validate(columns[i].dataType(), columnPath + ".type");
+ validateExpression(columns[i].defaultValue(), columnPath +
".defaultValue");
+ }
+ }
+
+ /**
+ * Validates types and expressions in a table creation request.
+ *
+ * @param columns The new columns.
+ * @param partitioning The partitioning, or null when absent.
+ * @param distribution The distribution, or null when absent.
+ * @param sortOrders The sort orders, or null when absent.
+ * @throws IllegalArgumentException If a new type is not writable.
+ */
+ public static void validateTable(
+ Column[] columns,
+ @Nullable Transform[] partitioning,
+ @Nullable Distribution distribution,
+ @Nullable SortOrder[] sortOrders) {
+ validateColumns(columns, "columns");
+ if (partitioning != null) {
+ for (int i = 0; i < partitioning.length; i++) {
+ validateExpression(partitioning[i], "partitioning[" + i + "]");
+ }
+ }
+ if (distribution != null) {
+ Expression[] expressions = distribution.expressions();
+ for (int i = 0; i < expressions.length; i++) {
+ validateExpression(expressions[i], "distribution.funcArgs[" + i + "]");
+ }
+ }
+ if (sortOrders != null) {
+ for (int i = 0; i < sortOrders.length; i++) {
+ validateExpression(sortOrders[i].expression(), "sortOrders[" + i +
"].sortTerm");
+ }
+ }
+ }
+
+ /**
+ * Validates only the types or defaults introduced by a table change.
+ *
+ * @param change The table change.
+ * @param fieldPath The request field path used in validation errors.
+ * @throws IllegalArgumentException If a new type is not writable.
+ */
+ public static void validateTableChange(TableChange change, String fieldPath)
{
+ if (change instanceof TableChange.AddColumn) {
+ TableChange.AddColumn add = (TableChange.AddColumn) change;
+ validate(add.getDataType(), fieldPath + ".type");
+ validateExpression(add.getDefaultValue(), fieldPath + ".defaultValue");
+ } else if (change instanceof TableChange.UpdateColumnType) {
+ validate(((TableChange.UpdateColumnType) change).getNewDataType(),
fieldPath + ".newType");
+ } else if (change instanceof TableChange.UpdateColumnDefaultValue) {
+ validateExpression(
+ ((TableChange.UpdateColumnDefaultValue) change).getNewDefaultValue(),
+ fieldPath + ".newDefaultValue");
+ }
+ }
+
+ /**
+ * Validates a newly registered function definition, including parameter
defaults.
+ *
+ * @param definition The new definition.
+ * @param fieldPath The request field path used in validation errors.
+ * @throws IllegalArgumentException If a new type is not writable.
+ */
+ public static void validateDefinition(FunctionDefinition definition, String
fieldPath) {
+ Preconditions.checkArgument(
+ definition != null, "\"%s\" field is required and cannot be null",
fieldPath);
+ FunctionParam[] parameters = definition.parameters();
+ for (int i = 0; i < parameters.length; i++) {
Review Comment:
`validateDefinition` dereferences `parameters()`, `returnColumns()` and
their elements without null checks, while the DTO-side
`FunctionDataTypeValidator` does check them. A Java API caller of
`registerFunction` / `AddDefinition` with a null element gets an NPE (500)
instead of an `IllegalArgumentException` (400). Could we add the same `"%s"
field is required and cannot be null` checks here?
##########
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:
Nit: core dispatchers hard-code REST field paths (`partitions[0]`,
`updates[i]`, `definitions[i]`), so Java API callers see JSON field names in
error messages. It also repeats the check already done in
`AddPartitionsRequest.validate`. Fine to keep, but neutral paths such as
`partition` may read better at this layer.
##########
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:
`fieldPrefix` is only used for the columns path. The representations,
null-representation, null-column and duplicate-dialect errors ignore it, so
`updates[1]` with empty representations reports `"representations" field is
required` without the index.
##########
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:
Only AddColumn / UpdateColumnType / UpdateColumnDefaultValue get the
`updates[i].` prefix; other update types still report errors without an index.
Instead of the instanceof/cast chain, how about a `validate(String
fieldPrefix)` on `TableUpdateRequest` (defaulting to `validate()`) so that
every update type can include the index? The same applies to
`ViewUpdatesRequest`.
##########
common/src/main/java/org/apache/gravitino/utils/WritableDataTypeValidator.java:
##########
@@ -0,0 +1,294 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.utils;
+
+import com.google.common.base.Preconditions;
+import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionParam;
+import org.apache.gravitino.rel.Column;
+import org.apache.gravitino.rel.TableChange;
+import org.apache.gravitino.rel.expressions.Expression;
+import org.apache.gravitino.rel.expressions.FunctionExpression;
+import org.apache.gravitino.rel.expressions.distributions.Distribution;
+import org.apache.gravitino.rel.expressions.literals.Literal;
+import org.apache.gravitino.rel.expressions.sorts.SortOrder;
+import org.apache.gravitino.rel.expressions.transforms.Transform;
+import org.apache.gravitino.rel.partitions.IdentityPartition;
+import org.apache.gravitino.rel.partitions.ListPartition;
+import org.apache.gravitino.rel.partitions.Partition;
+import org.apache.gravitino.rel.partitions.RangePartition;
+import org.apache.gravitino.rel.types.Type;
+import org.apache.gravitino.rel.types.Types;
+
+/**
+ * Validates data types in newly written metadata. Read and import paths, and
selectors identifying
+ * existing metadata, must retain support for {@link Types.UnparsedType}.
+ *
+ * <p>This checks the type representation, not whether a connector supports
the type or whether a
+ * literal value matches its declared type.
+ */
+public final class WritableDataTypeValidator {
+
+ private WritableDataTypeValidator() {}
+
+ /**
+ * Validates a type, including all nested types.
+ *
+ * @param dataType The type to validate.
+ * @param fieldPath The request field path used in validation errors.
+ * @throws IllegalArgumentException If the type is unresolved or an external
type is blank.
+ */
+ public static void validate(Type dataType, String fieldPath) {
+ Preconditions.checkArgument(
+ dataType != null, "\"%s\" field is required and cannot be null",
fieldPath);
+ Preconditions.checkArgument(
+ !(dataType instanceof Types.UnparsedType),
+ "\"%s\" must be a Gravitino-recognized data type or an explicit
ExternalType; "
+ + "UnparsedType is not allowed for new metadata",
+ fieldPath);
+
+ if (dataType instanceof Types.ExternalType) {
+ Types.ExternalType externalType = (Types.ExternalType) dataType;
+ Preconditions.checkArgument(
+ StringUtils.isNotBlank(externalType.catalogString()),
+ "\"%s.catalogString\" field is required and cannot be blank",
+ fieldPath);
+ return;
+ }
+
+ if (dataType instanceof Types.StructType) {
+ Types.StructType.Field[] fields = ((Types.StructType) dataType).fields();
+ Preconditions.checkArgument(
+ fields != null, "\"%s.fields\" field is required and cannot be
null", fieldPath);
+ for (int i = 0; i < fields.length; i++) {
+ Preconditions.checkArgument(
+ fields[i] != null,
+ "\"%s.fields[%s]\" field is required and cannot be null",
+ fieldPath,
+ i);
+ validate(fields[i].type(), fieldPath + ".fields[" + i + "].type");
+ }
+ return;
+ }
+
+ if (dataType instanceof Types.ListType) {
+ validate(((Types.ListType) dataType).elementType(), fieldPath +
".elementType");
+ return;
+ }
+
+ if (dataType instanceof Types.MapType) {
+ Types.MapType mapType = (Types.MapType) dataType;
+ validate(mapType.keyType(), fieldPath + ".keyType");
+ validate(mapType.valueType(), fieldPath + ".valueType");
+ return;
+ }
+
+ if (dataType instanceof Types.UnionType) {
+ Type[] types = ((Types.UnionType) dataType).types();
+ Preconditions.checkArgument(
+ types != null, "\"%s.types\" field is required and cannot be null",
fieldPath);
+ for (int i = 0; i < types.length; i++) {
+ validate(types[i], fieldPath + ".types[" + i + "]");
+ }
+ return;
+ }
+
+ Preconditions.checkArgument(
+ dataType instanceof Type.PrimitiveType || dataType instanceof
Types.NullType,
+ "\"%s\" must be a Gravitino-recognized data type or an explicit
ExternalType",
+ fieldPath);
+ }
+
+ /**
+ * Validates literal types recursively within an expression and any
partition assignments.
+ *
+ * @param expression The expression to validate, or null when absent.
+ * @param fieldPath The request field path used in validation errors.
+ * @throws IllegalArgumentException If a literal contains a non-writable
type, a traversal array
+ * is null, or a partition assignment is null or unsupported.
+ */
+ public static void validateExpression(@Nullable Expression expression,
String fieldPath) {
+ if (expression == null) {
+ return;
+ }
+ if (expression instanceof Literal) {
+ validate(((Literal<?>) expression).dataType(), fieldPath + ".dataType");
+ }
+ if (expression instanceof Transform) {
+ Partition[] assignments = ((Transform) expression).assignments();
+ Preconditions.checkArgument(
+ assignments != null,
+ "\"%s.assignments\" field is required and cannot be null",
+ fieldPath);
+ for (int i = 0; i < assignments.length; i++) {
+ validatePartition(assignments[i], fieldPath + ".assignments[" + i +
"]");
+ }
+ }
+ Expression[] children = expression.children();
+ String childField =
+ expression instanceof FunctionExpression || expression instanceof
Transform
+ ? ".funcArgs"
+ : ".children";
+ Preconditions.checkArgument(
+ children != null, "\"%s\" field is required and cannot be null",
fieldPath + childField);
+ for (int i = 0; i < children.length; i++) {
+ validateExpression(children[i], fieldPath + childField + "[" + i + "]");
+ }
+ }
+
+ /**
+ * Validates the literal types in a newly added partition.
+ *
+ * @param partition The partition to validate.
+ * @param fieldPath The request field path used in validation errors.
+ * @throws IllegalArgumentException If a literal contains a non-writable
type, a traversal array
+ * is null, or the partition is null or unsupported.
+ */
+ public static void validatePartition(Partition partition, String fieldPath) {
+ Preconditions.checkArgument(
+ partition != null, "\"%s\" field is required and cannot be null",
fieldPath);
+ if (partition instanceof IdentityPartition) {
+ Literal<?>[] values = ((IdentityPartition) partition).values();
+ Preconditions.checkArgument(
+ values != null, "\"%s.values\" field is required and cannot be
null", fieldPath);
+ for (int i = 0; i < values.length; i++) {
+ validateExpression(values[i], fieldPath + ".values[" + i + "]");
+ }
+ } else if (partition instanceof ListPartition) {
+ Literal<?>[][] lists = ((ListPartition) partition).lists();
+ Preconditions.checkArgument(
+ lists != null, "\"%s.lists\" field is required and cannot be null",
fieldPath);
+ for (int i = 0; i < lists.length; i++) {
+ Preconditions.checkArgument(
+ lists[i] != null,
+ "\"%s.lists[%s]\" field is required and cannot be null",
+ fieldPath,
+ i);
+ for (int j = 0; j < lists[i].length; j++) {
+ validateExpression(lists[i][j], fieldPath + ".lists[" + i + "][" + j
+ "]");
+ }
+ }
+ } else if (partition instanceof RangePartition) {
+ RangePartition range = (RangePartition) partition;
+ validateExpression(range.lower(), fieldPath + ".lower");
+ validateExpression(range.upper(), fieldPath + ".upper");
+ } else {
+ throw new IllegalArgumentException(
+ String.format(
+ "\"%s\" must be an IdentityPartition, ListPartition or
RangePartition", fieldPath));
+ }
+ }
+
+ /**
+ * Validates the types and defaults of newly supplied table or view columns.
+ *
+ * @param columns The new columns.
+ * @param fieldPath The request field path of the columns array.
+ * @throws IllegalArgumentException If a new type or literal type is not
writable.
+ */
+ public static void validateColumns(Column[] columns, String fieldPath) {
+ for (int i = 0; i < columns.length; i++) {
+ String columnPath = fieldPath + "[" + i + "]";
+ validate(columns[i].dataType(), columnPath + ".type");
Review Comment:
Same here: `columns[i]` is not null-checked, and in `validateTable` neither
are `sortOrders[i]` (L244) and `distribution.expressions()` (L237). They are
reachable from `TableOperationDispatcher.createTable` /
`ViewOperationDispatcher` via the Java API. Since every other traversal in this
class reports a field-path error, it would be good to be consistent.
--
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]
