github-actions[bot] commented on code in PR #69032:
URL: https://github.com/apache/doris/pull/69032#discussion_r4426807477
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/RuntimeFilterPushDownVisitor.java:
##########
@@ -119,13 +119,15 @@ public static PushDownContext
createPushDownContext(RuntimeFilterContext rfConte
/**
* A context is valid if probeExpr references exactly one input slot
and both expressions
- * have non-Variant types. Scalar runtime filters cannot express
Variant canonical equality;
- * explicit casts to supported scalar types can still produce runtime
filters.
+ * have supported predicate types. Spatial types have no BE runtime
predicate kernel;
+ * Variant canonical equality is also unsupported. Supported scalar
projections can still filter.
* Invalid when probeExpr is a constant (e.g., from OneRowRelation) or
multi-slot.
*/
public boolean isValid() {
return !srcExpr.getDataType().isVariantType()
&& !probeExpr.getDataType().isVariantType()
+ && !srcExpr.getDataType().isSpatialType()
Review Comment:
[P2] Reject nested spatial runtime-filter keys. isSpatialType() only matches
direct Geometry/Geography, so ARRAY/MAP/STRUCT values containing a spatial leaf
pass this guard and isScanAcceptableProbeExpr accepts the bare slot. The BE
runtime-filter factories have no complex-type or spatial implementations and
throw while creating the set/minmax/bloom filter. Use the recursive
typeContainsSpatialType() check in both visitor gates.
##########
be/src/core/data_type/define_primitive_type.h:
##########
@@ -75,7 +75,9 @@ enum PrimitiveType : PrimitiveNative {
TYPE_VARBINARY, /* 41, varbinary */
TYPE_TIMESTAMPTZ, /* 42, timestamptz */
TYPE_TIMESTAMP_NS, /* 43, signed Int64 epoch nanoseconds
*/
- TYPE_UUID /* 44, uuid */
+ TYPE_UUID, /* 44, uuid */
+ TYPE_GEOMETRY, /* 45, Iceberg geometry */
Review Comment:
[P1] Handle identity partitions on spatial columns. Adding
GEOMETRY/GEOGRAPHY lets IcebergSchemaBuilder accept identity for these columns,
but the BE partition writer has no spatial cases in value extraction or
path/commit rendering and throws Unsupported type for partition on each dynamic
INSERT. Reject this transform during DDL or serialize the WKB bytes
consistently for both path and commit values.
##########
be/src/format/table/iceberg/types.cpp:
##########
@@ -192,6 +201,31 @@ std::unique_ptr<PrimitiveType>
Types::from_primitive_string(const std::string& t
return std::make_unique<DecimalType>(precision, scale);
}
+ std::regex geometry(R"(geometry\s*(?:\(\s*([^)]*?)\s*\))?)",
std::regex::icase);
+ if (std::regex_match(type_string, match, geometry,
std::regex_constants::match_default)) {
+ const std::string crs =
+ match[1].matched ? trim(match[1].str()) :
GeometryType::DEFAULT_CRS;
+ if (crs.empty()) {
+ throw doris::Exception(doris::ErrorCode::INTERNAL_ERROR,
+ "Invalid CRS: (empty string)");
+ }
+ return std::make_unique<GeometryType>(crs);
+ }
+
Review Comment:
[P2] Preserve comma-bearing Geography CRS values. FE accepts quoted custom
CRS text and GeographyType serializes it inside geography(<crs>, <algorithm>),
but this regex stops CRS capture at the first comma and rejects a value such as
projjson:{"a":1,"b":2} while loading the table. Parse the separator without
treating commas inside CRS as delimiters, or enforce the same restriction when
the type is created.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergTypeMapping.java:
##########
@@ -126,6 +127,13 @@ private static ConnectorType
fromPrimitive(Type.PrimitiveType primitive,
case UUID:
// Preserve logical UUID semantics independently of the binary
mapping option.
return ConnectorType.of("UUID");
Review Comment:
[P2] Preserve spatial type identity in schema-evolution metadata. These new
GEOMETRY/GEOGRAPHY mappings still fall through
IcebergSchemaUtils.buildPrimitiveColumnType() to a TPrimitiveType.STRING
carrier. BE restore_current_primitive_type() trusts that carrier and replaces
the projected DataTypeSpatial fallback with STRING, so missing-field initial
defaults and equality-delete keys are parsed/compared as strings and lose
CRS/type identity. Preserve the resolved spatial fallback (TColumnType has no
CRS/algorithm fields) or extend the carrier, and add native
default/equality-delete coverage.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java:
##########
@@ -1146,6 +1146,15 @@ public void createTable(ConnectorSession session,
ConnectorCreateTableRequest re
rejectReservedRowLineageColumns(request);
validateSortOrder(request);
Schema schema = IcebergSchemaBuilder.buildSchema(request.getColumns());
+ boolean spatial = false;
+ for (Types.NestedField field : schema.columns()) {
+ spatial |=
IcebergSpatialSchemaValidator.containsSpatial(field.type());
+ }
+ if (spatial) {
Review Comment:
[P2] Validate existing spatial tables at write-plan admission. This new
check runs on CREATE/ALTER paths, but planWrite() only calls
validateVariantWrite before building INSERT/UPDATE/MERGE/REWRITE sinks. An
externally-created V2 or ORC Iceberg table with GEOMETRY/GEOGRAPHY is mapped as
spatial and reaches a sink that cannot use the spatial Parquet writer. Apply
the same recursive schema plus format/version validation before every data-file
write.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
