leaves12138 commented on code in PR #9251:
URL: https://github.com/apache/paimon/pull/9251#discussion_r3792155252
##########
paimon-core/src/main/java/org/apache/paimon/schema/SchemaValidation.java:
##########
@@ -449,6 +455,106 @@ private static void validateOnlyContainPrimitiveType(
}
}
+ private static void validateGeospatialTypes(
+ TableSchema schema, CoreOptions options, RowType rowType) {
+ boolean hasGeospatial =
+ containsType(
+ rowType,
+ type -> type.isAnyOf(DataTypeRoot.GEOMETRY,
DataTypeRoot.GEOGRAPHY));
+ if (!hasGeospatial) {
+ return;
+ }
+
+ checkArgument(
+ CoreOptions.FILE_FORMAT_PARQUET.equals(options.formatType()),
+ "Geometry and geography columns require '%s'='parquet', but
was '%s'.",
+ CoreOptions.FILE_FORMAT.key(),
+ options.formatType());
+ options.fileFormatPerLevel()
+ .forEach(
+ (level, format) ->
+ checkArgument(
+
CoreOptions.FILE_FORMAT_PARQUET.equals(format),
+ "Geometry and geography columns
require parquet at every level, but '%s' contains '%s:%s'.",
+
CoreOptions.FILE_FORMAT_PER_LEVEL.key(),
+ level,
+ format));
+ checkArgument(
+ options.changelogFileFormat() == null
+ ||
CoreOptions.FILE_FORMAT_PARQUET.equals(options.changelogFileFormat()),
+ "Geometry and geography columns require '%s' to be parquet,
but was '%s'.",
+ CoreOptions.CHANGELOG_FILE_FORMAT.key(),
+ options.changelogFileFormat());
+ validateIcebergGeospatialTypes(rowType, options);
+
+ List<String> geospatialClusteringColumns =
+ schema.fields().stream()
+ .filter(field ->
options.clusteringColumns().contains(field.name()))
+ .filter(
+ field ->
+ containsType(
+ field.type(),
+ type ->
+ type.isAnyOf(
+
DataTypeRoot.GEOMETRY,
+
DataTypeRoot.GEOGRAPHY)))
+ .map(DataField::name)
+ .collect(Collectors.toList());
+ checkArgument(
+ geospatialClusteringColumns.isEmpty(),
+ "Geometry and geography columns cannot be clustering columns:
%s.",
+ geospatialClusteringColumns);
+
+ Set<String> geospatialFields =
+ schema.fields().stream()
+ .filter(
+ field ->
+ field.type()
+ .isAnyOf(
+ DataTypeRoot.GEOMETRY,
+
DataTypeRoot.GEOGRAPHY))
+ .map(DataField::name)
+ .collect(Collectors.toSet());
+ Set<String> geospatialBucketKeys = new HashSet<>(schema.bucketKeys());
+ geospatialBucketKeys.retainAll(geospatialFields);
+ checkArgument(
+ geospatialBucketKeys.isEmpty(),
+ "Geometry and geography columns cannot be bucket keys: %s.",
+ geospatialBucketKeys);
+ Set<String> geospatialSequenceFields = new
HashSet<>(options.sequenceField());
Review Comment:
This check only includes fields whose **top-level** type is `GEOMETRY` or
`GEOGRAPHY`. A nested geospatial type can therefore still be selected as a
sequence field. For example, `seq ROW<geom GEOMETRY>` with `sequence.field=seq`
passes `validateTableSchema`, but creating the writer's
`UserDefinedSeqComparator` fails in `GenerateUtils.generateRowCompare` with
`IllegalArgumentException: Illegal type: GEOMETRY(OGC:CRS84)`.
Could we reject sequence fields whose type recursively contains a geospatial
type (similar to the clustering-column check above), and add a regression test?
--
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]
