[feature](iceberg) Add Iceberg V3 spatial type support - #67907
Open
OIiveirra wants to merge 37 commits into
Open
[feature](iceberg) Add Iceberg V3 spatial type support#67907OIiveirra wants to merge 37 commits into
OIiveirra wants to merge 37 commits into
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate and critical findings affect metadata serialization, spatial I/O, function binding, validation, and test compliance.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds Iceberg V3 GEOMETRY and GEOGRAPHY support for external tables, including CRS/algorithm metadata, raw-WKB Parquet I/O, and spatial-function integration.
Changes:
- Adds FE/BE spatial types and Thrift/protobuf metadata.
- Adds Iceberg schema conversion, Parquet annotations, WKB handling, and write validation.
- Adds spatial functions, tests, and regression coverage.
File summaries
| File | Summary |
|---|---|
regression-test/suites/external_table_p0/iceberg/test_iceberg_spatial_v3.groovy |
Adds Iceberg V3 spatial regression coverage. |
gensrc/thrift/Types.thrift |
Adds spatial primitive metadata. |
gensrc/proto/types.proto |
Adds spatial protobuf metadata fields. |
fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinitionTest.java |
Tests internal-table spatial rejection. |
fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/SpatialConstructorFunctionTest.java |
Tests spatial constructors. Critical: uses disallowed JUnit 4 imports; switch to JUnit 5. |
fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergUtilsTest.java |
Tests Iceberg spatial mapping and write rules. |
fe/fe-core/src/test/java/org/apache/doris/datasource/iceberg/IcebergSpatialWriteAnalyzerTest.java |
Tests spatial write validation. |
fe/fe-core/src/main/java/org/apache/doris/nereids/types/GeometryType.java |
Adds parameterized geometry type. |
fe/fe-core/src/main/java/org/apache/doris/nereids/types/GeographyType.java |
Adds parameterized geography type. |
fe/fe-core/src/main/java/org/apache/doris/nereids/types/DataType.java |
Integrates spatial Nereids types. |
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java |
Rejects spatial internal-table columns. Moderate: nested spatial types can bypass this check. |
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/ScalarFunctionVisitor.java |
Adds spatial function visitors. |
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/StGeomFromWKB.java |
Adds geometry WKB construction. |
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/StGeometryFromWKB.java |
Adds geometry WKB construction alias. |
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/StGeogFromWKB.java |
Adds geography WKB construction. |
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/StAstext.java |
Adds spatial-aware text conversion. Moderate: other supported spatial functions still lack spatial-aware signatures. |
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSink.java |
Integrates spatial INSERT validation. |
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindExpression.java |
Integrates spatial MERGE validation. |
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergUtils.java |
Maps Iceberg spatial schemas and validates writes. Moderate: nested spatial schemas pass analysis but fail in the Parquet writer. |
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergSpatialWriteAnalyzer.java |
Validates spatial write compatibility. Moderate: untyped NULL is rejected before sink coercion. |
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/DorisTypeToIcebergType.java |
Converts Doris spatial types to Iceberg types. Moderate: CREATE/ALTER paths lack the required V3/Parquet guard. |
fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinScalarFunctions.java |
Registers spatial functions. |
fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnDef.java |
Rejects spatial legacy internal-table columns. Moderate: nested spatial types can bypass this check. |
fe/fe-common/src/test/java/org/apache/doris/catalog/SpatialTypeTest.java |
Tests catalog spatial types. |
fe/fe-common/src/main/java/org/apache/doris/catalog/Type.java |
Handles spatial type metadata. |
fe/fe-common/src/main/java/org/apache/doris/catalog/ScalarType.java |
Preserves CRS and geography algorithms. |
fe/fe-common/src/main/java/org/apache/doris/catalog/PrimitiveType.java |
Registers spatial primitive types. |
be/test/format/transformer/vparquet_transformer_test.cpp |
Tests spatial Parquet writing. |
be/test/format/table/iceberg/arrow_schema_util_test.cpp |
Tests Iceberg Arrow schema mapping. |
be/test/exprs/function/function_geo_test.cpp |
Tests spatial functions. Moderate: both Geography test paths omit the required CRS and algorithm. |
be/test/core/data_type/storage_field_type_test.cpp |
Tests storage field type mappings. |
be/test/core/data_type/data_type_spatial_test.cpp |
Tests backend spatial type behavior. |
be/src/format/transformer/vparquet_transformer.cpp |
Writes spatial Parquet annotations. |
be/src/format/table/iceberg/types.h |
Defines Iceberg spatial types. |
be/src/format/table/iceberg/types.cpp |
Implements Iceberg spatial type handling. |
be/src/format/table/iceberg/arrow_schema_util.h |
Declares Arrow schema utilities. |
be/src/format/table/iceberg/arrow_schema_util.cpp |
Adds Iceberg spatial Arrow annotations. Moderate: the read path lacks matching spatial schema conversion. |
be/src/exprs/function/geo/wkb_parse.h |
Declares WKB parsing helpers. |
be/src/exprs/function/geo/wkb_parse.cpp |
Implements raw-WKB parsing. |
be/src/exprs/function/geo/geo_types.h |
Defines geometry types. |
be/src/exprs/function/geo/geo_types.cpp |
Implements geometry types. |
be/src/exprs/function/geo/geo_common.cpp |
Provides shared spatial functionality. |
be/src/exprs/function/geo/functions_geo.h |
Declares spatial functions. |
be/src/exprs/function/geo/functions_geo.cpp |
Adds WKB parsing and spatial validation. Moderate: Geography construction can omit its required default algorithm; 0x-prefixed hex is mishandled; spatial function signatures are not reachable from many Nereids calls. |
be/src/core/data_type/storage_field_type.cpp |
Adds spatial storage mappings. |
be/src/core/data_type/primitive_type.cpp |
Registers backend primitive mappings. |
be/src/core/data_type/define_primitive_type.h |
Defines primitive type registration. |
be/src/core/data_type/data_type_spatial.h |
Declares backend spatial types. |
be/src/core/data_type/data_type_spatial.cpp |
Implements spatial types and serialization. Moderate: protobuf serialization omits CRS and Geography algorithm metadata. |
be/src/core/data_type/data_type_factory.cpp |
Constructs backend data types. Moderate: protobuf deserialization ignores serialized spatial metadata. |
be/src/core/data_type_serde/data_type_varbinary_serde.cpp |
Supports spatial-related varbinary serialization. |
be/src/core/column/column_spatial.h |
Declares raw-WKB spatial columns. |
be/src/core/column/column_spatial.cpp |
Implements raw-WKB spatial columns. |
Review details
Suppressed comments (9)
be/src/core/data_type/data_type_factory.cpp:503
- Even if the protobuf fields are populated, the protobuf
DataTypeFactorypath falls through tocreate_data_type(primitive_type, ...), which reaches these default constructors and ignoresPScalarType.spatial_*. As a result, non-default CRS/algorithm values cannot survive protobuf deserialization. Add a spatial branch in the protobuf overload that constructsDataTypeSpatialfrom the serialized fields, using defaults only for genuinely missing legacy fields.
case TYPE_GEOMETRY:
nested = std::make_shared<DataTypeSpatial>(TYPE_GEOMETRY);
break;
case TYPE_GEOGRAPHY:
nested = std::make_shared<DataTypeSpatial>(TYPE_GEOGRAPHY, "OGC:CRS84", "spherical");
be/src/exprs/function/geo/functions_geo.cpp:790
- The decoder below explicitly accepts
0x-prefixed hex, but the precedingGeoShape::from_wkbcall parses the original text and only strips the\\xform. ConsequentlyST_GEOMFROMWKB('0x...')andST_GEOGFROMWKB('0x...')are marked NULL before this decoder is reached. Decode the hex first and validate the resulting bytes withfrom_wkb_bytes.
auto value = geo->get_data_at(row);
std::unique_ptr<GeoShape> shape = GeoShape::from_wkb(value.data, value.size, status);
if (shape == nullptr || status != GEO_PARSE_OK) {
null_map_data[row] = 1;
res->insert_default();
continue;
}
if (!decode_wkb_hex(value, &wkb)) {
be/src/exprs/function/geo/functions_geo.cpp:80
- The new raw-WKB decoder is not reachable for most Nereids calls with the new spatial types.
StLength,StAsBinary,StGeometryType,StX/StY, and the measurement/relationship functions still advertise only Varchar/String signatures and, unlikeStAstext, have no spatial-awaresearchSignature; aGEOGRAPHY(...)argument therefore fails FE signature resolution before this helper andvalidate_geography_semanticscan run. Add spatial overloads or equivalent FE signature handling for the functions covered by this backend support.
static std::unique_ptr<GeoShape> decode_geo_shape(StringRef value, const DataTypePtr& type,
GeoParseStatus* parse_status = nullptr) {
if (!is_spatial_type(type)) {
return GeoShape::from_encoded(value.data, value.size);
}
GeoParseStatus status;
auto shape = GeoShape::from_wkb_bytes(value.data, value.size, status);
if (parse_status != nullptr) {
*parse_status = status;
}
return status == GEO_PARSE_OK ? std::move(shape) : nullptr;
be/src/format/table/iceberg/arrow_schema_util.cpp:95
- These annotations make the writer emit spatial Parquet metadata, but the Iceberg read path still has no matching support: V1
FieldDescriptor::convert_to_doris_typefalls back from GEOMETRY/GEOGRAPHY BYTE_ARRAY fields to VARBINARY/STRING, andPhysicalToLogicalConverterhas no conversion intoDataTypeSpatial. Consequently the regression's read-back after insert cannot materialize the WKB into a spatial column. Add the Parquet schema/converter mapping for both annotations before relying on this write-only Arrow mapping.
case iceberg::TypeID::GEOMETRY:
metadata[ICEBERG_BINARY_TYPE] = GEOMETRY_BINARY_TYPE_VALUE;
arrow_type = arrow::binary();
break;
be/test/exprs/function/function_geo_test.cpp:181
- This second test path creates a Geography result type with an empty algorithm as well. It violates the same
DataTypeSpatialinvariant and can fail under DCHECK-enabled builds before the unsupported-WKB behavior is tested; provideOGC:CRS84andsphericalfor the Geography branch.
auto result_type = make_nullable(std::make_shared<DataTypeSpatial>(
function_name == "st_geomfromwkb" ? TYPE_GEOMETRY : TYPE_GEOGRAPHY));
fe/fe-core/src/main/java/org/apache/doris/analysis/ColumnDef.java:431
- The legacy analyzer has the same root-only check: nested
ARRAY<GEOMETRY>/STRUCT<...GEOGRAPHY>types do not have GEOMETRY or GEOGRAPHY as their root primitive and therefore can pass internal-table analysis, even thoughDataTypeSpatialcannot be stored by Doris internal tables. Apply the recursive spatial rejection used by the Nereids path here as well.
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/DorisTypeToIcebergType.java:144 - These new mappings are also used by Iceberg CREATE/ALTER schema operations, but those paths only apply the existing VARIANT validation (
IcebergMetadataOps.java:399,772) and CREATE defaults to format-version 2 (IcebergMetadataOps.java:397). A spatial column can reach schema creation or alteration without the required v3/Parquet guard, even though the new writer only supports that combination; add the spatial validation before committing the schema.
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/StAstext.java:80 - This special-case signature handling only makes
ST_ASTEXTbind a spatial operand.StDistance,StContains,StLength,StAsBinary,StGeometryType,StX, andStYstill expose only Varchar/String signatures, so Nereids rejects calls such asST_DISTANCE(iceberg_geography, iceberg_geography)before the new BE implementations can run. Add corresponding spatial-aware signatures/search handling for the supported functions while retaining the Geography metadata checks.
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/ColumnDefinition.java:388 - This check only examines the root catalog primitive, so an internal column such as
ARRAY<GEOMETRY>orSTRUCT<shape GEOGRAPHY>bypasses the rejection because its root is ARRAY/STRUCT. The nestedDataTypeSpatialis later unsupported by storage, so reject spatial types recursively here (or reject nested spatial schemas explicitly) to preserve the stated internal-table restriction.
- Files reviewed: 53/53 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+26
to
+27
| import org.junit.Assert; | ||
| import org.junit.Test; |
Comment on lines
+126
to
+127
| void DataTypeSpatial::to_protobuf(PTypeDesc* /* ptype */, PTypeNode* /* node */, | ||
| PScalarType* /* scalar_type */) const {} |
Comment on lines
+812
to
+814
| DataTypePtr get_return_type_impl(const DataTypes&) const override { | ||
| return make_nullable(std::make_shared<DataTypeSpatial>(Impl::OUTPUT_TYPE)); | ||
| } |
| input_column->insert_data("0101000000000000000000F03F0000000000000040", 42); | ||
| auto input_type = std::make_shared<DataTypeString>(); | ||
| ColumnsWithTypeAndName arguments {{std::move(input_column), input_type, "wkb"}}; | ||
| auto result_type = make_nullable(std::make_shared<DataTypeSpatial>(TYPE_GEOGRAPHY)); |
Comment on lines
+94
to
+98
| static void validateSpatialConversion(Type sourceType, ScalarType targetType, String columnName) { | ||
| if (!(sourceType instanceof ScalarType) || !((ScalarType) sourceType).isSpatialType()) { | ||
| throw new AnalysisException("Iceberg spatial write cannot convert input column '" + columnName | ||
| + "' from " + sourceType.toSql() + " to " + targetType.toSql()); | ||
| } |
Comment on lines
827
to
+833
| public static void validateWriteSchema(Table table, List<Column> columns) { | ||
| boolean writesVariant = columns.stream().anyMatch(column -> containsVariant(column.getType())); | ||
| boolean writesSpatial = columns.stream().anyMatch(column -> containsSpatial(column.getType())); | ||
| FileFormat fileFormat = getFileFormat(table); | ||
| if (writesVariant) { | ||
| if (writesVariant || writesSpatial) { | ||
| validateWriteSchema(columns, getFormatVersion(table), fileFormat); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: N/A (DORIS-28228)
Related PR: N/A
Problem Summary:
Add Iceberg V3
GEOMETRYandGEOGRAPHYsupport for external tables. The change adds bidirectional Iceberg schema mapping with CRS and Geography algorithm preservation, Arrow/Parquet spatial annotations, raw WKB read/write handling, and spatial function support.Scope is limited to Iceberg external tables. Doris internal tables reject these types. Paimon, Lance, GeoArrow, spatial indexes, bounds pruning, and predicate pushdown are intentionally excluded.
For Geography, non-default CRS/algorithm values are preserved for read and write. Spatial measurement and relationship functions currently require
GEOGRAPHY(OGC:CRS84, spherical)and return a stable NotSupported error for other combinations to avoid incorrect calculations.Release note
Support Iceberg V3 GEOMETRY and GEOGRAPHY external-table schema mapping and raw-WKB Parquet read/write.
Check List (For Author)
Test
IcebergUtilsTest,IcebergSpatialWriteAnalyzerTest,SpatialConstructorFunctionTest, andColumnDefinitionTest).javax.annotation.Generatedand Nereids pattern-generator classpath issues.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)