Repository navigation
Conversation
### What problem does this PR solve?\n\nIssue Number: close apache#28228\n\nRelated PR: apache#67907\n\nProblem Summary: Preserve parameterized geometry and geography types through Nereids function binding and add the geography WKB constructor on master.\n\n### Release note\n\nAdd spatial function type binding for geometry and geography values.\n\n### Check List (For Author)\n\n- Test: FE focused test pending; BE build pending\n- Behavior changed: Yes (spatial function return and input types)\n- Does this need documentation: No
Align generated Nereids spatial constructors with the native Geometry return type and preserve spatial inputs for ST_ASTEXT.
Record the master-port decisions, focused FE test result, and remaining BE and format gaps.
Add Geometry and Geography to the BE Iceberg schema model, preserve their Arrow binary metadata, and reject spatial values in internal-table DDL including nested containers.
Allow Arrow and Parquet binary materialization to operate on the spatial column container while preserving existing VARBINARY behavior. ### Check List (For Author)\n\n- Test: FE DDL test pending; BE build pending\n- Behavior changed: Yes (spatial binary read/write)\n- Does this need documentation: No
Allow spatial functions to consume raw WKB columns and preserve canonical WKB bytes in geometry and geography constructors. ### Check List (For Author)\n\n- Test: BE unit test pending; FE tests unaffected\n- Behavior changed: Yes (spatial WKB function inputs)\n- Does this need documentation: No
Reject malformed spatial payloads before geo functions execute and expose the validator for Iceberg write paths. ### Check List (For Author)\n\n- Test: BE unit test pending; dependency setup still blocks BE build\n- Behavior changed: Yes (invalid spatial input errors)\n- Does this need documentation: No
Emit top-level Geometry and Geography logical annotations in Iceberg Parquet files, validate WKB inputs before conversion, and reject nested spatial columns that the writer cannot represent. ### Check List (For Author)\n\n- Test: C++ writer test added; BE build pending dependency setup\n- Behavior changed: Yes (Iceberg Parquet schema and input validation)\n- Does this need documentation: No
### What problem does this PR solve?\n\nIssue Number: N/A\n\nRelated PR: apache#67907\n\nProblem Summary: Validate raw spatial WKB inputs before spatial functions and Iceberg writes while preserving Geography SRID compatibility.\n\n### Release note\n\nNone\n\n### Check List (For Author)\n\n- Test: FE focused tests passed; BE build blocked during third-party submodule preparation\n- Behavior changed: Yes (invalid Geometry WKB is rejected earlier)\n- Does this need documentation: No
### What problem does this PR solve?\n\nIssue Number: N/A\n\nRelated PR: apache#67907\n\nProblem Summary: Remove stale comments that described explicitly supported Iceberg spatial types as unsupported and record the current validation commit in the work log.\n\n### Release note\n\nNone\n\n### Check List (For Author)\n\n- Test: No need to test; comment and worklog only\n- Behavior changed: No\n- Does this need documentation: No
### What problem does this PR solve?\n\nIssue Number: N/A\n\nRelated PR: apache#67907\n\nProblem Summary: The mapping implementation handles Geometry and Geography explicitly, but its fallback comment still listed them as unsupported.\n\n### Release note\n\nNone\n\n### Check List (For Author)\n\n- Test: No need to test; comment only\n- Behavior changed: No\n- Does this need documentation: No
### What problem does this PR solve?\n\nIssue Number: N/A\n\nRelated PR: apache#67907\n\nProblem Summary: Add an external Iceberg V3 regression that creates a Geometry table, writes raw WKB through Doris, reads it back from Parquet, and checks ST_AsText output.\n\n### Release note\n\nNone\n\n### Check List (For Author)\n\n- Test: Added gated external Iceberg regression; not run because the external Docker environment is unavailable\n- Behavior changed: No\n- Does this need documentation: No
### What problem does this PR solve?\n\nIssue Number: N/A\n\nRelated PR: apache#67907\n\nProblem Summary: Generic connector sink coercion could hide Geometry/Geography kind, CRS, and edge-algorithm mismatches before Iceberg writes. Validate spatial source expressions against target metadata before the common coercion path, while preserving NULL writes.\n\n### Release note\n\nNone\n\n### Check List (For Author)\n\n- Test: ./build.sh --fe; FE_UT_PARALLEL=1 ./run-fe-ut.sh --run org.apache.doris.datasource.connector.converter.ConnectorSpatialWriteValidatorTest\n- Behavior changed: Yes (invalid spatial write conversions fail during analysis)\n- Does this need documentation: No
### What problem does this PR solve?\n\nIssue Number: N/A\n\nRelated PR: apache#67907\n\nProblem Summary: Adding external-only spatial primitive types left the exhaustive storage-field mapping without explicit cases, causing BE compilation to fail under -Werror.\n\n### Release note\n\nNone\n\n### Check List (For Author)\n\n- Test: BE build resumed after this compile fix\n- Behavior changed: No; spatial types still reject internal segment storage\n- Does this need documentation: No
### What problem does this PR solve? Related PR: apache#67907 Problem Summary: Align spatial serialization with current master APIs. Put optional spatial metadata on PScalarType, adapt generic binary SerDe to StringRef and Arrow offsets, remove obsolete column overrides and version constants, and verify Parquet CRS through concrete logical types. ### Release note None ### Check List (For Author) - Test: ./build.sh --be passed; BE spatial unit tests compiled but ASAN linking failed due toolchain duplicate glibc symbols; release retry pending - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#67907 Problem Summary: Connector changelog UPDATE and MERGE plans returned before the generic sink validator, allowing spatial metadata to be coerced without checking kind, CRS, or geography algorithm. Validate each changelog expression before casting and cover both paths with plan-builder tests. ### Release note None ### Check List (For Author) - Test: FE connector spatial and changelog plan tests, 13 tests passed - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#67907 Problem Summary: Geography values created without explicit metadata are valid under the Iceberg V3 default contract, but the BE type constructor rejected an empty algorithm. Normalize the default to spherical while preserving Geometry and explicit metadata semantics. ### Release note None ### Check List (For Author) - Test: focused BE gtest exposed and isolated the default metadata case; follow-up spatial suite in progress - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#67907 Problem Summary: Keep projected Geometry accessors in raw WKB space, validate S2 Geography metadata before execution, preserve legacy encoded WKB plans and current master geo functions, fix spatial serialized-size accounting, and use the supported Parquet group annotation API. Cover projected Geometry and legacy EWKB with focused tests. ### Release note Spatial metadata mismatches fail explicitly rather than using incorrect spherical semantics. ### Check List (For Author) - Test: run-be-ut.sh, 86 tests passed with ASAN and local GLIBC_COMPATIBILITY=OFF; clang-format 16.0.6; build hygiene passed; clang-tidy in progress - Behavior changed: Yes, reject unsupported spatial metadata - Does this need documentation: Yes, pending spatial feature documentation
### What problem does this PR solve? Related PR: apache#67907 Problem Summary: Apply clang-format 16 to the spatial feature scope and resolve new auto and integer sign comparison diagnostics while retaining verified geo behavior. ### Release note None ### Check List (For Author) - Test: run-be-ut.sh, 86/86 passed with ASAN and local GLIBC_COMPATIBILITY=OFF; check-format.sh passed; whitespace and build hygiene passed - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#67907 Problem Summary: Resolve qualified-auto diagnostics in the new spatial column implementation and record verified personal test service health. ### Release note None ### Check List (For Author) - Test: full feature clang-tidy in progress; existing spatial BE suite passed 86/86 before these qualifier-only changes - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#67907 Problem Summary: ST_AsBinary copied Geometry WKB directly while ignoring the nullable column null map. Propagate NULL before copying raw bytes and test accessors with valid hidden payloads. Also resolve the fixed binary width integer-comparison diagnostic. ### Release note ST_AsBinary returns NULL for nullable Geometry input rows. ### Check List (For Author) - Test: run-be-ut.sh, 87/87 spatial and geo tests passed with ASAN and local GLIBC_COMPATIBILITY=OFF - Behavior changed: Yes, preserve NULL results - Does this need documentation: No
### What problem does this PR solve? Issue Number: N/A Related PR: apache#67907 Problem Summary: Iceberg spatial schemas have native Doris types, but SQL CREATE and ALTER cannot declare them. Add Geometry and Geography grammar with CRS and edge algorithm parameters, preserve their use as non-reserved column names, and normalize supported Geography algorithms in the shared type factory. Dispatch column types through the parser visitor so CREATE and ALTER use the same type parsing path. ### Release note Allow GEOMETRY and GEOGRAPHY declarations with CRS and Geography edge algorithm parameters for Iceberg external tables. ### Check List (For Author) - Test: Unit Test; standard run-fe-ut.sh, DataTypeTest 38/38 and ColumnDefinitionTest 8/8 passed, fe-spatial-ddl-ut.log BUILD SUCCESS - Behavior changed: Yes; SQL recognizes parameterized spatial types and rejects unsupported Geography algorithms - Does this need documentation: Yes; spatial external-table documentation pending
### What problem does this PR solve? Issue Number: N/A Related PR: apache#67907 Problem Summary: Iceberg CREATE and column evolution can introduce spatial columns on V2 or non-Parquet tables, or introduce nested spatial values that Doris cannot write. Validate these constraints before remote schema mutations. CREATE uses effective catalog override, request, and default properties; ADD and MODIFY use actual table metadata and the writer's existing format resolution, including the write-format alias. Ordinary column changes retain their existing path. ### Release note Reject Iceberg spatial schema changes that require unsupported table versions, write formats, or nested spatial writes. ### Check List (For Author) - Test: Unit Test; standard run-fe-ut.sh via personal wrapper, IcebergSpatialDdlTest 8/8, metadata DDL 37/37 and column evolution 23/23 passed, fe-spatial-spi-ddl-ut.log BUILD SUCCESS - Behavior changed: Yes; unsupported spatial schema changes fail before remote mutation - Does this need documentation: Yes; spatial external-table limitations documentation pending
### What problem does this PR solve? Related PR: apache#69032 Problem Summary: Valid raw Geography LineStrings can round to antipodal S2 endpoints and fail admission before stable calculation. Use the existing raw-arc preparation for typed lines and polygon rings, retaining line closure, prior alias admission and explicit invalid-edge rejection. Separately, repeated SELECT NULL slots reused one ExprId when retagged for spatial sink columns, allowing project merging to corrupt an ordinary sibling cast. Give each retagged output its own Alias identity and retain existing sink binding. ### Release note Correct valid near-antipodal Geography calculations and repeated NULL writes into ordinary and spatial Iceberg columns. Exactly antipodal edges remain unsupported; raw WKB and legacy STRING behavior are preserved. ### Check List (For Author) - Test: 131 targeted ASAN BE Geo/WKB tests; 44 FE normalizer, validator and INSERT tests; standard BE/FE ASAN build; GIS and Iceberg spatial regression generation/verification; clang-format16, build hygiene and changed-line clang-tidy - Behavior changed: Yes, admitted Geography arcs are stabilized before validation and independently retagged NULL outputs retain distinct identities - Does this need documentation: No, existing supported-domain boundaries are unchanged
|
/review |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
There was a problem hiding this comment.
Static review of e3b3d00: one new P2 correctness finding is inline. Equivalent raw-meridian subdivision can change ST_Touches because a rounded-zero atomic edge becomes an interior loop. No new P0/P1 was verified.
The historical P1 4269360799 is fixed: both scanner schema converters retain optional spatial nullability, and COUNT's definition-level threshold excludes NULLs. No existing blocking comment remains independently confirmed. The deferred nullable-selection performance issue 4367818661 still applies and is not duplicated.
Critical checkpoint conclusions:
- Goal and proof: Iceberg V3 spatial identity/read/write is integrated across FE and BE; the inline topology defect prevents complete correctness. Unit and regression sources cover the principal flows.
- Scope and reuse: the broad change follows the required type, connector, planner, reader and writer boundaries; binary SerDe behavior is shared rather than reimplemented.
- Concurrency and locks: new normalization/noding state is local to each call. No new thread entry, shared synchronization variable or lock-order defect was identified; writer cleanup waits for existing asynchronous close work.
- Lifecycle and initialization: spatial arenas own copied WKB; CASE/aggregate outputs own retained values. Resolver caches and shape indexes have scoped lifetimes; no new cross-TU initialization dependency was found.
- Configuration: no new configuration item requires dynamic-update handling.
- Compatibility: legacy STRING and ordinary BINARY paths remain separately dispatched; spatial metadata and Parquet annotations are preserved. Connector SPI moves to major 23. Old engines do not support the new spatial types; mixed-major execution is outside this feature's stated scope.
- Protocol IDs: every touched definition was compared with branch-3.1, 4.0, 4.1 and 4.2. New values/IDs are unused there, and added fields are optional; no collision was found.
- Parallel paths: both Parquet scanners, annotated/unannotated inputs, explicit Hive binary overlays, scalar/nullable/constant/nested values, VALUES/SELECT/CTE, data-write/delete-only MERGE and changelog paths were traced.
- Conditional checks: added admission and identity checks have concrete unsupported inputs and explicit failures. Fresh aliases preserve independently retagged outputs; NULL provenance tracing stops at joins and other semantic boundaries.
- Test coverage: BE/FE unit and SQL regression sources exercise both kinds, metadata variants, nested NULLs, ordinary controls and negative cases. The rounded-collision plus OPEN-interior combination in the finding is missing.
- Expected results: all three changed golden files were inspected against their SQL and relevant consumers, including WKB identity, NULL counts, ordering and units. Their generation and runtime results were not verified in this invocation.
- Observability: binding/format failures retain operation/type context; existing native reader counters expose selection fallback. No new distributed phase requiring separate metrics was identified.
- Persistence and transactions: no new EditLog, visible-version, delete-bitmap or transaction-state protocol is added; external metadata uses the existing connector mutation path.
- Writes and crash boundaries: validation precedes sink coercion; successful close still gates existing Iceberg commit. Failure cleanup closes outstanding writer work before deleting output; no additional partial-commit path was identified.
- FE-BE passing: kind, CRS and Geography algorithm are carried through scalar descriptors and recursive protobuf/block paths; function return metadata and NULL handling were checked.
- Performance: raw parsing avoids typed hex round trips, mutable filtering avoids payload recopying, and NULL lineage uses scoped caches. The known native nullable-selection fallback remains; no additional distinct performance finding was verified or benchmarked.
- Further issues: the final sweep covered all 182 changed paths and independently classified all candidates and initial risks; no unresolved code candidate or uncovered changed-path category remains.
No additional user focus was supplied. Review converged in 2 rounds after complete FE/BE coverage and a separate risk pass; all latest-round reviewers returned NO_NEW_VALUABLE_FINDINGS against the frozen payload. Convergence describes this bounded review, not a claim that the feature is defect-free.
Validation was static inspection and arithmetic/source analysis only. No builds, tests, benchmarks, services, source edits or submodule initialization were performed. Runtime interoperability, crash behavior and performance remain unexecuted validation limits.
Review converged after 2 round(s).
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
### What problem does this PR solve? Related PR: apache#69032 Problem Summary: Merge the two official upstream/master commits. Preserve the upstream connector security surface and lance-c runtime BTree updates while retaining the existing connector API 23.0 spatial capability record. ### Release note None ### Check List (For Author) - Test: ConnectorPluginSurfaceTest and targeted spatial tests will be rerun after the merge - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69032 Problem Summary: Distinct raw WKB vertices can round to one S2Point. The zero-length S2 edge must remain available for raw identity but must not become an OPEN or AA boundary marker, which changes ST_Touches for an equivalent meridian subdivision. Skip only the zero-length S2 edge and retain real raw endpoints. ### Release note None ### Check List (For Author) - Test: 99 targeted Geography BE unit tests passed before the upstream merge; the new rounded-zero ST_Touches test passed in that run; FE ConnectorPluginSurfaceTest 2/2 passed after the merge - Behavior changed: Yes, supported Geography ST_Touches preserves open endpoint semantics - Does this need documentation: No
|
/review |
There was a problem hiding this comment.
Request changes: one new P1 protocol-identity collision and one new P2 connector-plugin compatibility defect, detailed inline. Reviewed all 181 changed paths at 9307cec, with complete FE/regression and BE/protocol reviews plus a separate spherical-topology risk review.
Critical checkpoint conclusions:
- Goal and evidence: the Iceberg V3 spatial read/write chain, parameterized types, raw WKB accessors and default spherical Geography calculations are wired end to end. Added unit/regression sources cover these paths; execution was prohibited in this review.
- Scope and reuse: the cross-module changes support the feature; concrete binary SerDe, owning aggregate states and existing sink interfaces are reused. No unrelated production change was identified.
- Concurrency and locks: analysis/noding state is per invocation; test counters are thread-local. No new worker or lock-order change was found. Failed writer cleanup waits for outstanding close work before object deletion.
- Lifecycle and initialization: spatial columns and aggregate states own WKB; filtering retains its arena and erasure reclaims dead payload. Temporary indexes/resolver caches use scoped lifetimes. No new dependent static-initialization issue was identified.
- Configuration: no production configuration item was added.
- Symbol/storage compatibility: external spatial annotations and binary output metadata are explicit; internal Doris spatial storage remains rejected. The expanded connector SPI retains its previous major, causing the P2 plugin/kernel mismatch.
- Thrift/Protobuf: compared every touched definition on HEAD and branch-3.1/4.0/4.1/4.2. New metadata fields are optional and unused in those branches, but Geometry reuses branch-4.1 FILE enum values in both protocol domains (P1). This is distinct from the earlier feature-negotiation/heartbeat discussion.
- Parallel paths: checked Parquet v1/v2, annotated/unannotated binary, dictionary/selected values, nested/nullable/constant carriers, both area units, typed/legacy functions, INSERT VALUES/SELECT/CTE, MERGE data/delete-only and changelog writes.
- Conditional checks: shape, CRS/algorithm, NULL-parent and unsupported-operation checks have concrete failure paths. Rounded-coordinate topology residuals remain known review context, described below.
- Test breadth: source coverage includes metadata identities, both spatial kinds, positive/negative admission, ownership, NULLs, nested values, ExprId/project merging, scanner COUNT behavior and failed-writer cleanup. Runtime and interoperability behavior were not independently executed.
- Changed expected results: reviewed the added assertions and regression output against fixture/query definitions; no additional incorrect expected result was established. Output generation provenance and passing results cannot be proved by static inspection.
- Observability: existing Status/analysis errors identify invalid WKB rows, mismatched column identity and failed cleanup paths. Existing nullable-selection fallback accounting remains; no new distributed control flow requiring a separate metric was identified.
- Transactions and persistence: existing Iceberg mutation/commit machinery is reused; no new EditLog or coordinator transaction protocol was added. Protocol identity is blocked by the P1 finding.
- Writes and crash/error paths: spatial validation precedes coercion/file writes, output annotations preserve logical identity, and failed physical-writer teardown completes before cleanup. No additional publication/atomicity defect was verified.
- FE-BE metadata: CRS and Geography algorithm pass through Thrift, protobuf type descriptors and block metadata; spatial constant folding is guarded because FE cannot decode spatial PValues. The enum collision prevents compatible reconstruction of an older FILE descriptor.
- Performance: checked mutable filtering, window arena reclamation, cached NULL lineage, constant-distance decoding and noding work. No benchmark was run; the known candidate-search and nullable-selection limitations remain below.
- Other/user focus: checked statistics exclusions, binary MySQL metadata and ordinary-type controls. No additional user-provided focus was supplied.
Existing-context disposition: the sole earlier P1, 4269360799, is fixed at this head: both schema converters retain nullability and the native COUNT threshold follows it. No existing P0/P1 remains confirmed, so existing_blocking_comment_ids is empty. Prior NULL-provenance/ExprId, stabilized-admission/measurement and unsupported-operation repairs were rechecked. We did not repost residual P2 concerns already covered by rounded identity/open boundaries, raw endpoint identity, dense candidate work, COLLECT_SET dispatch, or the deferred nullable-selection batching. Those mechanisms are not all fully resolved; deduplication does not imply they are fixed.
Validation limits: static source, protocol-definition and arithmetic inspection only. No builds, Doris tests, benchmarks, service startup, source changes or submodule initialization were performed. All assigned changed paths and candidate/risk dispositions are covered; executed validation remains outside this contract.
Review converged after 2 round(s).
FE UT Coverage ReportIncrement line coverage |
### What problem does this PR solve? Issue Number: None Related PR: apache#69032 Problem Summary: The upstream analytic spill tests and spatial window tests add overlapping includes. Merge official master and preserve both test sets with their required headers. ### Release note None ### Check List (For Author) - Test: Manual conflict review; conflict-file whitespace check passed. Upstream spill golden has an inherited blank line at EOF. Focused tests follow with the compatibility fix. - Behavior changed: No; retain upstream behavior and existing spatial tests. - Does this need documentation: No
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69032 Problem Summary: Spatial protocol enums reused branch-4.1 FILE identities, and spatial connector additions still advertised upstream API 23. Reserve the FILE numbers, allocate unused spatial IDs, and require connector API 24 so incompatible plugins fail before provider construction. Add fixed-ID and legacy FILE rejection tests. The performance build also failed because GCC 15's native float128 limits collided with Boost's indirect specialization; disable the unused quadmath wrappers only around this cpp_int include on GCC 15+, preserving the integer calculations. ### Release note Reserve release FILE protocol identities and reject incompatible spatial connector API majors before plugin loading. ### Check List (For Author) - Test: Unit Test / Manual test - Standard targeted ASAN BE UT: 189 passed; final comment-only type-test rerun: 18 passed. - Standard FE SPI/API gate and wiring UT: 194 passed, including 152 SPI own tests. - Standard FE build and Checkstyle; clang-format16, header hygiene and changed-line clang-tidy. - Actual GCC 15.2 cpp_int include probe fails before the fix and passes after it. - Private BE UT compatibility overlay used; these checks do not certify full community CI. - Behavior changed: Yes. Use unambiguous protocol IDs and connector API 24; restore GCC 15 compilation. - Does this need documentation: No. Internal compatibility fix; existing spatial user contract is unchanged.
|
/review |
There was a problem hiding this comment.
One new P2 wrong-result finding: default Geography ST_Equals can treat unequal raw line extents as equal after endpoint conversion. It is detailed inline at the CLOSED equality consumer. Both independently confirmed existing P1s are repaired: optional spatial nullability/COUNT (4269360799), and branch-4.1 FILE protocol identities (4384986088). No existing P0/P1 remains applicable, so existing_blocking_comment_ids is empty. The API-major P2 is also repaired by API 24.0 and its matching surface/version baseline.
Reviewed all 182 changed paths at f02598b using the supplied aggregate diff, two complete FE/BE reviewers and a separate Geography risk reviewer. No additional user focus was supplied. The inspected flow is: Iceberg schema -> ConnectorType/FE type -> Thrift/protobuf -> BE spatial column -> Parquet read/functions/exchange -> validated Arrow/Parquet writer -> existing Iceberg commit.
Critical checkpoint conclusions:
- Goal and proof: typed Iceberg V3 spatial read/write and raw WKB access are implemented, with explicitly bounded spherical XY calculations. Added unit/regression sources exercise the chain; the inline equality case remains a correctness gap.
- Scope and reuse: changes follow the required parser/type/connector/reader/function/writer boundaries and reuse binary SerDe and owning column/aggregate mechanisms. No unrelated production change was identified.
- Concurrency and locks: no new production worker, lock order or shared mutable analysis/noding state was found. Existing asynchronous file publication is the relevant concurrency boundary; cleanup drains writer work before deletion.
- Lifecycle/static initialization: WKB arenas, copied aggregate states, dictionary generations and scoped resolver/index state were traced. Mutable filtering retains its arena; analytic erasure reclaims dead payload. No additional ownership cycle or cross-TU initialization dependency was established.
- Configuration: no new production configuration item was added. Existing catalog defaults/overrides and actual table version/format govern spatial admission.
- Symbol/storage compatibility: the expanded connector SPI now requires major 24, with matching pins/baseline. Legacy STRING/EWKB and ordinary BINARY paths remain separate; internal spatial storage is rejected. Mixed-version spatial execution requires supporting endpoints and is outside the stated feature domain.
- Thrift/protobuf: read every touched definition on HEAD and live-verified branch-3.1/4.0/4.1/4.2/master snapshots. Spatial enum IDs do not reuse FILE, all new metadata field IDs are unused on those branches, and all new fields are optional. A moved branch-4.2 ref was refreshed before concluding this audit.
- Parallel paths: checked both Parquet scanners, annotated/unannotated binary and Hive BINARY overlays, plain/dictionary selection, nullable/constant/nested carriers, both spatial kinds, typed/legacy functions, both area units, VALUES/SELECT/CTE, data-write/delete-only MERGE and changelog writes.
- Conditions/errors: added type/CRS/algorithm/WKB/version/format/NULL-parent checks have concrete failure paths. Source identity is checked before sink casts, and independently retagged NULL outputs receive fresh ExprIds. The equality finding loses raw identity after otherwise valid admission.
- Test coverage: FE/BE unit and SQL regression sources cover metadata mismatch, both kinds/endian encodings, NULLs/EMPTY/dimensions, nesting, ownership, late aggregate/window/state gates, scanner COUNT and failed writer cleanup. Unequal colliding line endpoints at CLOSED equality are missing.
- Changed results: the three changed golden files were reviewed against their queries, WKB, counts, units, ordering and consumer behavior. No additional incorrect expected result was established. Their generation provenance and passing execution were not independently verified.
- Observability: validation retains row/function/column/type context, cleanup logs paths, and existing reader counters expose the known selection fallback. No new distributed phase requiring separate metrics was identified.
- Persistence/transactions: scalar parameters retain JSON/Thrift/protobuf identity; existing Iceberg metadata mutation/commit machinery is reused. No new EditLog, visible-version, delete-bitmap or transaction-state protocol was added.
- Writes/atomicity/crashes: validation precedes coercion/file writes; successful close still gates existing commit. Failed-open/close cleanup finishes publishing owners before deletion. No additional partial-commit, leak or deadlock defect was verified by source inspection.
- FE-BE passing: kind, CRS and Geography algorithm are preserved in scalar/nested descriptors, block metadata and protobuf type factories. Spatial constant folding stays guarded because FE cannot decode spatial PValues; MySQL metadata and ST_AsBinary use binary carriers.
- Performance: concrete binary consumers, in-place mutable filtering, bounded live arena retention, memoized NULL provenance and representative noding complexity controls were inspected. No benchmark or general worst-case complexity guarantee is claimed.
- Further issues/user concerns: all assigned changed-path categories and initial risks have dispositions. Previously reported OPEN/Touches/raw-identity residuals, dense-candidate concerns, generic complex runtime-filter/MAP-key limitations and deferred nullable-selection batching were not reposted. Deduplication does not imply those mechanisms are fully fixed. The new inline concerns the separate CLOSED equality consumer and unequal raw extents.
Review converged in 2 rounds: all latest-round reviewers returned NO_NEW_VALUABLE_FINDINGS against the unchanged frozen payload, with complete assigned static coverage and no unresolved new candidate. Convergence describes the bounded review process, not defect-free code.
Validation limits: static source, protocol-definition and arithmetic inspection only. No builds, Doris tests, benchmarks, services, style/hygiene execution, source changes or submodule initialization were performed. Runtime interoperability, crash behavior, final ELF ABI requirements and golden-generation provenance remain unexecuted validation limits.
Review converged after 2 round(s).
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69032 Problem Summary: ST_Equals considered LINESTRING(0 58,0 59) equal to LINESTRING(0 58.00000000000001,0 59), although POINT(0 58) belongs only to the first line. Distinct raw endpoints can become the same S2Point, and the rounded index drops the raw interval between them. Retain canonical raw atomic edges during the existing noding pass and compare them before accepting line equality. Keep the existing OPEN/Touches representation and extract only the original index-emission tail to satisfy the function-size gate. Add both-order witnesses, equivalent-coverage controls and a standard-generated SQL regression. ### Release note Fix typed Geography ST_Equals incorrectly accepting lines with different raw extents when their endpoints round to the same S2Point. ### Check List (For Author) - Test: Unit Test / Regression test - New test fails on the original implementation; final targeted ASAN BE geo/WKB UT: 134 passed. - GIS standard golden generation and non-generation verification passed. - Existing Iceberg spatial suite passed; all 162 prior golden sections unchanged, one added. - Standard ASAN BE+FE build, final post-framework rebuild and artifact identity checks passed. - Checkstyle, clang-format16, header hygiene and changed-line clang-tidy passed. - Private BE UT compatibility overlay used; no full community CI or benchmark claim. - Behavior changed: Yes. Unequal raw Geography line extents return false from ST_Equals. - Does this need documentation: No. The documented point-set equality contract is unchanged.
TPC-H: Total hot run time: 28629 ms |
TPC-DS: Total hot run time: 151206 ms |
ClickBench: Total hot run time: 24.29 s |
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69032 Problem Summary: Upstream master adds scan-task scheduling at connector API 24, which overlaps this branch's spatial SPI major and conflicts with its recorded surface. Preserve both interface sets, keep the unchanged metadata-method baseline, and advance the spatial contract and version pin to API 25. Retain upstream scheduling/labels and the existing spatial capability/metadata checks. The four already verified Geography equality fix files are unchanged by the merge. ### Release note Require connector API 25 for the spatial SPI combined with upstream scheduling. ### Check List (For Author) - Test: Unit Test / Regression test - Post-merge FE: 2061 executed, zero failures/errors; 14 pre-existing disabled or external/manual skips. - SPI own suite: all 152 executed and passed; post-merge BE geo/WKB/ORC seek UT: 150 passed. - Standard ASAN BE+FE build, Checkstyle, header hygiene and API25 artifact checks passed. - Post-merge GIS and Iceberg SQL verification passed; 162 previous golden sections unchanged. - Exact baseline union and unchanged spatial fix blobs verified; no full community CI claim. - Behavior changed: Yes. Preserve compatible contracts while rejecting mismatched API majors. - Does this need documentation: No. This merge does not expand the spatial user contract.
|
/review |
There was a problem hiding this comment.
Static review of PR #69032 at 6fec365. Two new P2 correctness findings are inlined: raw Geography coverage is lost in containment/overlap, and typed NULL UPDATE assignments fail before normalization. These should be corrected; no new P0/P1 was found.
Existing P1 comments 4269360799 (OPTIONAL spatial COUNT) and 4384986088 (FILE protocol IDs) were independently rechecked and are fixed at this head, so no existing blocking ID is retained. Existing discussions and replies were deduplicated, including the raw-equality variants. The known native nullable-selection batching limitation in 4367818661 remains a deferred performance item. No additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and evidence: Iceberg V3 parameterized spatial schema, raw WKB I/O, nested values and admitted Geography calculations are wired end to end. Added unit/regression sources cover the feature; the two inline cases remain gaps.
- Scope and reuse: the broad change is cohesive across connector/type/column/SerDe/planner/scanner/writer boundaries; shared validation and signature helpers reduce duplication.
- Concurrency and locks: no new worker thread or lock order was introduced. Failed writer cleanup closes the owned asynchronous writer before destroying it and deleting the object.
- Lifecycle and static initialization: column arenas, copied aggregate states, constants and file-writer ownership were traced. No new cross-translation-unit dynamic initialization dependency was identified.
- Configuration: no new configuration or session variable; nested spatial admission uses an explicit connector capability.
- Compatibility: binary payload/metadata identity and ordinary binary overlays were checked across both scanners. Connector API 25 and its surface baseline preserve the merged upstream scheduler and spatial additions. Existing mixed-version support decisions were treated as known context.
- Protocol identities: every touched definition was compared with current 3.1/4.0/4.1/4.2/master. Spatial enum IDs are unused there, release FILE IDs remain reserved, and new metadata fields are optional.
- Parallel paths: annotated/unannotated files, v1/v2, scalar/nested, nullable/constant, VALUES/SELECT/CTE and row-level DML were reviewed. UPDATE's missing typed-NULL normalization is inlined.
- Conditions and errors: unsupported type/CRS/algorithm/format/WKB cases fail explicitly. Rounded-index containment acceptance is the remaining verified Geography defect.
- Test coverage: inspected added positive/negative cases for endian variants, NULL parents/leaves, malformed selected values, state merging, analytic trimming, DDL restrictions and failed writer cleanup. No tests were executed.
- Result changes: reviewed the three changed regression suites and goldens, including ordinary VARBINARY controls and spatial rereads. Result generation provenance and runtime correctness were not independently validated.
- Observability: existing profile paths and row/column/type/file error context remain available; no new distributed service or separate metric requirement was identified.
- Persistence and failover: spatial parameters use existing serialized type metadata and connector schema operations. No new internal journal/rowset visibility path was introduced; failover was not exercised.
- Writes and atomicity: validation precedes file data writing and remote spatial DDL mutation; existing transaction/commit machinery is retained. Failure cleanup ownership was inspected; crash behavior was not executed.
- FE/BE transport: Thrift, PScalarType, PColumnMeta, nested descriptors and nullable/constant round trips preserve spatial parameters. Unsupported FE spatial constant decoding is excluded; internal storage/point-query paths are rejected by DDL admission.
- Performance: mutable filtering avoids copying retained WKB, analytic erase reclaims discarded arena payload, and noding merges repeated coverage. The existing nullable-selection limitation is deduplicated; no benchmark or general worst-case complexity guarantee is claimed.
- Core invariants and other issues: committed-data visibility, partition versions and MoW delete bitmaps are not changed. Owning column memory and failure propagation were inspected. The final independent sweep found no additional substantiated issue beyond the inline set.
Coverage: all 182 authoritative changed paths and relevant callers/consumers were assigned and inspected across FE/protocol, BE/scanner/writer and dedicated Geography reviews. This is source and arithmetic evidence only: no build, test, benchmark, runtime interoperability or artifact validation was performed. Review convergence describes completion of this bounded static process, not proof of bug-free numerical topology or performance.
Review converged after 2 round(s).
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69032 Problem Summary: Distinct Geography line endpoints can round to the same S2 point. Contains/Within then accept a shorter raw line as covering a longer one, and Overlaps loses partial or tiny positive overlaps. Reuse the canonical raw atomic edges from the existing shared noding pass for directional coverage and line overlap, while retaining S2 checks for degenerate positions. Row-level UPDATE projects typed NULL assignments before constant folding, so an INT-typed NULL slot is rejected when coerced to a spatial target. Reuse the existing proven-NULL normalizer with the merge sink's data-column ordinals before coercion. Preserve routing, hidden passthrough, ordinary casts, Variant lineage checks and non-NULL spatial identity validation. ### Release note Fix default Geography line containment and overlap at rounded endpoints, and accept proven typed NULL assignments to nullable Iceberg spatial columns during UPDATE. ### Check List (For Author) - Test: ASAN BE geo/WKB UT 136/136 and selected FE UT 45/45, with zero failures or skips; standard ASAN BE+FE build and Java Checkstyle; exact GIS and Iceberg suites generated and verified with zero failures/fatal/skips; 163 prior golden sections unchanged. Private BE UT compatibility overlay used. clang-format16, hygiene, changed-line clang-tidy and diff --check passed. - Behavior changed: Yes, correct existing spatial predicate and NULL-write semantics; preserve ordinary-key/spatial-value support. - Does this need documentation: No, this fixes existing contracts without adding a supported domain; documentation PR remains apache/doris-website#4239.
|
/review |
There was a problem hiding this comment.
Reviewed the complete 182-file aggregate PR at fcd19869858f3da8d05756f7854d7ada0c060f56. Three P2 correctness issues remain, all reported inline: pre-analysis MERGE typed-NULL coercion, polygon raw-boundary identity, and false line contact from colliding converted endpoints. No P0/P1 issue was independently confirmed still applicable.
Critical checkpoint conclusions:
- Goal and proof: spatial Iceberg V3 schema/read/write/transport and the restricted Geography calculation domain are wired end to end. Unit and SQL tests cover substantial positive/negative behavior; the three inline cases expose remaining gaps.
- Scope and reuse: the broad type-system change is feature-related; binary transfer, owning aggregate states, S2 and Arrow schema machinery are reused. No unrelated source modification was identified.
- Concurrency and locks: no new worker threads or locking protocol were introduced. Query-local normalization/noding state and the existing asynchronous file-writer close path were traced; no new lock-order defect was substantiated.
- Lifecycle and initialization: COW/arena ownership, aggregate-state copies, ring ownership, streams and failed-open teardown were inspected. No new ownership cycle or cross-translation-unit static initialization dependency was found.
- Configuration: no new dynamic configuration item was added.
- Compatibility: spatial kind/CRS/algorithm survive the reviewed descriptors and block metadata. Connector API 25.0 matches the new surface and pinned baseline. The stated requirement that participating FEs/BEs support spatial types remains; mixed-version execution was not validated.
- Thrift/protobuf IDs: every touched definition was compared with live-matching branch-3.1/4.0/4.1/4.2 snapshots. New metadata fields are optional and use free IDs; spatial enum values avoid branch-4.1 FILE identities. Existing P1
4384986088is repaired. - Parallel paths: FE INSERT VALUES/SELECT/CTE, UPDATE/MERGE, aggregate/window/set/state legality, BE const/nullable/CASE/state transport, v1/v2 readers and recursive Parquet writeback were reviewed. The distinct early MERGE synthesis gap is inline. Explicit BINARY overlays and unsupported-format/domain behavior were checked.
- Conditional checks: changed validation/cleanup branches have concrete input or lifecycle reasons. Mixed raw-coordinate and rounded topology acceptance still needs the two inline repairs.
- Test coverage: all changed BE/FE test hunks and all three GIS/Iceberg/VARBINARY SQL/golden pairs were inspected, including malformed input, NULL/EMPTY/dimensions, metadata mismatch, ownership, selected dictionaries, routing, cleanup and ordinary-key spatial values. Tests do not cover the three reported triggers through their actual consumers.
- Changed results: inspected golden bytes/counts/nulls/order against source and fixtures; no separate incorrect expected-result finding was substantiated. Test execution and golden-generation provenance were not independently validated.
- Observability: existing status/error context and reader/profile counters remain available. No additional required metric was identified. No throughput or general worst-case complexity claim is made.
- Persistence and failover: spatial parameters are carried in type metadata; existing schema identity, transaction/commit and replay mechanisms remain in use. No new EditLog/visible-version/delete-bitmap protocol was introduced; crash/failover behavior was not exercised.
- Writes and atomicity: metadata identity is checked before coercion in the reviewed write paths; referenced CRS writes reject before Parquet open, and rollback closes/destroys output ownership before final deletion. No further write/cleanup defect was substantiated statically.
- FE/BE variables: both type descriptors and protobuf column metadata carry spatial parameters; nested/nullable/constant paths were checked. Remote spatial constant folding remains excluded because FE cannot decode those PValues.
- Performance and memory: owning spatial columns, in-place filtering, arena reclamation, selected dictionary validation and merged noding coverage were traced. Existing P2
4367818661remains the known deferred nullable-selection batching limitation; it is not reposted. A generic ARRAY runtime-filter issue was dismissed as pre-existing. - Other concerns and user focus: no additional user focus was supplied. Existing P1
4269360799is repaired in both spatial schema converters; no historical P0/P1 is carried as blocking. Prior threads were used to exclude duplicates, including the repaired standalone UPDATE case.
Validation was static source inspection plus binary64 arithmetic for coordinate conversion. No builds, tests, services, benchmarks or interoperability runs were performed, and repository source was not modified. The review has no remaining unreviewed assigned path or unresolved candidate; execution and exhaustive numeric/performance validation remain outside this review's evidence.
Review converged after 2 round(s).
| if (sink.isWritesDataFiles()) { | ||
| // UPDATE projects CAST(NULL AS INT) before folding; restore a target-typed | ||
| // NULL before coercing the output slot, without retagging routing columns. | ||
| LogicalPlan source = validateAndNormalizeMergeOutput(sink.getCols(), (LogicalPlan) sink.child(), |
There was a problem hiding this comment.
[P2] Normalize typed NULLs before MERGE branch synthesis
For a nullable Iceberg V3 MOR table t(id INT, geom GEOMETRY), MERGE INTO t USING (SELECT 1 id) s ON t.id=s.id WHEN MATCHED THEN UPDATE SET geom=CAST(NULL AS INT) still fails with an INT-to-GEOMETRY cast error instead of writing NULL. During command synthesis, the reduced intended tree is:
MergeSink[geom:GEOMETRY]
Project[operation, row_id, id,
IF(branch=0, target_cast(CAST(NULL AS INT)), NULL:GEOMETRY)]
Join[t, s]
ExternalRowLevelMergePlanBuilder.generateFinalProjections calls castUnbound to create target_cast; the original parser Cast already reports INT, so checkCanCastTo throws before that tree or its sink is built. RowLevelDmlCommand synthesizes before planner.plan, making this new normalization call unreachable for the failing input. Geography and not-matched INSERT branches share the path.
Normalize proven-NULL MERGE branch values before their eager target casts, preserving non-NULL identity checks, and cover actual matched/not-matched MERGE commands. This is the separate pre-binding MERGE path, distinct from existing 4387635824's repaired standalone UPDATE sink coercion. Evidence is static, not an executed SQL test.
| options.set_polyline_model(S2BooleanOperation::PolylineModel::CLOSED); | ||
| options.set_polyline_loops_have_boundaries(false); | ||
| if (name == "st_equals") { | ||
| *result = left_edges == right_edges && S2BooleanOperation::Equals(left, right, options); |
There was a problem hiding this comment.
[P2] Preserve raw polygon boundaries before accepting equality
For default GEOGRAPHY imported as WKB, let A=POLYGON((0 58,1 59,-1 59,0 58)) and B=POLYGON((0 58.00000000000001,1 59,-1 59,0 58.00000000000001)). A's southern apex is outside B, but the pinned degree conversion maps both apex latitudes to 0x1.03258031ade7cp+0, giving identical S2 vertices.
These short, valid triangle edges receive no synthetic midpoint. The other raw apex has an endpoint longitude on each sloping edge and is excluded by append_edge's strict longitude interval; it is not on the top edge. Both polygon indexes therefore remain identical. raw_line_coverage is false for polygons, so both edge sets here are empty and ST_Equals(A,B) returns true. The parallel rounded coverage test also accepts ST_Contains(B,A) / ST_Within(A,B) despite the excluded apex.
Retain canonical raw polygon boundary/coverage identity for these decisions, preserving ring roles and equivalent subdivisions; add unequal colliding-apex and inward-containment controls. This is a polygon consumer distinct from existing 4386509606/4387635816's line-only fixes. Evidence is source and binary64 arithmetic, not a Doris runtime test.
| // A nearby disjoint curve must not become a boundary contact through the legacy | ||
| // conversion-distance fallback used below by Intersects/Disjoint. | ||
| if (name == "st_touches") { | ||
| *result = intersects && !S2BooleanOperation::Intersects(left, right, interior_options); |
There was a problem hiding this comment.
[P2] Verify raw contact before accepting rounded line endpoints
For default GEOGRAPHY WKB, A=LINESTRING(0 57,0 58) and B=LINESTRING(0 58.00000000000001,0 59) have disjoint raw extents, so ST_Touches(A,B) must be false in either order. The pinned degree conversion maps their adjacent endpoints to the same radians 0x1.03258031ade7cp+0, hence the same S2Point.
Shared meridian noding emits each separate interval; neither covers the raw gap. This path does not collect raw coverage, so its rounded indexes share a CLOSED endpoint while their OPEN interiors are disjoint, exactly the S2 boundary-model distinction. The expression here therefore returns true before the distance fallback.
Confirm genuine raw contact before accepting the rounded endpoint intersection, retaining true endpoint/subdivision/alias controls and adding both-order raw-gap negatives. This is a false contact on unequal disjoint inputs, distinct from existing 4381111390's equivalent-subdivision/AA-marker false negative. Evidence is source and binary64 arithmetic, not a Doris runtime test.
### What problem does this PR solve? Issue Number: N/A Related PR: apache#69032 Problem Summary: MERGE synthesizes eager target casts before analysis, so CAST(NULL AS INT) fails when assigned to a spatial target. Retag only cast chains rooted at a literal NULL before the existing eager cast. Preserve actual values, routing, branch selection and write identity checks. Distinct raw Polygon apexes can become the same S2 point and cause false equality or containment. Collect canonical raw boundary edges with ring-depth parity during existing noding. Distinguish equality by those boundaries and explicitly reject containment/within/overlap when different raw boundaries collapse to one S2 region. Preserve normal containment and equivalent subdivisions without adding a raw region solver. Rounded line endpoints across a real raw gap can also produce a false Touches result. Require a genuine raw open endpoint contact for line-line candidates, reusing the existing membership predicate. ### Release note Accept literal-NULL cast chains in spatial MERGE branches. Preserve raw Polygon identity and reject S2-ambiguous polygon containment/overlap explicitly. Reject false line touches across rounded endpoint gaps. ### Check List (For Author) - Test: ASAN BE geo/WKB UT 140/140 and selected FE UT 51/51, zero failures or skips; standard ASAN BE+FE build and Checkstyle; exact GIS and Iceberg suites generated and verified with zero failures/fatal/skips, retaining all 165 prior golden sections and adding three. Private BE UT compatibility overlay used. clang-format16, hygiene, changed-line clang-tidy and diff --check passed. - Behavior changed: Yes, fix MERGE NULLs and relation correctness with a documented finite-precision rejection; preserve ordinary-key/spatial-value and nested support. - Does this need documentation: Yes, apache/doris-website#4239.
What problem does this PR solve?
Issue Number: N/A
Related PR: #67907
Problem Summary: Support Iceberg V3 GEOMETRY/GEOGRAPHY external tables on master. Preserve spatial kind, CRS, Geography edge algorithm, NULL/EMPTY and original WKB through schema mapping, execution and Parquet writeback. Check write identity before coercion or remote mutation.
Summary
ST_AsBinaryreturns the original bytes as VARBINARY;ST_AsTextretains dimension markers and ordinates. Calculations reject non-XY inputs. Typed PostGIS EWKB flags/SRID remain unsupported; historical STRING constructors retain their behavior.projjson:CRS writes before file creation. Preserve nondefault metadata without automatic CRS conversion. CREATE/ALTER/CTAS use an explicit Iceberg capability for nested spatial leaves; the spatial metadata and nested-spatial capability surface uses connector API 25.0, distinct from upstream scheduler API 24.0.GEOGRAPHY(OGC:CRS84, spherical)and nonempty XY Point/LineString/Polygon. Complete Contains/Within and Touches combinations in that domain; addST_Equals(topological point-set equality),ST_Overlaps(same-dimension proper overlap) andST_DWithin(a,b,d)(inclusive finite nonnegative metre threshold). Preserve raw WKB and legacy STRING behavior; no Geometry planar, Multi*/EMPTY/Z/M calculations, spatial SQL key semantics or indexes are added. Domain-ineligible inputs fail explicitly and NULL propagates.ST_AsBinary/ST_AsTextprojections. Direct spatial Flight schemas and generic file-TVF spatial inference remain unsupported. Reject constant-list spatial IN/NOT IN; suppress unsupported spatial runtime filters and exclude spatial columns from statistics collection.Paimon/Lance/GeoArrow, Doris internal-table storage, spatial indexes, bounds pruning, predicate pushdown, CRS conversion, additional Geography algorithms, typed WKT input and higher-dimensional calculations are outside scope.
Evidence
Raw line coverage and typed NULL UPDATE
fcd19869858: Contains/Within and line Overlaps now reuse canonical raw atomic edges from the existing noding pass, preserving directional coverage and positive overlaps that round to one S2 point. Row-level sink coercion reuses proven-NULL normalization by data-column position; routing, hidden passthrough, ordinary casts, Variant lineage and non-NULL spatial identity checks remain. Failure cases were reproduced on the old implementation. Targeted ASAN geo/WKB UT 136/136 and FE UT 45/45 passed with zero failures/skips; standard ASAN BE+FE build, Checkstyle, clang-format16, hygiene and changed-line clang-tidy passed. Exact GIS and Iceberg suites passed standard generation and non-generation verification; all 163 previous golden sections are unchanged, with two added. Ordinary-key/spatial-value and static nested controls remain covered. Source, artifacts and actual task-owned processes were matched; private BE UT compatibility overlay used. No full community CI pass is claimed.Raw line equality
904315704c1, merged in6fec365cdbc: Before,LINESTRING(0 58,0 59)compared equal toLINESTRING(0 58.00000000000001,0 59)althoughPOINT(0 58)belongs only to the first line. Line equality now compares canonical raw atomic edges collected during the existing noding pass before accepting S2 equality; no tolerance or second noding pass is added. The new test fails on the old implementation. Post-merge ASAN geo/WKB/ORC seek UT passed 150/150. GIS golden generation and verification, plus post-merge GIS and complete targeted Iceberg SQL verification, passed with zero failures/fatal/skips: all 162 previous golden sections are unchanged and one is added. Both-order witnesses, equivalent subdivision/reversal/backtracking and date-line/pole aliases are covered; ordinary-key/spatial-value support remains.Upstream sync
6fec365cdbc: Official masterba3a660f213is included. Resolve only the SPI surface/version conflicts: retain upstream scheduler API 24 and the spatial surface at API 25, with the exact union of both plugin baselines and an unchanged metadata baseline. Post-merge FE validation executed 2061 tests without failures/errors, including all 152 SPI own tests; 14 unchanged known-issue or external/manual/live tests remain skipped and are not counted as passing. Standard ASAN BE+FE build, Checkstyle, format16, hygiene, changed-line C++ checks and source/artifact/runtime identity checks passed. All ten current connector entrypoint jars match build targets and advertise API 25. Private BE UT overlay used; new full community CI, coverage and benchmark results remain pending.Protocol/API/GCC compatibility
f02598ba88e: Reserve branch-4.1 FILE protobuf 45 / Thrift 47; spatial IDs are now 46/47 and 48/49. Advance the spatial SPI to API 24.0 so mismatched plugins are rejected before provider construction. Limit the GCC15 workaround to the unused Boost quadmath wrappers around cpp_int; the actual GCC15.2 include probe fails before and passes after the fix, with integer calculation code unchanged. Official masterba3d62155ccis included via75663629ef6, preserving both sets of window tests. Targeted ASAN BE UT 189/189 passed; after a test-only explanatory comment, the final type-test rerun passed 18/18. FE SPI/API gate/wiring UT 194/194 passed, including 152 SPI own tests. Standard FE build, Checkstyle, clang-format16, hygiene and changed-line clang-tidy passed; all ten current connector entrypoint jars match their build targets at API 24.0. Private BE UT compatibility overlay used. These targeted checks do not certify benchmarks or the new full community CI run.Earlier rounded-zero Geography edge P2
9307cec2c8f: A valid meridian subdivision can round an adjacent raw edge to the same S2Point. The raw edge remains available for identity, but the zero-length S2 edge is skipped so it cannot create an OPEN/AA boundary marker and changeST_Touches. Added original/subdivided/reversed operand coverage; focused BE Geography tests pass. The official upstream/master sync is included in merge commit5c39eb7aca4; GitHub reports mergeable=true.Line admission and independent NULL outputs
e3b3d0022c9: Before, the reported valid near-antipodal Geography line failed S2 admission, and repeated NULL output slots changed an ordinary INT-to-BIGINT cast input to Geometry during project merging; both cases fail in pre-fix tests. Typed lines now share stable raw-arc admission with polygon rings, while each retagged sink output receives a distinct Alias identity. Raw WKB, legacy STRING and ordinary-key/spatial-value support are preserved. Final ASAN Geo/WKB UT 131/131, FE normalizer/validator/INSERT UT 44/44, standard BE+FE ASAN build, Checkstyle, clang-format16, hygiene and changed-line clang-tidy passed. Exact GIS and Iceberg suites passed standard generation and verification with zero failures/fatal/skips: all 160 previous sections are unchanged, two are added. Official upstream/master961fe301f7bwas already included; no new merge conflict or support domain was introduced. Private UT compatibility overlay used; new community CI results remain pending.Raw vertex identity and polygon admission
7d6fb854546: Preserve canonical original XY and WKB sequence/ring identity even when distinct coordinates round to one S2Point. Validate typed polygon rings using the existing stable raw arcs before hole containment, retaining input ring depths after S2 reordering and keeping raw accessors/legacy STRING unchanged. The two reported cases fail before the fix. Final targeted ASAN Geo/WKB UT 129/129, upstream HTTP TVF FE UT 5/5, standard BE+FE ASAN build and final BE rebuild, Checkstyle, clang-format16, hygiene and changed-line clang-tidy passed. GIS generation/verification and the targeted Iceberg suite passed with zero failures/fatal/skips; all 158 prior sections are unchanged and two GIS sections are added. Official upstream/master961fe301f7bmerged automatically inc0eedd7; no manual conflicts. Ordinary-key/spatial-value support and the documented supported domain remain unchanged. Private UT compatibility overlay used; no full community CI pass is claimed.Latest area/bearing/NULL repair
2a42a52f1dband upstream merge21ab2f95f33: Include official master8d2bd1e14b6, preserving upstream split offsets and VARBINARY CASE. Typed area now measures the stable raw-coordinate rings used by predicates; original-degree half-angle bearings avoid near-antipodal cancellation. Spatial INSERT SELECT recovers proven literal NULLs through safe projections/CTEs and nested constructors before coercion, retaining non-NULL identity checks and ordinary-key spatial values. Arbitrary shared expressions are not expanded. Both numeric regressions failed before the fix. Merge-specific BE UT 30/30, Geo/WKB ASAN UT 124/124, focused FE UT 43/43, standard BE/FE ASAN build, Checkstyle, format16, hygiene and changed-line clang-tidy passed. GIS and Iceberg generation plus non-generation verification passed with zero failures/fatal/skips: 153 existing sections unchanged, four new sections; one existing ST_Angle value changed by 9e-16 radians and is closer to an independent high-precision reference. One negative CAST assertion now expects the earlier CRS mismatch. Private UT compatibility overlay used; no expanded support domain or full community CI pass is claimed.Polygon-interior repair
536bf508fb2: Raw and midpoint-subdivided near-antipodal rings could disagree on interior predicates. Existing point/nonpoint predicates now share stable arcs with smaller-ring normalization and preserved hole/boundary roles. Final Geo/WKB ASAN UT 122/122, standard BE build, clang-format16, hygiene and changed-line clang-tidy passed; FE artifacts are unchanged. GIS generation/verification and targeted Iceberg verification passed: all 153 previous golden sections unchanged, one GIS section added. Tests include reversed rings, holes, thin triangles, pole aliases and synthetic midpoint crossings. No new support domain or full community CI claim.Raw-arc repair
350d7c51986: Before, a near-antipodal edge returned about 237 km for an off-plane point within 0.112 m of its raw arc; 32/64/128 distinct uncertain circles required 1024/4096/16384 exact proofs. Stable raw-coordinate normals and distance-only midpoints now preserve subdivision-independent distance, while dense/sparse longitude indexes avoid rescanning proven point clusters. Final ASAN Geo/WKB UT 118/118, standard BE+FE build and final BE rebuild, Checkstyle, clang-format16, hygiene and changed-line clang-tidy passed. GIS standard generation and verification passed: all 48 previous sections unchanged, one added. The full targeted Iceberg suite passed with all 104 sections unchanged, retaining ordinary-key/spatial-value coverage. Candidate-visit checks cover 32/64/128 spokes both alone (160/320/640 visits) and with ordinary-point padding (329/649/1289); this is not a general worst-case complexity or throughput claim. Private UT compatibility overlay used; no full community CI pass is claimed.Upstream merge
af66f59ab50: Official master89f2ed38996is included. Preserve upstream metadata-only DDL and nested spatial capability at connector API 23.0. Post-merge selected FE/SPI/Iceberg tests passed 564/564; standard FE build and Checkstyle passed. That FE evidence is bound to the merge commit. Current BE/integration evidence is recorded above.Function completion
d6e0d4e9146: Previously, nonpoint Contains/Within and Touches combinations were rejected and Equals/Overlaps/DWithin were absent. The admitted Geography domain now implements those predicates. Targeted ASAN BE geo/WKB 107/107 and FE constructor/aggregate-legality 38/38 passed, as did standard ASAN BE+FE build, Checkstyle, clang-format16, hygiene and changed-line clang-tidy. Exact GIS and Iceberg suites passed generation and non-generation verification with zero failures/fatal/skips; all 137 prior golden sections are unchanged and 11 were added. Ordinary-key/spatial-value and static-container paths remain covered.Earlier upstream merge
329498a9c2d: Merge official master7a5d64e85af; only the connector version-test comment conflicted. Keep upstream credential APIs and advance the spatial API change to 22.0. Post-merge targeted FE/SPI validation passed 262 tests, including all 149 SPI own tests. Function production bytes are unchanged by the merge. Product build and integration evidence above is bound tod6e0d4e9146; no full post-merge BE deployment or full community CI pass is claimed.Earlier targeted fix
59fd5424679: Before, ARRAY spatial ordering keys passed analysis and reached raw-WKB comparison. After, MIN_BY/MAX_BY reject those keys before execution while ordinary-key nested result values retain their original bytes, including state merging. FE legality tests (25), standard FE build/Checkstyle and exact Iceberg generation plus non-generation verification passed; all 97 previous golden sections are unchanged, with five new positive sections. BE source and its full artifact inventory are unchanged.Before: Nested spatial writes were rejected, and unsupported spatial window partition/COUNT_AT inputs reached raw-byte comparisons or missing BE factories. Actual INSERT VALUES constructors retained a typed NULL leaf's original CRS before target casting.
After (nested implementation
124f9807b6b): Targeted FE UT 319/319, including the SPI own suite 149/149; the final real-constructor normalization paths and identity controls were rechecked with 32/32 FE tests. Source-bound local ASAN BE UT 113/113 passed. Standard BE/FE builds, Java Checkstyle, clang-format16, BE hygiene and changed-line clang-tidy passed.The exact
test_iceberg_spatial_v3suite passed standard generation and non-generation verification: 1 suite, zero failures/skips each. All 80 previous golden sections are unchanged; nine new sections cover nested DDL/CTAS/writeback, typed NULLs, ordinary-key spatial window values and ordinary Variant neighbors. Both-kind partition/COUNT_AT/Variant negative paths are exercised on actual Iceberg columns.Spark 4.1.0 / Iceberg 1.12.0 and PyArrow 26.0.0 independently read three Doris-written files with seven rows and eight nested spatial columns. Original little-/big-endian WKB, XYZM, EMPTY, NULL parents/leaves and empty containers match. Spark-native annotated nested files also read through Doris. Bilingual documentation SQL examples execute successfully.
Local regression used only task-owned services, with an 8-core/32GB ASAN BE after retaining a failed run that hit its earlier 16GB process limit. These targeted results do not certify full community CI, coverage or performance; community builds are requested separately.
Earlier upstream merge
4d3e58a717f: Resolved only the connector version/pin conflict while preserving upstream metadata-delete APIs and nested spatial capability at API 21.0. Targeted merge validation passed 335 FE/SPI tests, including all 149 SPI tests, and BE build hygiene. The full community CI request targets this merged head; the product build/regression evidence above remains bound to124f9807b6b.Merge Danger
Door: Two-way for code; written Iceberg V3 spatial data requires compatible readers.
Blast Radius: Types.
The change spans external schema mapping, shared type transport and execution dispatch. Internal storage formats, execution-version bumps and scheduler rewrites are excluded. All participating FEs/BEs must support spatial types.
Release note
Support Iceberg V3 GEOMETRY/GEOGRAPHY schema mapping and raw ISO XY/XYZ/XYM/XYZM WKB Parquet read/write in top-level columns and ARRAY/MAP values/STRUCT fields, preserving type parameters with explicit XY calculation limits. Native spatial Variant values remain unsupported. Complete the admitted Geography Contains/Within/Touches combinations and add typed ST_Equals, ST_Overlaps and ST_DWithin.
Check List (For Author)
Check List (For Reviewer who merge this PR)