diff --git a/.gitignore b/.gitignore index 074926e..d33570d 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,6 @@ duckdb_unittest_tempdir/ testext test/python/__pycache__/ .Rhistory + +# Wasm toolchain installed by `just wasm-setup` (emsdk + vcpkg). +.vendor diff --git a/CMakeLists.txt b/CMakeLists.txt index 09f3479..5b2cd42 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,7 +23,6 @@ set(EXTENSION_SOURCES src/functions/geom_accessors.cpp src/functions/distance.cpp src/functions/transforms.cpp - src/functions/arrow_native.cpp src/functions/struct_metadata.cpp src/kernel/solid_model.cpp src/kernel/payload.cpp @@ -42,8 +41,7 @@ set(EXTENSION_SOURCES src/kernel/geom_construct.cpp src/kernel/geom_analysis.cpp src/kernel/geom_serialize.cpp - src/kernel/crs_transform.cpp - src/kernel/arrow_native_import.cpp) + src/kernel/crs_transform.cpp) # PROJ backs ST_3DTransform (kernel/crs_transform.cpp). Under the vcpkg toolchain # the vcpkg copy is found first; on a bare macOS dev box fall back to Homebrew. diff --git a/README.md b/README.md index fb321ca..22417da 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,7 @@ Full build, test, and distribution notes: [docs/README.md](docs/README.md). | --- | --- | | [docs/FUNCTIONS.md](docs/FUNCTIONS.md) | **Function reference** — every function, with signatures and runnable examples | | [docs/EXAMPLE.md](docs/EXAMPLE.md) | Hands-on walkthrough against real 3DBAG data | -| [docs/TESTING.md](docs/TESTING.md) | Manual notebook: every public function run against real CityJSON, CityJSONSeq, CityParquet and arrow-native data | +| [docs/TESTING.md](docs/TESTING.md) | Manual notebook: every public function run against real CityJSON, CityJSONSeq and CityParquet data | | [docs/DESIGN_DOC.md](docs/DESIGN_DOC.md) | Architecture & design philosophy: type model, layering, invariants | | [docs/CITYJSON_INTEROP.md](docs/CITYJSON_INTEROP.md) | Composing with the `cityjson` extension; running the interop tests | | [docs/FUTURE_WORK.md](docs/FUTURE_WORK.md) | Deferred design decisions | diff --git a/docs/DESIGN_DOC.md b/docs/DESIGN_DOC.md index 8ac5e92..f597077 100644 --- a/docs/DESIGN_DOC.md +++ b/docs/DESIGN_DOC.md @@ -259,11 +259,6 @@ cavities. The sidecar's `shells` key restores that grouping, which in turn enabl This keeps `duckdb-3d` ignorant of CityJSON files, LoD selection, and semantic surfaces — all of which stay upstream. -An **experimental** arrow-native ingestion path (`ST_3DFromArrowNative` and siblings) reads -nested `LIST`/`STRUCT` boundary columns directly, skipping WKB serialization entirely while -producing the identical payload. It is part of a cross-repo experiment with `cityparquet-rs` -and `duckdb-cityjson` and is **not** part of the settled v1 surface. - --- ## 8. Validation & measurement semantics @@ -457,6 +452,3 @@ surface; PROJ-backed `ST_3DTransform`. See the booleans (union / difference / intersection), true 3D convex hulls, tessellation, straight skeletons, medial axes, and topology-repair workflows. These are gated on whether to take on a CGAL or SFCGAL dependency — a decision deliberately not yet made, per §2.4. - -**Experimental.** Arrow-native ingestion (§7), pending the outcome of the cross-repo -experiment with `cityparquet-rs` and `duckdb-cityjson`. diff --git a/docs/FUNCTIONS.md b/docs/FUNCTIONS.md index 8d501d4..c8b4805 100644 --- a/docs/FUNCTIONS.md +++ b/docs/FUNCTIONS.md @@ -121,15 +121,11 @@ Convert between them via WKB: `ST_Geom3DFromWKB(ST_3DAsWKB(solid))` goes solid ## Conventions -**Null propagation.** Any `NULL` argument yields `NULL`, with two deliberate exceptions: +**Null propagation.** Any `NULL` argument yields `NULL`, with one deliberate exception: +`ST_3DFromWKB(wkb, NULL)` builds the solid *without* metadata and returns a non-`NULL` +result — a missing sidecar is not an error. -- `ST_3DFromWKB(wkb, NULL)` builds the solid *without* metadata and returns a non-`NULL` - result — a missing sidecar is not an error. -- The arrow-native constructors return `NULL` if *any* argument is `NULL`, because - `geometry_properties.type` is load-bearing there. - -**`TRY` variants** (`ST_3DTryFromWKB`, `ST_3DTryFromArrowNative`, `ST_Geom3DTryFromArrowNative`) -catch **row-level** errors and return `NULL` instead. Bind-time errors — a malformed metadata +**`TRY` variants** (`ST_3DTryFromWKB`) catch **row-level** errors and return `NULL` instead. Bind-time errors — a malformed metadata STRUCT, a wrong argument type — still raise. There is **no** `ST_Geom3DTryFromWKB`. **Errors, not repair.** The extension never silently fixes geometry. `ST_3DVolume` on a @@ -160,13 +156,51 @@ stay un-prefixed. ## Import / construction +The single-argument constructors take either a `BLOB` of WKB or DuckDB's native +`GEOMETRY`, so a GeoParquet column that arrives carrying the Parquet `GEOMETRY` +logical type needs no `ST_AsWKB` in between — see +[WKB or GEOMETRY](#wkb-or-geometry) below. + | Function | Signature | Returns | | --- | --- | --- | -| `ST_3DFromWKB` | `(wkb BLOB)` | `SOLID_3D` | +| `ST_3DFromWKB` | `(wkb BLOB \| GEOMETRY)` | `SOLID_3D` | | `ST_3DFromWKB` | `(wkb BLOB, geometry_properties VARCHAR)` | `SOLID_3D` | | `ST_3DFromWKB` | `(wkb BLOB, geometry_properties STRUCT)` | `SOLID_3D` | | `ST_3DTryFromWKB` | same three overloads | `SOLID_3D` or `NULL` | -| `ST_Geom3DFromWKB` | `(wkb BLOB)` | `GEOM_3D` | +| `ST_Geom3DFromWKB` | `(wkb BLOB \| GEOMETRY)` | `GEOM_3D` | + +### WKB or GEOMETRY + +A CityParquet package annotates its GeoParquet-legal geometry columns with the Parquet +`GEOMETRY` logical type, and DuckDB promotes such a column to its native `GEOMETRY` on +read. `geometry_lod0_0::BLOB` is not a way back — the cast is unimplemented — so the +single-argument constructors accept `GEOMETRY` directly: + +```sql +SELECT count(*) AS n, + ROUND(max(abs(ST_3DFootprintArea(ST_Geom3DFromWKB(geometry_lod0_0)) + - ST_3DFootprintArea(ST_Geom3DFromWKB(ST_AsWKB(geometry_lod0_0))))), 12) AS max_abs_diff +FROM read_parquet('building.parquet') WHERE geometry_lod0_0 IS NOT NULL; +``` +``` +┌───────┬──────────────┐ +│ n │ max_abs_diff │ +├───────┼──────────────┤ +│ 1115 │ 0.0 │ +└───────┴──────────────┘ +``` + +Two consequences worth knowing: + +- **Solid columns are unaffected**, because they never carry the annotation and could not + survive it: DuckDB's geometry model has no polyhedral surface, and `ST_GeomFromWKB` + raises `Unsupported geometry type in WKB` on solid bytes. A solid therefore cannot reach + these functions as `GEOMETRY` at all; on `ST_3DFromWKB` / `ST_3DTryFromWKB` the + `GEOMETRY` form matters for foreign GeoParquet columns, where `MultiPolygon Z` is common + and the `TRY` form's `NULL` is the useful answer. +- The argument is bound through a single `ANY` candidate rather than one overload per + type, so an untyped `ST_3DFromWKB(NULL)` still binds. Anything that is neither + `GEOMETRY` nor implicitly castable to `BLOB` is rejected at bind time as before. ### `ST_3DFromWKB` / `ST_3DTryFromWKB` @@ -210,32 +244,6 @@ SELECT ST_3DGeometryType(ST_Geom3DFromWKB(geometry)) AS gtype FROM ex; └──────────────────────┘ ``` -### Arrow-native constructors (experimental) - -`ST_3DFromArrowNative`, `ST_3DTryFromArrowNative`, `ST_Geom3DFromArrowNative`, -`ST_Geom3DTryFromArrowNative` ingest nested `LIST`/`STRUCT` boundary columns plus a vertex -pool **directly, bypassing WKB** — while producing exactly the same payload. - -``` -ST_3DFromArrowNative(boundaries, vertices, geometry_properties) → SOLID_3D -``` - -- `boundaries` — `INTEGER[][][][][]` (solid → shell → face → ring → vertex index) -- `vertices` — `STRUCT(x DOUBLE, y DOUBLE, z DOUBLE)[]` -- `geometry_properties` — JSON `VARCHAR` **or** the CityParquet `STRUCT`; **required** - -`geometry_properties.type` is load-bearing and dispatches solid-family -(`Solid`/`MultiSolid`/`CompositeSolid`, the `ST_3D*` pair) versus surface-family -(`MultiSurface`/`CompositeSurface`, the `ST_Geom3D*` pair). A single-shell `Solid` and a -padded `MultiSurface` are physically indistinguishable, so the family cannot be inferred from -shape — passing the wrong family raises (or yields `NULL` in the `TRY` form). - -> **Experimental.** These are part of an in-progress cross-repo experiment with -> `cityparquet-rs` and `duckdb-cityjson`, not part of the settled v1 surface. The producing -> readers are not yet released upstream. - ---- - ## Export / serialization | Function | Signature | Returns | Notes | @@ -702,7 +710,7 @@ FROM ex; | Category | Functions | | --- | --- | -| **Import** | `ST_3DFromWKB`, `ST_3DTryFromWKB`, `ST_Geom3DFromWKB`, `ST_3DFromArrowNative`*, `ST_3DTryFromArrowNative`*, `ST_Geom3DFromArrowNative`*, `ST_Geom3DTryFromArrowNative`* | +| **Import** | `ST_3DFromWKB`, `ST_3DTryFromWKB`, `ST_Geom3DFromWKB` | | **Export** | `ST_3DAsWKB`, `ST_3DAsText`, `ST_3DAsGeoJSON`, `ST_3DAsBinary` | | **Introspection** | `ST_3DBounds`, `ST_3DNumSolids`, `ST_3DNumShells`, `ST_3DNumFaces`, `ST_3DZMin`, `ST_3DZMax`, `ST_NDims`, `ST_3DHasZ`, `ST_CoordDim`, `ST_3DGeometryType`, `ST_3DDimension`, `ST_3DNumGeometries`, `ST_3DX`, `ST_3DY`, `ST_3DZ`, `ST_IsPlanar` | | **Validation** | `ST_3DIsClosed`, `ST_3DIsManifold`, `ST_3DIsOriented`, `ST_3DValidationReport` | @@ -710,8 +718,6 @@ FROM ex; | **Distance** | `ST_3DDistance`, `ST_3DMaxDistance`, `ST_3DDWithin`, `ST_3DDFullyWithin`, `ST_3DIntersects`, `ST_3DClosestPoint`, `ST_3DShortestLine` | | **Transform / construct** | `ST_3DTranslate`, `ST_3DScale`, `ST_3DRotateX`, `ST_3DRotateY`, `ST_3DRotateZ`, `ST_3DTransform`, `ST_3DExtrude`, `ST_MakeSolid`, `ST_3DCentroid`, `ST_3DConvexHull`, `ST_Force3D` | -`*` experimental (arrow-native ingestion). - **Not implemented.** These PostGIS names appear in comparison tables but are **not** registered: `ST_3DIsValid` (validity is a field of `ST_3DValidationReport`), `ST_3DUnion` / `ST_3DIntersection` / `ST_3DDifference`, `ST_3DLongestLine`, `ST_3DExtent`, `ST_Affine`, diff --git a/docs/TESTING.md b/docs/TESTING.md index 42d4f3d..cc93117 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -1,9 +1,8 @@ # Manual testing notebook — every public function against real data -A copy-pasteable SQL walkthrough that exercises **all 49 public `duckdb-3d` functions** +A copy-pasteable SQL walkthrough that exercises **all 45 public `duckdb-3d` functions** against real 3D city models: local CityJSON, remote CityJSONSeq (3DBAG Delft and -Helsinki), an on-disk **CityParquet** package, and the **arrow-native** geometry -encoding. Every cell below was executed and its printed output is the real output, +Helsinki) and an on-disk **CityParquet** package. Every cell below was executed and its printed output is the real output, not an illustration. This complements the other docs rather than repeating them: @@ -25,7 +24,8 @@ this walkthrough found and fixed. Two extensions are needed: `three_d` (this repo) and `cityjson` (the sibling [`duckdb-cityjson`](https://github.com/cityjson/duckdb-cityjson) repo) for the readers. -`spatial` and `json` are used by a handful of cells and autoload. +`spatial` and `json` are used by a handful of cells. The extension build ships with +`autoload_known_extensions` off, so `LOAD` them explicitly where a cell needs them. ```sh GEN=ninja make release # builds ./build/release/duckdb with three_d linked in @@ -749,7 +749,7 @@ SELECT * FROM cityparquet_write('pkg', '/tmp/cp_test/pkg_out', crs => 'EPSG:7415 ┌────────────┬────────┐ ┌──────────────────┬─────────┬──────┬─────────┐ │ table_name │ role │ │ file │ action │ rows │ bytes │ ├────────────┼────────┤ ├──────────────────┼─────────┼──────┼─────────┤ -│ building │ object │ │ building.parquet │ written │ 2231 │ 4235239 │ +│ building │ object │ │ building.parquet │ written │ 2231 │ 3690150 │ └────────────┴────────┘ │ metadata.json │ written │ 0 │ 6722 │ └──────────────────┴─────────┴──────┴─────────┘ ``` @@ -790,6 +790,7 @@ shells = [[6]] ``` ```sql +LOAD json; CREATE TABLE cp AS SELECT id, ST_3DTryFromWKB(geometry_lod2_2, geometry_properties_lod2_2) AS s_struct, @@ -867,37 +868,66 @@ FROM parts p JOIN cp c USING (id); **Every one of 1116 solids is byte-identical** whether it arrives via streamed CityJSONSeq or via a Parquet file on disk. That is the CityParquet round-trip guarantee. -## 22 — LoD0 is GeoParquet, and needs a bridge +## 22 — LoD0 is GeoParquet, and reads as GEOMETRY -CityParquet's LoD0 footprint column is written as a **Parquet-native `GEOMETRY`** with an -embedded PROJJSON CRS; the solid LoDs stay `BLOB` because solids are outside GeoParquet's -WKB vocabulary. That asymmetry is visible on read-back: +CityParquet's LoD0 footprint column carries the **Parquet-native `GEOMETRY`** logical +type; the solid LoDs stay `BLOB`. A writer annotates a column exactly when it declares it +in the `geo` footer, and a `PolyhedralSurface Z` is not legal GeoParquet — but the sharper +reason is that DuckDB promotes any annotated column and converts it eagerly, and its +geometry model has no polyhedral surface. Annotating a solid column would therefore make +even `SELECT count(*)` over it fail, before any `ST_3D*` function saw a value. That +asymmetry is visible on read-back: ```sql -SELECT column_name, CASE WHEN column_type = 'BLOB' THEN 'BLOB' ELSE 'GEOMETRY()' END AS as_read +LOAD spatial; +SELECT column_name, column_type FROM (DESCRIBE SELECT * FROM read_parquet('/tmp/cp_test/pkg_out/building.parquet')) WHERE column_name LIKE 'geometry_lod%' ORDER BY 1; ``` ``` -┌─────────────────┬──────────────────────┐ -│ column_name │ as_read │ -├─────────────────┼──────────────────────┤ -│ geometry_lod0_0 │ GEOMETRY() │ -│ geometry_lod1_2 │ BLOB │ -│ geometry_lod1_3 │ BLOB │ -│ geometry_lod2_2 │ BLOB │ -└─────────────────┴──────────────────────┘ +┌─────────────────┬───────────────────────┐ +│ column_name │ column_type │ +├─────────────────┼───────────────────────┤ +│ geometry_lod0_0 │ GEOMETRY('EPSG:7415') │ +│ geometry_lod1_2 │ BLOB │ +│ geometry_lod1_3 │ BLOB │ +│ geometry_lod2_2 │ BLOB │ +└─────────────────┴───────────────────────┘ ``` -`ST_Geom3DFromWKB` takes `BLOB`, and **`geometry_lod0_0::BLOB` does not work** — DuckDB -v1.5.4 raises `Unimplemented type for cast (GEOMETRY(...) -> BLOB)`. Use `spatial`'s -`ST_AsWKB` as the bridge: +**`EPSG:7415` is a rendering, not what is stored.** `cityjson`'s writer puts the whole +PROJJSON document in the logical type's `crs` parameter, and `spatial` — loaded here — +resolves it back to its authority code for display. Without `spatial` the same column +prints as `GEOMETRY('{"$schema":"https://proj.org/schemas/v0.5/projjson…')`. The Parquet +side is unambiguous: + +```sql +SELECT name, logical_type IS NOT NULL AS annotated, length(logical_type) AS logical_type_len +FROM parquet_schema('/tmp/cp_test/pkg_out/building.parquet') +WHERE name LIKE 'geometry_lod%' ORDER BY 1; +``` + +``` +┌─────────────────┬───────────┬──────────────────┐ +│ name │ annotated │ logical_type_len │ +├─────────────────┼───────────┼──────────────────┤ +│ geometry_lod0_0 │ true │ 2081 │ +│ geometry_lod1_2 │ false │ NULL │ +│ geometry_lod1_3 │ false │ NULL │ +│ geometry_lod2_2 │ false │ NULL │ +└─────────────────┴───────────┴──────────────────┘ +``` + +**`geometry_lod0_0::BLOB` does not work** — DuckDB v1.5.4 raises +`Unimplemented type for cast (GEOMETRY('EPSG:7415') -> BLOB) when casting from source +column geometry_lod0_0`. `SET enable_geoparquet_conversion=false` does not help either: +the promotion follows the logical type, not the `geo` footer, so the column is still +`GEOMETRY` with the setting off. The constructors take the column as it comes: ```sql -LOAD spatial; SELECT count(*) AS n, - ROUND(max(abs(ST_3DFootprintArea(ST_Geom3DFromWKB(ST_AsWKB(geometry_lod0_0))) + ROUND(max(abs(ST_3DFootprintArea(ST_Geom3DFromWKB(geometry_lod0_0)) - ST_Area(geometry_lod0_0))), 12) AS max_abs_diff FROM read_parquet('/tmp/cp_test/pkg_out/building.parquet') WHERE geometry_lod0_0 IS NOT NULL; ``` @@ -910,6 +940,23 @@ FROM read_parquet('/tmp/cp_test/pkg_out/building.parquet') WHERE geometry_lod0_0 └──────┴───────────────┘ ``` +Routing the same column through `spatial`'s `ST_AsWKB` first is still valid and gives the +identical answer — it is simply no longer necessary: + +```sql +SELECT ROUND(max(abs(ST_3DFootprintArea(ST_Geom3DFromWKB(geometry_lod0_0)) + - ST_3DFootprintArea(ST_Geom3DFromWKB(ST_AsWKB(geometry_lod0_0))))), 12) AS direct_vs_bridged +FROM read_parquet('/tmp/cp_test/pkg_out/building.parquet') WHERE geometry_lod0_0 IS NOT NULL; +``` + +``` +┌───────────────────┐ +│ direct_vs_bridged │ +├───────────────────┤ +│ 0.0 │ +└───────────────────┘ +``` + This doubles as a **third independent oracle**: `ST_3DFootprintArea` agrees with `spatial`'s GEOS-backed `ST_Area` to 6.7e-5 m² worst case across 1115 footprints — about 4e-9 relative on areas up to 15 000 m². @@ -919,7 +966,7 @@ This doubles as a **third independent oracle**: `ST_3DFootprintArea` agrees with ```sql WITH raw AS (SELECT * FROM read_parquet('/tmp/cp_test/pkg_out/building.parquet')) SELECT b.id AS building, - ROUND(ST_3DFootprintArea(ST_Geom3DFromWKB(ST_AsWKB(b.geometry_lod0_0))), 2) AS lod0_area, + ROUND(ST_3DFootprintArea(ST_Geom3DFromWKB(b.geometry_lod0_0)), 2) AS lod0_area, ROUND(ST_3DVolume(ST_3DFromWKB(p.geometry_lod1_2, p.geometry_properties_lod1_2)), 1) AS lod12_vol, ROUND(ST_3DVolume(ST_3DFromWKB(p.geometry_lod2_2, p.geometry_properties_lod2_2)), 1) AS lod22_vol, ROUND(ST_3DFootprintArea(ST_3DFromWKB(p.geometry_lod2_2, p.geometry_properties_lod2_2)), 2) AS lod22_fp @@ -950,153 +997,6 @@ rows — which is what happened on the first attempt and looked like a broken fi generalisation), and diverges most on `…139`, a multi-part building where only one part is joined. LoD1.2 volume is a prism approximation and sits either side of LoD2.2. ---- - -# Part D — Arrow-native encoding - -The arrow-native path skips WKB entirely: nested `INTEGER[][][][][]` boundaries plus a -`STRUCT(x,y,z)[]` vertex pool go straight into the same payload. Before this notebook it -was only exercised on hand-built synthetic fixtures -(`test/sql/st_3d_from_arrow_native.test`, `test/cpp/test_arrow_native_import.cpp`); these -are the first runs against real data. - -## 24 — The columns `cityjson` emits - -```sql -SELECT column_name, column_type FROM ( - DESCRIBE SELECT * FROM read_cityjsonseq('../cityparquet-rs/tests/fixtures/delft.city.jsonl', - lod => '2.2', geometry_encoding := 'arrow-native') -) WHERE column_name LIKE '%geometry%'; -``` - -``` -┌────────────────────────────┬────────────────────────────────────────────────────────────────────────────────────────┐ -│ column_name │ column_type │ -├────────────────────────────┼────────────────────────────────────────────────────────────────────────────────────────┤ -│ geometry_lod2_2 │ INTEGER[][][][][] │ -│ geometry_vertices_lod2_2 │ STRUCT(x DOUBLE, y DOUBLE, z DOUBLE)[] │ -│ geometry_properties_lod2_2 │ STRUCT("type" VARCHAR, surfaces VARCHAR, face_semantics INTEGER[], shells INTEGER[][]) │ -└────────────────────────────┴────────────────────────────────────────────────────────────────────────────────────────┘ -``` - -Exactly the `(boundaries, vertices, geometry_properties)` triple the constructors expect. - -## 25 — Solid family: arrow-native equals WKB, byte for byte - -```sql -CREATE TABLE an AS -SELECT id, - ST_3DFromArrowNative(geometry_lod2_2, geometry_vertices_lod2_2, geometry_properties_lod2_2) AS s, - ST_3DTryFromArrowNative(geometry_lod2_2, geometry_vertices_lod2_2, geometry_properties_lod2_2) AS s_try -FROM read_cityjsonseq('../cityparquet-rs/tests/fixtures/delft.city.jsonl', - lod => '2.2', geometry_encoding := 'arrow-native') -WHERE geometry_lod2_2 IS NOT NULL; - -SELECT count(*) AS rows, count(s) AS imported, count(s_try) AS imported_try FROM an; - -SELECT count(*) AS joined, - count(*) FILTER (WHERE ST_3DAsWKB(a.s) = ST_3DAsWKB(c.s_struct)) AS identical_wkb, - count(*) FILTER (WHERE ST_3DNumFaces(a.s) = ST_3DNumFaces(c.s_struct)) AS same_faces, - ROUND(max(abs(ST_3DFootprintArea(a.s) - ST_3DFootprintArea(c.s_struct))), 12) AS max_fp_diff -FROM an a JOIN cp c USING (id); -``` - -``` -┌──────┬──────────┬──────────────┐ ┌────────┬───────────────┬────────────┬─────────────┐ -│ rows │ imported │ imported_try │ │ joined │ identical_wkb │ same_faces │ max_fp_diff │ -├──────┼──────────┼──────────────┤ ├────────┼───────────────┼────────────┼─────────────┤ -│ 1116 │ 1116 │ 1116 │ │ 1116 │ 1116 │ 1116 │ 0.0 │ -└──────┴──────────┴──────────────┘ └────────┴───────────────┴────────────┴─────────────┘ -``` - -All 1116 real solids canonicalise to identical bytes on both paths. - -## 26 — Surface family, and the family-dispatch contract - -`geometry_properties.type` decides which constructor pair applies. On the LoD3 model -(all `MultiSurface`/`CompositeSurface`), the `ST_Geom3D*` pair succeeds and the `ST_3D*` -pair returns `NULL`: - -```sql -SELECT count(*) AS rows, - count(ST_3DTryFromArrowNative(geometry_lod3_0, geometry_vertices_lod3_0, geometry_properties_lod3_0)) AS as_solid_try, - count(ST_Geom3DTryFromArrowNative(geometry_lod3_0, geometry_vertices_lod3_0, geometry_properties_lod3_0)) AS as_geom_try, - count(ST_Geom3DFromArrowNative(geometry_lod3_0, geometry_vertices_lod3_0, geometry_properties_lod3_0)) AS as_geom_strict -FROM read_cityjson('../cityparquet-rs/tests/fixtures/lod3_railway.city.json', - lod => '3', geometry_encoding := 'arrow-native') -WHERE geometry_lod3_0 IS NOT NULL; -``` - -``` -┌──────┬──────────────┬─────────────┬────────────────┐ -│ rows │ as_solid_try │ as_geom_try │ as_geom_strict │ -├──────┼──────────────┼─────────────┼────────────────┤ -│ 105 │ 0 │ 105 │ 105 │ -└──────┴──────────────┴─────────────┴────────────────┘ -``` - -## 27 — Where arrow-native and WKB legitimately differ - -```sql -WITH a AS ( - SELECT id, ST_Geom3DFromArrowNative(geometry_lod3_0, geometry_vertices_lod3_0, geometry_properties_lod3_0) AS g - FROM read_cityjson('../cityparquet-rs/tests/fixtures/lod3_railway.city.json', - lod => '3', geometry_encoding := 'arrow-native') - WHERE geometry_lod3_0 IS NOT NULL), -w AS ( - SELECT id, ST_Geom3DFromWKB(geometry_lod3_0) AS g - FROM read_cityjson('../cityparquet-rs/tests/fixtures/lod3_railway.city.json', lod => '3') - WHERE geometry_lod3_0 IS NOT NULL) -SELECT count(*) AS joined, - count(*) FILTER (WHERE ST_3DAsBinary(a.g) = ST_3DAsBinary(w.g)) AS identical_wkb, - count(*) FILTER (WHERE ST_3DNumGeometries(a.g) = ST_3DNumGeometries(w.g)) AS same_patch_count, - ROUND(max(abs(ST_3DFootprintArea(a.g) - ST_3DFootprintArea(w.g))), 12) AS max_fp_diff -FROM a JOIN w USING (id); -``` - -``` -┌────────┬───────────────┬──────────────────┬─────────────┐ -│ joined │ identical_wkb │ same_patch_count │ max_fp_diff │ -├────────┼───────────────┼──────────────────┼─────────────┤ -│ 105 │ 90 │ 105 │ 0.0 │ -└────────┴───────────────┴──────────────────┴─────────────┘ -``` - -**15 of 105 are not byte-identical**, yet patch counts and areas match exactly. The -arrow-native output is *smaller*, always by an exact multiple of 24 bytes (one XYZ -vertex): the arrow-native importer collapses duplicate ring vertices — both repeated -indices and distinct pool indices holding equal coordinates — while the WKB path -preserves whatever the producer wrote. Verified on the source: - -```sql -WITH rings AS ( - SELECT id, unnest(ring) AS r FROM ( - SELECT id, flatten(flatten(flatten(geometry_lod3_0))) AS ring - FROM read_cityjson('../cityparquet-rs/tests/fixtures/lod3_railway.city.json', - lod => '3', geometry_encoding := 'arrow-native') - WHERE id IN ('GMLID_0373494_301709_129', 'GMLID_855011_330784_753'))) -SELECT id, count(*) AS rings, - count(*) FILTER (WHERE len(r) <> len(list_distinct(r))) AS rings_with_repeated_index -FROM rings GROUP BY 1; -``` - -``` -┌──────────────────────────┬───────┬───────────────────────────┐ -│ id │ rings │ rings_with_repeated_index │ -├──────────────────────────┼───────┼───────────────────────────┤ -│ GMLID_855011_330784_753 │ 102 │ 0 │ -│ GMLID_0373494_301709_129 │ 102 │ 2 │ -└──────────────────────────┴───────┴───────────────────────────┘ -``` - -`…129` has two rings with a repeated *index* — exactly the 48-byte difference. `…753` -has none, so its 48 bytes come from the other mechanism: distinct pool indices carrying -equal coordinates. Both are deliberate (`test/cpp/test_arrow_native_import.cpp` pins -them); neither changes the geometry. **Do not assert byte equality across the two -encodings** — compare measurements or patch counts instead. - ---- - ## Quirks and known gaps ### A real bug this walkthrough found: volume drift under rotation @@ -1149,15 +1049,15 @@ and Newell's ring area, which made the degenerate-face verdict position-dependen - **§15 — Z changes under `ST_3DTransform` despite horizontal-only reprojection.** The centroid is area-weighted, so a reprojected XY footprint reweights it. The Z *coordinates* pass through untouched. -- **§27 — 15 of 105 geometries differ bytewise between arrow-native and WKB.** Duplicate - ring vertices are collapsed on the arrow-native path only. Same geometry, same areas. ### Traps that cost time - **`FILTER` does not guard a raising function** (§18). `SUM(ST_3DVolume(s)) FILTER (WHERE …is_valid)` still evaluates `ST_3DVolume` on invalid rows and raises. Filter in `WHERE`. -- **`geometry_lod0_0::BLOB` raises** in DuckDB v1.5.4 (§22). Bridge Parquet-native - `GEOMETRY` through `spatial`'s `ST_AsWKB`. +- **`geometry_lod0_0::BLOB` raises** in DuckDB v1.5.4 (§22), and + `SET enable_geoparquet_conversion=false` is not a way out — the promotion follows the + Parquet logical type, not the `geo` footer. Pass the `GEOMETRY` column straight to + `ST_Geom3DFromWKB`, which takes it. - **LoD0 and the solid LoDs never share a row** in 3DBAG (§23). Join `parents`/`children`, or the query silently returns nothing. - **The LoD suffix is normalised**: `lod => '2'` yields `geometry_lod2_0`. @@ -1178,8 +1078,6 @@ and Newell's ring area, which made the degenerate-face verdict position-dependen [FUTURE_WORK.md §1](./FUTURE_WORK.md). - **`ST_3DConvexHull` is a 2D XY hull** at minimum Z, not a true 3D hull — that needs the deferred CGAL/SFCGAL backend ([DESIGN_DOC.md §16](./DESIGN_DOC.md)). -- **Arrow-native constructors are experimental** and not part of the settled v1 surface - ([FUNCTIONS.md](./FUNCTIONS.md#arrow-native-constructors-experimental)). --- @@ -1203,12 +1101,11 @@ The behaviours it demonstrates are covered automatically as follows: | §15 | `test/sql/st_transform.test`, `test/cpp/test_crs_transform.cpp` | | §16 | `test/sql/geom_3d_construct.test`, `test/cpp/test_geom_construct.cpp` | | §20 | `test/sql/st_3d_from_wkb_struct.test` (the CityParquet STRUCT sidecar) | -| §24–§27 | `test/sql/st_3d_from_arrow_native.test`, `st_geom3d_from_arrow_native.test`, `test/cpp/test_arrow_native_import.cpp` — all **synthetic**; §25/§27 are the only real-data runs | +| §22 (GEOMETRY input) | `test/sql/wkb_from_geometry.test` (the constructors' `GEOMETRY` argument) | | §22 (oracle) | Partly `test/sql/postgis_oracle.test`; the `spatial` cross-check here is manual | **Genuinely uncovered by any automated test:** the CityParquet write→read round trip -(§19–§23) and the arrow-native path on real data (§25–§27). Both are candidates for new -gated test files; see [TEST_COVERAGE.md](./TEST_COVERAGE.md) for the oracle strategy. +(§19–§23). It is a candidate for a new gated test file; see [TEST_COVERAGE.md](./TEST_COVERAGE.md) for the oracle strategy. Remember `THREE_D_TEST_FIXTURES=1` if you run `build/release/duckdb` directly — without it the `st_aswkb*` helpers are unregistered and every file declaring diff --git a/docs/TEST_COVERAGE.md b/docs/TEST_COVERAGE.md index 500d823..dd0ae1f 100644 --- a/docs/TEST_COVERAGE.md +++ b/docs/TEST_COVERAGE.md @@ -92,7 +92,7 @@ Three comparisons are deliberately indirect: | `ST_3DDimension`, `ST_3DNumGeometries` on a `PolyhedralSurface` | PostGIS reads a *closed* `PolyhedralSurface` as a solid (`ST_Dimension` = 3, and 2 for an open one) and `ST_NumGeometries` as its patch count. This extension reports the topological dimension of the surface (2) and treats the surface as one geometry | oracled on the `geom` rows; `st_3d_introspection.test`, `geom_3d_accessors.test` | | `ST_3DNumSolids`, `ST_3DNumShells`, `ST_3DNumFaces` | Shell and patch counting differ structurally: the `shells` sidecar partition is a CityGML concept PostGIS's flat `PolyhedralSurface` has no equivalent of | `test/cpp/test_inner_shell.cpp`, `st_3d_hollow_solid.test`, `st_3d_multisolid.test`, `cityjson_multisolid.test` | | `ST_3DValidationReport`, `ST_3DIsManifold`, `ST_3DIsOriented` | PostGIS repairs or rejects; this extension flags without repairing. Its own report is authoritative | `st_3d_validation.test`, `test/cpp/test_validation.cpp`, `docs/TESTING.md` §8–9 | -| `ST_3DFromWKB`, `ST_3DTryFromWKB`, `ST_Geom3DFromWKB`, `ST_MakeSolid`, `ST_3DExtrude`, `ST_Force3D`, `ST_3DHasZ`, the `*FromArrowNative` family | Import and construction contracts, with no PostGIS analogue. Their correctness is proven *downstream*: every oracled measurement above runs on geometry they produced, so a broken importer fails the area/volume/bbox comparisons | `st_3d_from_wkb*.test`, `st_3d_from_arrow_native.test`, `geom_3d_construct.test`, `test/cpp/` | +| `ST_3DFromWKB`, `ST_3DTryFromWKB`, `ST_Geom3DFromWKB`, `ST_MakeSolid`, `ST_3DExtrude`, `ST_Force3D`, `ST_3DHasZ` | Import and construction contracts, with no PostGIS analogue. Their correctness is proven *downstream*: every oracled measurement above runs on geometry they produced, so a broken importer fails the area/volume/bbox comparisons | `st_3d_from_wkb*.test`, `geom_3d_construct.test`, `test/cpp/` | ### Metamorphic properties — dependency-free invariants diff --git a/justfile b/justfile index e85898f..17b8d21 100644 --- a/justfile +++ b/justfile @@ -68,6 +68,74 @@ cityjson_extension := env_var_or_default("CITYJSON_EXTENSION", "../duckdb-cityjs shell-cityjson: ./build/release/duckdb -unsigned -cmd "LOAD '{{cityjson_extension}}'; LOAD three_d;" +# ── DuckDB-Wasm ── + +# Wasm toolchain pins, mirroring the CI distribution pipeline. +# emsdk: extension-ci-tools v1.5.4 `_extension_distribution.yml` pins +# emscripten-core/setup-emsdk@v13 with version 3.1.71. +# vcpkg baseline: the `builtin-baseline` commit in vcpkg.json — a plain shallow clone +# does NOT contain it, so `wasm-setup` fetches it explicitly (manifest resolution +# fails otherwise). +emsdk_version := "3.1.71" +vcpkg_baseline := "84bab45d415d22042bd0b9081aea57f362da3f35" + +# Installs the pinned emsdk and a durable vcpkg checkout under the gitignored +# .vendor/. Idempotent — safe to re-run. ~2 GB and ~10 min the first time. + +# One-time toolchain bootstrap for `just wasm` (emsdk + vcpkg into .vendor/). +wasm-setup: + #!/usr/bin/env bash + set -euo pipefail + mkdir -p .vendor + if [ ! -d .vendor/emsdk ]; then + git clone --depth 200 https://github.com/emscripten-core/emsdk .vendor/emsdk + fi + (cd .vendor/emsdk && ./emsdk install {{emsdk_version}} && ./emsdk activate {{emsdk_version}}) + # A blobless partial clone, NOT a shallow one. vcpkg resolves `proj` through its + # versions database to a specific *port tree* — 62e9ace for 9.4.0 — and a shallow + # clone contains no history to find it in, so the install dies with "failed to + # unpack tree object ... vcpkg was cloned as a shallow repository". `--filter` + # keeps every commit and tree while fetching blobs on demand, which resolves any + # port version at a fraction of a full clone's size. + if [ ! -d .vendor/vcpkg ]; then + git clone --filter=blob:none https://github.com/microsoft/vcpkg .vendor/vcpkg + (cd .vendor/vcpkg && ./bootstrap-vcpkg.sh -disableMetrics) + fi + +# Build the DuckDB-Wasm extension. Run `just wasm-setup` once first. The first build +# is slow — vcpkg compiles PROJ and its dependencies for wasm32-emscripten — and +# later builds reuse those binaries. Output lands in +# build//extension/three_d/three_d.duckdb_extension.wasm, and the build also +# writes a loadable extension repository at build//repository/. +# +# The default is `wasm_eh`, not `wasm_mvp` as in the sibling duckdb-cityjson repo, +# because the consumers differ: that repo's default serves its wasm_mvp smoke +# harness, whereas here a browser is the only consumer. duckdb-wasm's selectBundle() +# picks the `eh` bundle wherever native wasm exceptions are available, and an `eh` +# instance can only load `eh` extensions. +# +# NOTE: no ninja here, unlike every other build recipe. The wasm targets in +# extension-ci-tools' duckdb_extension.Makefile hardcode +# `emmake make -j8 -Cbuild/`, so a Ninja-generated tree has no makefile and +# the build dies with "No targets specified and no makefile found". Recovering also +# needs `rm -rf build/` — CMake refuses to switch generator in place. +# +# ST_3DTransform calls proj_create_crs_to_crs, which reads PROJ's EPSG database at +# runtime. That database is not in the Emscripten filesystem, so expect that one +# function to fail under wasm; the other ST_3D* functions do not touch it. + +# Build the DuckDB-Wasm extension (eh by default) with the pinned emsdk + .vendor/vcpkg. +wasm flavour="wasm_eh": + #!/usr/bin/env bash + set -euo pipefail + case "{{flavour}}" in + wasm_mvp|wasm_eh|wasm_threads) ;; + *) echo "unknown wasm flavour: {{flavour}} (want wasm_mvp, wasm_eh or wasm_threads)" >&2; exit 2 ;; + esac + source .vendor/emsdk/emsdk_env.sh + VCPKG_TOOLCHAIN_PATH="$(pwd)/.vendor/vcpkg/scripts/buildsystems/vcpkg.cmake" make {{flavour}} + echo "-> build/{{flavour}}/extension/three_d/three_d.duckdb_extension.wasm" + # Remove build artifacts. clean: make clean diff --git a/src/functions/arrow_native.cpp b/src/functions/arrow_native.cpp deleted file mode 100644 index 608f04b..0000000 --- a/src/functions/arrow_native.cpp +++ /dev/null @@ -1,430 +0,0 @@ -#include "functions/three_d_functions.hpp" - -#include "duckdb/common/exception.hpp" -#include "duckdb/function/function_set.hpp" -#include "duckdb/function/scalar_function.hpp" - -#include "kernel/arrow_native_import.hpp" -#include "kernel/metadata_parser.hpp" -#include "kernel/solid_model.hpp" - -#include -#include -#include - -namespace duckdb { - -// Kernel names this file uses unqualified. Using-declarations rather than a -// using-directive, which clang-tidy's google-build-using-namespace rejects. -using duckdb_3d::ArrowNativeBoundaries; -using duckdb_3d::GeometryMetadata; -using duckdb_3d::ParseGeometryProperties; -using duckdb_3d::Vertex3D; - -// ────────────────────────────────────────────────────────────── -// geometry_properties STRUCT("type" VARCHAR, surfaces JSON, face_semantics -// INTEGER[], shells INTEGER[][]) → the kernel's plain GeometryMetadata. -// duckdb-cityjson's arrow-native output types geometry_properties_lod* this -// way instead of VARCHAR JSON text; the shared -// ReadGeometryPropertiesStructRow (functions/struct_metadata.cpp) extracts the -// same shell-grouping information the JSON-text path parses, for this path and -// ST_3DFromWKB's (BLOB, ANY) overload alike. It resolves `type`/`shells` by -// name, so the arrow-native overloads' fixed field order and the WKB overload's -// bind-normalised (reorderable) struct both read correctly, and it accepts the -// producer's INTEGER face counts as well as the WKB bind's normalised HUGEINT -// ones. It lives in the SQL/vectorized layer rather than in -// kernel/metadata_parser.* because it reads a live DuckDB Vector (mirroring how -// a BLOB WKB argument is unwrapped into a plain uint8_t*/size before ParseWKB). -// ────────────────────────────────────────────────────────────── - -// ────────────────────────────────────────────────────────────── -// Arrow-native ingestion (arrow-native-type branch): ST_3DFromArrowNative / -// ST_3DTryFromArrowNative consume the nested LIST<...>> -// boundaries + LIST> vertices columns cityparquet-rs / -// duckdb-cityjson write directly — no WKB bytes, no geometry_properties. -// ────────────────────────────────────────────────────────────── - -//! boundaries: solid -> shell -> face -> ring -> index, 5 levels of LIST. -LogicalType ArrowNativeGeometryType() { - auto ring = LogicalType::LIST(LogicalType::INTEGER); - auto face = LogicalType::LIST(ring); - auto shell = LogicalType::LIST(face); - auto solid = LogicalType::LIST(shell); - return LogicalType::LIST(solid); -} - -//! vertices: a flat pool, LIST>, referenced by index -//! from the boundaries' innermost ring-index lists. -LogicalType ArrowNativeVerticesType() { - child_list_t fields; - fields.push_back(make_pair("x", LogicalType::DOUBLE)); - fields.push_back(make_pair("y", LogicalType::DOUBLE)); - fields.push_back(make_pair("z", LogicalType::DOUBLE)); - return LogicalType::LIST(LogicalType::STRUCT(std::move(fields))); -} - -//! A literal or constant-folded expression (as in a simple `SELECT ...` test -//! query, or the STRUCT geometry_properties overload) can produce a -//! non-FLAT_VECTOR at any nesting depth, not only the top level — -//! FlatVector::GetData/IsNull assert genuine flat vectors. `count` is this -//! level's own cardinality (its list's total element count across every row, -//! not the outer chunk's row count — list children are a single vector -//! shared/concatenated across all rows' entries). -//! Walks one row of the boundaries Vector, flattening each nested level as it -//! descends, into the kernel's plain-C++ CSR form (real per-level traversal, -//! not a raw buffer cast: DuckDB ListVectors are list_entry_t pairs into a -//! shared child, with possible non-flat intermediate children). -static duckdb_3d::ArrowNativeBoundaries ExtractArrowNativeBoundaries(Vector &boundaries_vec, idx_t row) { - ArrowNativeBoundaries result; - - // CSR construction mirrors model_builder.cpp exactly: push a leading 0 for - // each offset array once, then push the running cumulative count once - // per completed element (shell/face/ring) — offsets[i] is element i's - // start, offsets[i+1] its end, so pushing "the count so far" right after - // finishing element i is exactly offsets[i+1]. - result.solid_shell_offsets.push_back(0); - result.shell_face_offsets.push_back(0); - result.face_ring_offsets.push_back(0); - result.ring_vertex_offsets.push_back(0); - - uint32_t total_shells = 0, total_faces = 0, total_rings = 0; - - // Nullability invariant (design doc): "within a non-null geometry cell, - // no nested list element is itself null — every solid/shell/face/ring - // entry and every vertex index is present." Each level below checks the - // CHILD vector's validity for the specific slot about to be dereferenced - // before reading its list_entry_t/int32 — reading an unchecked null slot - // is not merely "wrong data", it's genuinely undefined: two identical - // queries were observed to disagree on whether the same null ring even - // raised an error, because nothing constrains what bytes sit behind a - // null slot. - auto solid_entry = FlatVector::GetData(boundaries_vec)[row]; - auto &shell_vec = ListVector::GetEntry(boundaries_vec); - FlattenIfNeeded(shell_vec, ListVector::GetListSize(boundaries_vec)); - auto &shell_validity = FlatVector::Validity(shell_vec); - - for (idx_t solid_idx = solid_entry.offset; solid_idx < solid_entry.offset + solid_entry.length; solid_idx++) { - if (!shell_validity.RowIsValid(solid_idx)) { - throw InvalidInputException("arrow-native geometry: null shell entry (no nested list element may be null)"); - } - auto shell_entry = FlatVector::GetData(shell_vec)[solid_idx]; - auto &face_vec = ListVector::GetEntry(shell_vec); - FlattenIfNeeded(face_vec, ListVector::GetListSize(shell_vec)); - auto &face_validity = FlatVector::Validity(face_vec); - - for (idx_t shell_idx = shell_entry.offset; shell_idx < shell_entry.offset + shell_entry.length; shell_idx++) { - if (!face_validity.RowIsValid(shell_idx)) { - throw InvalidInputException( - "arrow-native geometry: null face entry (no nested list element may be null)"); - } - auto face_entry = FlatVector::GetData(face_vec)[shell_idx]; - auto &ring_vec = ListVector::GetEntry(face_vec); - FlattenIfNeeded(ring_vec, ListVector::GetListSize(face_vec)); - auto &ring_validity = FlatVector::Validity(ring_vec); - - for (idx_t face_idx = face_entry.offset; face_idx < face_entry.offset + face_entry.length; face_idx++) { - if (!ring_validity.RowIsValid(face_idx)) { - throw InvalidInputException( - "arrow-native geometry: null ring entry (no nested list element may be null)"); - } - auto ring_entry = FlatVector::GetData(ring_vec)[face_idx]; - auto &index_vec = ListVector::GetEntry(ring_vec); - FlattenIfNeeded(index_vec, ListVector::GetListSize(ring_vec)); - auto &index_validity = FlatVector::Validity(index_vec); - - for (idx_t r = ring_entry.offset; r < ring_entry.offset + ring_entry.length; r++) { - if (!index_validity.RowIsValid(r)) { - throw InvalidInputException( - "arrow-native geometry: null vertex-index list entry (no nested list element may be null)"); - } - auto idx_ring_entry = FlatVector::GetData(index_vec)[r]; - auto &leaf_vec = ListVector::GetEntry(index_vec); - FlattenIfNeeded(leaf_vec, ListVector::GetListSize(index_vec)); - auto &leaf_validity = FlatVector::Validity(leaf_vec); - auto leaf_data = FlatVector::GetData(leaf_vec); - - for (idx_t k = idx_ring_entry.offset; k < idx_ring_entry.offset + idx_ring_entry.length; k++) { - if (!leaf_validity.RowIsValid(k)) { - throw InvalidInputException( - "arrow-native geometry: null vertex-pool index (no nested list element may be null)"); - } - int32_t raw = leaf_data[k]; - if (raw < 0) { - throw InvalidInputException("arrow-native geometry: negative vertex-pool index"); - } - result.ring_vertex_indices.push_back(static_cast(raw)); - } - total_rings++; - result.ring_vertex_offsets.push_back(static_cast(result.ring_vertex_indices.size())); - } - total_faces++; - result.face_ring_offsets.push_back(total_rings); - } - total_shells++; - result.shell_face_offsets.push_back(total_faces); - } - result.solid_shell_offsets.push_back(total_shells); - } - - return result; -} - -//! Walks one row of the vertices Vector into a plain vertex pool. -static std::vector ExtractArrowNativeVertices(Vector &vertices_vec, idx_t row) { - auto vert_entry = FlatVector::GetData(vertices_vec)[row]; - auto &struct_vec = ListVector::GetEntry(vertices_vec); - auto &children = StructVector::GetEntries(struct_vec); - auto list_size = ListVector::GetListSize(vertices_vec); - FlattenIfNeeded(*children[0], list_size); - FlattenIfNeeded(*children[1], list_size); - FlattenIfNeeded(*children[2], list_size); - // Nullability invariant (design doc): "no Struct entry is null and - // none of x/y/z is null within a present entry." - auto &struct_validity = FlatVector::Validity(struct_vec); - auto &x_validity = FlatVector::Validity(*children[0]); - auto &y_validity = FlatVector::Validity(*children[1]); - auto &z_validity = FlatVector::Validity(*children[2]); - auto x_data = FlatVector::GetData(*children[0]); - auto y_data = FlatVector::GetData(*children[1]); - auto z_data = FlatVector::GetData(*children[2]); - - std::vector vertices; - vertices.reserve(vert_entry.length); - for (idx_t i = vert_entry.offset; i < vert_entry.offset + vert_entry.length; i++) { - if (!struct_validity.RowIsValid(i) || !x_validity.RowIsValid(i) || !y_validity.RowIsValid(i) || - !z_validity.RowIsValid(i)) { - throw InvalidInputException("arrow-native geometry: null vertex-pool entry or coordinate " - "(no vertex or coordinate may be null)"); - } - vertices.push_back(Vertex3D {x_data[i], y_data[i], z_data[i]}); - } - return vertices; -} - -//! Design doc "critical invariant": the physical boundaries/vertices shape is -//! uniform across Solid-family and (padded) surface-family rows, so a single -//! column may legitimately mix them. Consumers MUST dispatch on -//! geometry_properties.type per row, never on physical shape — checking -//! shell-count/solid-count alone cannot distinguish a real single-shell -//! Solid from a padded MultiSurface (they are, by design, shape-identical). -static bool IsSolidFamilyType(const std::string &type) { - return type == "Solid" || type == "MultiSolid" || type == "CompositeSolid"; -} - -static bool IsSurfaceFamilyType(const std::string &type) { - return type == "MultiSurface" || type == "CompositeSurface"; -} - -//! Shared row logic for ST_3DFromArrowNative/ST_3DTryFromArrowNative, given an -//! already-extracted GeometryMetadata (from either the VARCHAR or STRUCT -//! geometry_properties overload). Checks the family, then builds+serializes. -//! metadata is also passed to the kernel so BuildSolidModelFromArrowNative -//! can regroup shells from metadata.shells — a real producer (confirmed: -//! cityparquet-rs's arrow_geom_write.rs) always pads a Solid's boundaries to -//! one physical shell, so trusting the physical nesting alone would merge -//! every interior shell into the exterior and silently skip -//! CheckInteriorShellWinding. -static std::string BuildSolidPayloadForRow(Vector &boundaries_vec, Vector &vertices_vec, idx_t row, - const duckdb_3d::GeometryMetadata &metadata) { - if (!IsSolidFamilyType(metadata.type)) { - throw InvalidInputException("geometry_properties.type '" + metadata.type + - "' is not a solid-family type (Solid/MultiSolid/CompositeSolid) — " - "call ST_Geom3DFromArrowNative for surface types"); - } - auto boundaries = ExtractArrowNativeBoundaries(boundaries_vec, row); - auto vertices = ExtractArrowNativeVertices(vertices_vec, row); - auto model = BuildSolidModelFromArrowNative(boundaries, vertices, metadata); - auto payload = SerializePayload(model); - return std::string(reinterpret_cast(payload.data()), payload.size()); -} - -//! Shared row logic for ST_Geom3DFromArrowNative/ST_Geom3DTryFromArrowNative. -//! Surface types carry no shells (BuildGeomModelFromArrowNative's own -//! padding-dimension assertion is the only structural check needed), so no -//! metadata-driven regrouping applies here. -static std::string BuildGeomPayloadForRow(Vector &boundaries_vec, Vector &vertices_vec, idx_t row, - const duckdb_3d::GeometryMetadata &metadata) { - if (!IsSurfaceFamilyType(metadata.type)) { - throw InvalidInputException("geometry_properties.type '" + metadata.type + - "' is not a surface-family type (MultiSurface/CompositeSurface) — " - "call ST_3DFromArrowNative for solid types"); - } - auto boundaries = ExtractArrowNativeBoundaries(boundaries_vec, row); - auto vertices = ExtractArrowNativeVertices(vertices_vec, row); - auto model = BuildGeomModelFromArrowNative(boundaries, vertices); - auto payload = SerializeGeomPayload(model); - return std::string(reinterpret_cast(payload.data()), payload.size()); -} - -// ────────────────────────────────────────────────────────────── -// Arrow-native ingestion: -// ST_3DFromArrowNative / ST_3DTryFromArrowNative(boundaries, vertices, -// geometry_properties) → SOLID_3D, plus the ST_Geom3D* twins → GEOM_3D. -// -// One executor covers all eight overloads: {solid, surface} × {plain, TRY} × -// {JSON VARCHAR metadata, geometry_properties STRUCT}. The VARCHAR overloads -// parse JSON text through the same ParseGeometryProperties the WKB path uses — -// per the design doc, arrow-native keeps geometry_properties as VARCHAR-JSON on -// both paths precisely so this parsing step is identical, not something the new -// path gets to skip. The STRUCT overloads mirror ST_3DFromWKB's struct -// overload: they bind directly against cityparquet-rs's real -// geometry_properties_lod* column (confirmed STRUCT, the same type for WKB and -// arrow-native rows) without an explicit to_json() cast. -// -// `.type` is load-bearing here — it dispatches solid-family vs surface-family -// in the row builders — rather than merely informational as on the WKB path. -// For the surface functions the boundaries' padding to solid-count 1 / -// shell-count 1 (see BuildGeomModelFromArrowNative) is only a defensive -// secondary check. -// ────────────────────────────────────────────────────────────── - -//! Shared executor for all eight arrow-native ingestion functions -//! (st_3d[try]fromarrownative and st_geom3d[try]fromarrownative, VARCHAR and -//! STRUCT geometry_properties). BUILD_ROW is BuildSolidPayloadForRow or -//! BuildGeomPayloadForRow; TRY_VARIANT turns per-row failures into NULLs. -using ArrowNativeRowBuilder = std::string (*)(Vector &, Vector &, idx_t, const duckdb_3d::GeometryMetadata &); - -template -static void FromArrowNativeExecutor(DataChunk &args, ExpressionState &state, Vector &result) { - auto &boundaries_vec = args.data[0]; - auto &vertices_vec = args.data[1]; - auto &meta_vec = args.data[2]; - auto count = args.size(); - bool all_constant = args.AllConstant(); - - boundaries_vec.Flatten(count); - vertices_vec.Flatten(count); - auto &boundaries_validity = FlatVector::Validity(boundaries_vec); - auto &vertices_validity = FlatVector::Validity(vertices_vec); - auto &result_validity = FlatVector::Validity(result); - auto result_data = FlatVector::GetData(result); - - UnifiedVectorFormat meta_data; - const string_t *meta_strings = nullptr; - if constexpr (SOURCE == MetaSource::STRUCT_FIELDS) { - meta_vec.Flatten(count); - } else { - meta_vec.ToUnifiedFormat(count, meta_data); - meta_strings = UnifiedVectorFormat::GetData(meta_data); - } - - for (idx_t i = 0; i < count; i++) { - bool meta_valid; - idx_t meta_idx = i; - if constexpr (SOURCE == MetaSource::STRUCT_FIELDS) { - meta_valid = FlatVector::Validity(meta_vec).RowIsValid(i); - } else { - meta_idx = meta_data.sel->get_index(i); - meta_valid = meta_data.validity.RowIsValid(meta_idx); - } - if (!boundaries_validity.RowIsValid(i) || !vertices_validity.RowIsValid(i) || !meta_valid) { - result_validity.SetInvalid(i); - result_data[i] = string_t(); - continue; - } - auto process_row = [&]() { - GeometryMetadata metadata; - if constexpr (SOURCE == MetaSource::STRUCT_FIELDS) { - metadata = ReadGeometryPropertiesStructRow(meta_vec, count, i); - } else { - auto &meta_str = meta_strings[meta_idx]; - metadata = ParseGeometryProperties(std::string(meta_str.GetData(), meta_str.GetSize())); - } - auto payload = BUILD_ROW(boundaries_vec, vertices_vec, i, metadata); - result_data[i] = StringVector::AddStringOrBlob(result, string_t(payload.data(), payload.size())); - }; - if constexpr (TRY_VARIANT) { - try { - process_row(); - } catch (...) { - result_validity.SetInvalid(i); - result_data[i] = string_t(); - } - } else { - process_row(); - } - } - - if (all_constant) { - result.SetVectorType(VectorType::CONSTANT_VECTOR); - } -} - -//! geometry_properties STRUCT("type" VARCHAR, surfaces JSON, face_semantics -//! INTEGER[], shells INTEGER[][]) — the shape duckdb-cityjson's arrow-native -//! output and cityparquet-rs both emit for geometry_properties_lod* instead -//! of/alongside VARCHAR JSON text. -static LogicalType GeometryPropertiesStructType() { - child_list_t geom_props_fields; - geom_props_fields.push_back(make_pair("type", LogicalType::VARCHAR)); - geom_props_fields.push_back(make_pair("surfaces", LogicalType::VARCHAR)); - geom_props_fields.push_back(make_pair("face_semantics", LogicalType::LIST(LogicalType::INTEGER))); - geom_props_fields.push_back(make_pair("shells", LogicalType::LIST(LogicalType::LIST(LogicalType::INTEGER)))); - return LogicalType::STRUCT(std::move(geom_props_fields)); -} - -void RegisterArrowNativeFunctions(ExtensionLoader &loader, const LogicalType &solid_3d_type, - const LogicalType &geom_3d_type) { - // Declared here (before any registration that needs it) so both the - // GEOM_3D and SOLID_3D arrow-native STRUCT overloads below can share it. - auto geometry_properties_struct_type = GeometryPropertiesStructType(); - - // ST_Geom3DFromArrowNative / ST_Geom3DTryFromArrowNative: VARCHAR and STRUCT - // geometry_properties overloads (STRUCT binds directly against - // cityparquet-rs's real geometry_properties_lod* column). - ScalarFunctionSet geom3d_from_arrow_native_set("st_geom3dfromarrownative"); - auto geom3d_from_arrow_native = - ScalarFunction({ArrowNativeGeometryType(), ArrowNativeVerticesType(), LogicalType::VARCHAR}, geom_3d_type, - FromArrowNativeExecutor); - geom3d_from_arrow_native.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); - geom3d_from_arrow_native_set.AddFunction(geom3d_from_arrow_native); - auto geom3d_from_arrow_native_struct = - ScalarFunction({ArrowNativeGeometryType(), ArrowNativeVerticesType(), geometry_properties_struct_type}, - geom_3d_type, FromArrowNativeExecutor); - geom3d_from_arrow_native_struct.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); - geom3d_from_arrow_native_set.AddFunction(geom3d_from_arrow_native_struct); - loader.RegisterFunction(geom3d_from_arrow_native_set); - - ScalarFunctionSet geom3d_try_from_arrow_native_set("st_geom3dtryfromarrownative"); - auto geom3d_try_from_arrow_native = - ScalarFunction({ArrowNativeGeometryType(), ArrowNativeVerticesType(), LogicalType::VARCHAR}, geom_3d_type, - FromArrowNativeExecutor); - geom3d_try_from_arrow_native.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); - geom3d_try_from_arrow_native_set.AddFunction(geom3d_try_from_arrow_native); - auto geom3d_try_from_arrow_native_struct = - ScalarFunction({ArrowNativeGeometryType(), ArrowNativeVerticesType(), geometry_properties_struct_type}, - geom_3d_type, FromArrowNativeExecutor); - geom3d_try_from_arrow_native_struct.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); - geom3d_try_from_arrow_native_set.AddFunction(geom3d_try_from_arrow_native_struct); - loader.RegisterFunction(geom3d_try_from_arrow_native_set); - - // ST_3DFromArrowNative / ST_3DTryFromArrowNative(boundaries, vertices, geometry_properties) -> SOLID_3D - // VARCHAR and STRUCT geometry_properties overloads, mirroring the WKB set above. - ScalarFunctionSet from_arrow_native_set("st_3dfromarrownative"); - auto from_arrow_native = - ScalarFunction({ArrowNativeGeometryType(), ArrowNativeVerticesType(), LogicalType::VARCHAR}, solid_3d_type, - FromArrowNativeExecutor); - from_arrow_native.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); - from_arrow_native_set.AddFunction(from_arrow_native); - auto from_arrow_native_struct = ScalarFunction( - {ArrowNativeGeometryType(), ArrowNativeVerticesType(), geometry_properties_struct_type}, solid_3d_type, - FromArrowNativeExecutor); - from_arrow_native_struct.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); - from_arrow_native_set.AddFunction(from_arrow_native_struct); - loader.RegisterFunction(from_arrow_native_set); - - ScalarFunctionSet try_from_arrow_native_set("st_3dtryfromarrownative"); - auto try_from_arrow_native = - ScalarFunction({ArrowNativeGeometryType(), ArrowNativeVerticesType(), LogicalType::VARCHAR}, solid_3d_type, - FromArrowNativeExecutor); - try_from_arrow_native.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); - try_from_arrow_native_set.AddFunction(try_from_arrow_native); - auto try_from_arrow_native_struct = ScalarFunction( - {ArrowNativeGeometryType(), ArrowNativeVerticesType(), geometry_properties_struct_type}, solid_3d_type, - FromArrowNativeExecutor); - try_from_arrow_native_struct.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); - try_from_arrow_native_set.AddFunction(try_from_arrow_native_struct); - loader.RegisterFunction(try_from_arrow_native_set); -} - -} // namespace duckdb diff --git a/src/functions/geom_accessors.cpp b/src/functions/geom_accessors.cpp index eaad3c7..148c801 100644 --- a/src/functions/geom_accessors.cpp +++ b/src/functions/geom_accessors.cpp @@ -1,6 +1,8 @@ #include "functions/three_d_functions.hpp" #include "duckdb/common/exception.hpp" +#include "duckdb/common/types/geometry.hpp" +#include "duckdb/function/function_set.hpp" #include "duckdb/function/scalar_function.hpp" #include "kernel/geom_analysis.hpp" @@ -26,10 +28,10 @@ using duckdb_3d::ReadGeomPayloadHeader; // GEOM_3D: general geometry construction and accessors // ────────────────────────────────────────────────────────────── -// ST_Geom3DFromWKB(wkb BLOB) → GEOM_3D +// ST_Geom3DFromWKB(wkb BLOB | GEOMETRY) → GEOM_3D // (named to avoid clashing with DuckDB core's st_geomfromwkb -> GEOMETRY) -static void ST_Geom3DFromWKBFun(DataChunk &args, ExpressionState &state, Vector &result) { - UnaryExecutor::Execute(args.data[0], result, args.size(), [&](string_t wkb) { +static void Geom3DFromWKBVector(Vector &wkb_vec, idx_t count, Vector &result) { + UnaryExecutor::Execute(wkb_vec, result, count, [&](string_t wkb) { auto model = ParseGeomWKB(reinterpret_cast(wkb.GetData()), wkb.GetSize()); auto payload = SerializeGeomPayload(model); return StringVector::AddStringOrBlob(result, @@ -37,6 +39,26 @@ static void ST_Geom3DFromWKBFun(DataChunk &args, ExpressionState &state, Vector }); } +static void ST_Geom3DFromWKBFun(DataChunk &args, ExpressionState &state, Vector &result) { + Geom3DFromWKBVector(args.data[0], args.size(), result); +} + +// ST_Geom3DFromWKB(geom GEOMETRY) → GEOM_3D. A CityParquet footprint column +// carries the Parquet GEOMETRY logical type, so DuckDB hands it over as GEOMETRY +// rather than BLOB. Geometry::ToBinary is the supported way down to WKB — the +// storage is physically a string, but not contractually raw WKB, so the string_t +// must not simply be reinterpreted. +static void ST_Geom3DFromGeometryFun(DataChunk &args, ExpressionState &state, Vector &result) { + Vector wkb_vec(LogicalType::BLOB); + Geometry::ToBinary(args.data[0], wkb_vec, args.size()); + Geom3DFromWKBVector(wkb_vec, args.size(), result); +} + +static unique_ptr BindGeom3DFromWkbArg(ClientContext &, ScalarFunction &bound_function, + vector> &arguments) { + return BindWkbArgument(bound_function, arguments, ST_Geom3DFromWKBFun, ST_Geom3DFromGeometryFun); +} + //! Reject a type code that is not in the GEOM_3D enum. The header-read accessors //! below consult exactly this field, so a crafted code must not be laundered into //! a plausible generic answer — unlike ST_3DZMin/ST_3DZMax, whose contract is @@ -315,8 +337,10 @@ static void ST_3DNumGeometriesFun(DataChunk &args, ExpressionState &state, Vecto void RegisterGeomAccessorFunctions(ExtensionLoader &loader, const LogicalType &solid_3d_type, const LogicalType &geom_3d_type) { - // GEOM_3D construction and accessors - loader.RegisterFunction(ScalarFunction("st_geom3dfromwkb", {LogicalType::BLOB}, geom_3d_type, ST_Geom3DFromWKBFun)); + // GEOM_3D construction and accessors. One ANY candidate, dispatched at bind + // time, so that BLOB, GEOMETRY and an untyped NULL all bind (see BindWkbArgument). + loader.RegisterFunction(ScalarFunction("st_geom3dfromwkb", {LogicalType::ANY}, geom_3d_type, ST_Geom3DFromWKBFun, + BindGeom3DFromWkbArg)); loader.RegisterFunction( ScalarFunction("st_3dgeometrytype", {geom_3d_type}, LogicalType::VARCHAR, ST_3DGeometryTypeFun)); diff --git a/src/functions/solid_io.cpp b/src/functions/solid_io.cpp index c3e9123..9fde34d 100644 --- a/src/functions/solid_io.cpp +++ b/src/functions/solid_io.cpp @@ -2,6 +2,7 @@ #include "duckdb/common/exception.hpp" #include "duckdb/common/string_util.hpp" +#include "duckdb/common/types/geometry.hpp" #include "duckdb/function/function_set.hpp" #include "duckdb/function/scalar_function.hpp" @@ -24,11 +25,10 @@ using duckdb_3d::ParseWKB; using duckdb_3d::SolidModel; // ────────────────────────────────────────────────────────────── -// ST_3DFromWKB(wkb BLOB) → SOLID_3D (BLOB) +// ST_3DFromWKB(wkb BLOB | GEOMETRY) → SOLID_3D (BLOB) // ────────────────────────────────────────────────────────────── -static void ST_3DFromWKBFun(DataChunk &args, ExpressionState &state, Vector &result) { - auto &wkb_vec = args.data[0]; - UnaryExecutor::Execute(wkb_vec, result, args.size(), [&](string_t wkb) { +static void FromWKBVector(Vector &wkb_vec, idx_t count, Vector &result) { + UnaryExecutor::Execute(wkb_vec, result, count, [&](string_t wkb) { auto surfaces = ParseWKB(reinterpret_cast(wkb.GetData()), wkb.GetSize()); auto model = BuildSolidModel(surfaces); auto payload = SerializePayload(model); @@ -37,13 +37,16 @@ static void ST_3DFromWKBFun(DataChunk &args, ExpressionState &state, Vector &res }); } +static void ST_3DFromWKBFun(DataChunk &args, ExpressionState &state, Vector &result) { + FromWKBVector(args.data[0], args.size(), result); +} + // ────────────────────────────────────────────────────────────── -// ST_3DTryFromWKB(wkb BLOB) → SOLID_3D (BLOB) or NULL +// ST_3DTryFromWKB(wkb BLOB | GEOMETRY) → SOLID_3D (BLOB) or NULL // ────────────────────────────────────────────────────────────── -static void ST_3DTryFromWKBFun(DataChunk &args, ExpressionState &state, Vector &result) { - auto &wkb_vec = args.data[0]; +static void TryFromWKBVector(Vector &wkb_vec, idx_t count, Vector &result) { UnaryExecutor::ExecuteWithNulls( - wkb_vec, result, args.size(), [&](string_t wkb, ValidityMask &mask, idx_t idx) -> string_t { + wkb_vec, result, count, [&](string_t wkb, ValidityMask &mask, idx_t idx) -> string_t { try { auto surfaces = ParseWKB(reinterpret_cast(wkb.GetData()), wkb.GetSize()); auto model = BuildSolidModel(surfaces); @@ -57,6 +60,40 @@ static void ST_3DTryFromWKBFun(DataChunk &args, ExpressionState &state, Vector & }); } +static void ST_3DTryFromWKBFun(DataChunk &args, ExpressionState &state, Vector &result) { + TryFromWKBVector(args.data[0], args.size(), result); +} + +// ────────────────────────────────────────────────────────────── +// ST_3DFromWKB / ST_3DTryFromWKB(geom GEOMETRY) → SOLID_3D +// +// A GeoParquet-legal column reaches a query as the Parquet GEOMETRY logical type +// rather than BLOB. Geometry::ToBinary is the supported route down to WKB: the +// storage is physically a string, but not contractually raw WKB, so the string_t +// must not simply be reinterpreted. +// ────────────────────────────────────────────────────────────── +static void ST_3DFromGeometryFun(DataChunk &args, ExpressionState &state, Vector &result) { + Vector wkb_vec(LogicalType::BLOB); + Geometry::ToBinary(args.data[0], wkb_vec, args.size()); + FromWKBVector(wkb_vec, args.size(), result); +} + +static void ST_3DTryFromGeometryFun(DataChunk &args, ExpressionState &state, Vector &result) { + Vector wkb_vec(LogicalType::BLOB); + Geometry::ToBinary(args.data[0], wkb_vec, args.size()); + TryFromWKBVector(wkb_vec, args.size(), result); +} + +static unique_ptr BindFromWkbArg(ClientContext &, ScalarFunction &bound_function, + vector> &arguments) { + return BindWkbArgument(bound_function, arguments, ST_3DFromWKBFun, ST_3DFromGeometryFun); +} + +static unique_ptr BindTryFromWkbArg(ClientContext &, ScalarFunction &bound_function, + vector> &arguments) { + return BindWkbArgument(bound_function, arguments, ST_3DTryFromWKBFun, ST_3DTryFromGeometryFun); +} + // ────────────────────────────────────────────────────────────── // ST_3DFromWKB(wkb BLOB, geometry_properties STRUCT) → SOLID_3D // @@ -69,9 +106,9 @@ static void ST_3DTryFromWKBFun(DataChunk &args, ExpressionState &state, Vector & // so it is a strict superset of the VARCHAR overload. // // The struct itself is read row-by-row by the shared, name-resolved -// ReadGeometryPropertiesStructRow (functions/struct_metadata.cpp), which the -// arrow-native STRUCT overloads use too — so the bind needs no per-field index -// bookkeeping, only the type normalisation and the missing-`shells` diagnosis. +// ReadGeometryPropertiesStructRow (functions/struct_metadata.cpp) — so the +// bind needs no per-field index bookkeeping, only the type normalisation and +// the missing-`shells` diagnosis. // ────────────────────────────────────────────────────────────── // ────────────────────────────────────────────────────────────── @@ -241,11 +278,12 @@ static void ST_3DAsWKBFun(DataChunk &args, ExpressionState &state, Vector &resul } void RegisterSolidIOFunctions(ExtensionLoader &loader, const LogicalType &solid_3d_type) { - // ST_3DFromWKB: 1-arg, 2-arg (VARCHAR), and 2-arg (STRUCT) overloads. + // ST_3DFromWKB: 1-arg (BLOB or GEOMETRY, dispatched at bind time), 2-arg + // (VARCHAR), and 2-arg (STRUCT) overloads. // The constructors return the SOLID_3D alias so their result carries the type // through to the typed consumer overloads without an explicit cast. ScalarFunctionSet from_wkb_set("st_3dfromwkb"); - from_wkb_set.AddFunction(ScalarFunction({LogicalType::BLOB}, solid_3d_type, ST_3DFromWKBFun)); + from_wkb_set.AddFunction(ScalarFunction({LogicalType::ANY}, solid_3d_type, ST_3DFromWKBFun, BindFromWkbArg)); auto from_wkb_2arg = ScalarFunction({LogicalType::BLOB, LogicalType::VARCHAR}, solid_3d_type, FromWKBWithMetaExecutor); from_wkb_2arg.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); @@ -258,9 +296,10 @@ void RegisterSolidIOFunctions(ExtensionLoader &loader, const LogicalType &solid_ from_wkb_set.AddFunction(from_wkb_any); loader.RegisterFunction(from_wkb_set); - // ST_3DTryFromWKB: 1-arg, 2-arg (VARCHAR), and 2-arg (STRUCT) overloads + // ST_3DTryFromWKB: the same three shapes ScalarFunctionSet try_from_wkb_set("st_3dtryfromwkb"); - try_from_wkb_set.AddFunction(ScalarFunction({LogicalType::BLOB}, solid_3d_type, ST_3DTryFromWKBFun)); + try_from_wkb_set.AddFunction( + ScalarFunction({LogicalType::ANY}, solid_3d_type, ST_3DTryFromWKBFun, BindTryFromWkbArg)); auto try_from_wkb_2arg = ScalarFunction({LogicalType::BLOB, LogicalType::VARCHAR}, solid_3d_type, FromWKBWithMetaExecutor); try_from_wkb_2arg.SetNullHandling(FunctionNullHandling::SPECIAL_HANDLING); diff --git a/src/functions/struct_metadata.cpp b/src/functions/struct_metadata.cpp index 5322011..497d21f 100644 --- a/src/functions/struct_metadata.cpp +++ b/src/functions/struct_metadata.cpp @@ -14,7 +14,7 @@ namespace duckdb { namespace { //! One face count, dispatched on the bound child type: HUGEINT after the -//! (BLOB, ANY) bind normalisation, INTEGER from the arrow-native overloads. +//! (BLOB, ANY) bind normalisation; INTEGER from a producer or SQL struct. uint32_t ReadFaceCount(Vector &int_vec, idx_t pos) { switch (int_vec.GetType().id()) { case LogicalTypeId::HUGEINT: { diff --git a/src/include/functions/three_d_functions.hpp b/src/include/functions/three_d_functions.hpp index 554dbae..66524ef 100644 --- a/src/include/functions/three_d_functions.hpp +++ b/src/include/functions/three_d_functions.hpp @@ -27,6 +27,30 @@ inline PayloadKind GetPayloadKind(const uint8_t *data, size_t size) { return PayloadKind::Unknown; } +//! Bind for the single-argument WKB constructors. The argument is declared ANY +//! so that ONE candidate covers both the BLOB a plain column carries and the +//! GEOMETRY a Parquet-annotated column carries. Two separate overloads would +//! read more naturally but make an untyped `ST_3DFromWKB(NULL)` ambiguous — +//! SQLNULL casts to either at the same cost — and that call has always bound. +inline unique_ptr BindWkbArgument(ScalarFunction &bound_function, + vector> &arguments, scalar_function_t blob_fn, + scalar_function_t geometry_fn) { + auto &arg_type = arguments[0]->return_type; + if (arg_type.id() == LogicalTypeId::UNKNOWN) { + // A prepared-statement '?' parameter: defer to a later re-bind. + throw ParameterNotResolvedException(); + } + if (arg_type.id() == LogicalTypeId::GEOMETRY) { + // Keep the argument's own type, CRS parameter and all, so no cast is added. + bound_function.arguments[0] = arg_type; + bound_function.function = std::move(geometry_fn); + return nullptr; + } + bound_function.arguments[0] = LogicalType::BLOB; + bound_function.function = std::move(blob_fn); + return nullptr; +} + //! A literal or constant-folded expression can produce a non-FLAT_VECTOR at any //! nesting depth — FlatVector::GetData/IsNull assert genuine flat vectors. //! `count` is this level's own cardinality (a list child's total element count @@ -41,11 +65,10 @@ inline void FlattenIfNeeded(Vector &vec, idx_t count) { //! GeometryMetadata. Resolves `type` and `shells` children by case-insensitive //! name, applies FlattenIfNeeded at every nesting level before dereferencing, //! and accepts both HUGEINT face counts (the (BLOB, ANY) bind normalises -//! shells to HUGEINT[][]) and INTEGER face counts (the arrow-native overloads -//! bind the producer's INTEGER[][] directly). `struct_vec` must already be +//! shells to HUGEINT[][]) and INTEGER face counts, which a producer or a +//! hand-built SQL struct may carry directly. `struct_vec` must already be //! flattened to `count` rows by the caller. Defined in -//! functions/struct_metadata.cpp; shared because the WKB (solid_io) and -//! arrow-native STRUCT overloads live in different translation units. +//! functions/struct_metadata.cpp. duckdb_3d::GeometryMetadata ReadGeometryPropertiesStructRow(Vector &struct_vec, idx_t count, idx_t row); //! The coordinate dimension every v1 value carries. Defined in @@ -63,7 +86,5 @@ void RegisterGeomAccessorFunctions(ExtensionLoader &loader, const LogicalType &s void RegisterDistanceFunctions(ExtensionLoader &loader, const LogicalType &geom_3d_type); void RegisterTransformFunctions(ExtensionLoader &loader, const LogicalType &solid_3d_type, const LogicalType &geom_3d_type); -void RegisterArrowNativeFunctions(ExtensionLoader &loader, const LogicalType &solid_3d_type, - const LogicalType &geom_3d_type); } // namespace duckdb diff --git a/src/include/kernel/arrow_native_import.hpp b/src/include/kernel/arrow_native_import.hpp deleted file mode 100644 index 33d49a6..0000000 --- a/src/include/kernel/arrow_native_import.hpp +++ /dev/null @@ -1,71 +0,0 @@ -#pragma once - -#include "kernel/geom_model.hpp" -#include "kernel/metadata_parser.hpp" -#include "kernel/solid_model.hpp" -#include -#include - -namespace duckdb_3d { - -//! Plain-C++, DuckDB-agnostic flattened representation of the arrow-native -//! nested boundaries shape (solid -> shell -> face -> ring -> index), CSR -//! offset arrays over a flat vertex-index buffer. The DuckDB-aware SQL layer -//! (three_d_extension.cpp) walks the nested LIST<...>> Vector -//! and produces this; this type — and everything below it in this file — -//! never sees a DuckDB Vector directly, keeping kernel/ DuckDB-free (mirrors -//! how ParseWKB only ever sees a flat uint8_t*/size, never a Vector). -//! -//! Both the Solid/MultiSolid/CompositeSolid family (BuildSolidModelFromArrowNative) -//! and the padded MultiSurface/CompositeSurface family (BuildGeomModelFromArrowNative) -//! share this exact same flattened shape — a surface value is a boundaries -//! value with solid_shell_offsets/shell_face_offsets padded to size 2 (one -//! solid, one shell), per the design doc's padding-dimension convention. -struct ArrowNativeBoundaries { - std::vector solid_shell_offsets; // [solid_count+1] - std::vector shell_face_offsets; // [shell_count+1] - std::vector face_ring_offsets; // [face_count+1] - std::vector ring_vertex_offsets; // [ring_count+1] - std::vector ring_vertex_indices; // raw indices into the vertex pool -}; - -//! Builds a validated, triangulated SolidModel from already-flattened -//! arrow-native boundaries + a vertex pool. This is a public SQL-callable -//! ingestion boundary, so — unlike simply trusting the writer's -//! distinct-index-compaction invariant — vertices are defensively -//! deduplicated by coordinate equality (mirroring model_builder.cpp's own -//! GetOrAddVertex) before indices are remapped and bounds-checked against -//! them; SolidModel's own construction pipeline, shared with the WKB path, -//! still runs in full: ComputeBBox, TriangulateSolidModel, ValidateSolidModel. -SolidModel BuildSolidModelFromArrowNative(const ArrowNativeBoundaries &boundaries, - const std::vector &vertices); - -//! Metadata-aware overload: regroups the physical boundaries' shells from -//! `metadata.shells` before delegating to the 2-arg overload above, mirroring -//! model_builder.cpp's BuildSolidModel(surfaces, metadata) exactly. Real -//! arrow-native producers (confirmed: cityparquet-rs's arrow_geom_write.rs) -//! always pad each solid to exactly one physical shell, flattening any real -//! interior shells into a single face list exactly like the WKB path -//! flattens them into one PolyhedralSurface — the real per-solid shell -//! partition lives only in geometry_properties.shells, never in the -//! boundaries' own "shell" nesting level for the Solid family. Skipping this -//! regrouping would silently merge an exterior and interior shell into one, -//! so CheckInteriorShellWinding never runs and a same-wound (invalid) cavity -//! would be accepted with its volume wrongly added instead of subtracted. -//! If `metadata.shells` is empty, delegates unchanged (matches -//! BuildSolidModel(surfaces) with no metadata: whatever grouping the -//! boundaries already have is used as-is). -SolidModel BuildSolidModelFromArrowNative(const ArrowNativeBoundaries &boundaries, - const std::vector &vertices, const GeometryMetadata &metadata); - -//! Builds a GeomModel (MultiPolygon Z family) from a padded (solid-count 1, -//! shell-count 1) ArrowNativeBoundaries + a vertex pool. Unlike -//! BuildSolidModelFromArrowNative, GeomModel is not index-based — ring -//! indices are dereferenced and expanded into inline coordinates, not copied. -//! Throws if the padding-dimension invariant doesn't hold (a real -//! multi-shell/multi-solid value was passed where a surface value was -//! expected — call BuildSolidModelFromArrowNative for that) or if an index -//! is out of range. -GeomModel BuildGeomModelFromArrowNative(const ArrowNativeBoundaries &boundaries, const std::vector &vertices); - -} // namespace duckdb_3d diff --git a/src/kernel/arrow_native_import.cpp b/src/kernel/arrow_native_import.cpp deleted file mode 100644 index fc3e38f..0000000 --- a/src/kernel/arrow_native_import.cpp +++ /dev/null @@ -1,249 +0,0 @@ -#include "kernel/arrow_native_import.hpp" -#include "kernel/triangulation.hpp" -#include "kernel/validation.hpp" -#include -#include -#include - -namespace duckdb_3d { - -namespace { - -//! Hash function for Vertex3D — mirrors model_builder.cpp's own combine -//! formula (kept local rather than shared: a private detail of vertex -//! deduplication, not part of either builder's public surface). -struct Vertex3DHash { - size_t operator()(const Vertex3D &v) const { - size_t h1 = std::hash {}(v.x); - size_t h2 = std::hash {}(v.y); - size_t h3 = std::hash {}(v.z); - h1 ^= h2 + 0x9e3779b9 + (h1 << 6) + (h1 >> 2); - h1 ^= h3 + 0x9e3779b9 + (h1 << 6) + (h1 >> 2); - return h1; - } -}; - -} // namespace - -SolidModel BuildSolidModelFromArrowNative(const ArrowNativeBoundaries &boundaries, - const std::vector &vertices) { - SolidModel model; - - model.solid_shell_offsets = boundaries.solid_shell_offsets; - model.shell_face_offsets = boundaries.shell_face_offsets; - model.face_ring_offsets = boundaries.face_ring_offsets; - - // Defensive coordinate-equality dedup, mirroring model_builder.cpp's - // GetOrAddVertex, but resolved lazily as each raw pool index is actually - // referenced by a ring — not precomputed over the whole input pool. This - // is a public SQL-callable ingestion boundary (unlike an internal-only - // path), so two things must hold: (1) a buggy writer that emits - // geometrically duplicate vertices at distinct pool indices must not - // silently produce mismatched edges downstream (ValidateSolidModel - // matches edges by vertex INDEX, so two indices for "the same" point - // would wrongly read as an open/non-manifold edge rather than a shared - // one); (2) a pool entry no ring ever references must never appear in - // model.vertices — it would otherwise silently pollute ComputeBBox (and - // anything else that iterates all model vertices) with a point outside - // the actual geometry. - std::unordered_map vertex_map; - std::unordered_map remap; // raw pool index -> compacted model index - - auto ResolveCompactIndex = [&](uint32_t raw) -> uint32_t { - if (raw >= vertices.size()) { - throw std::runtime_error( - "arrow-native geometry: vertex-pool index out of range (design doc validity invariant)"); - } - auto remap_it = remap.find(raw); - if (remap_it != remap.end()) { - return remap_it->second; - } - const Vertex3D &v = vertices[raw]; - auto vm_it = vertex_map.find(v); - uint32_t compact_idx; - if (vm_it != vertex_map.end()) { - compact_idx = vm_it->second; - } else { - compact_idx = static_cast(model.vertices.size()); - model.vertices.push_back(v); - vertex_map[v] = compact_idx; - } - remap[raw] = compact_idx; - return compact_idx; - }; - - // Walked ring-by-ring (not one flat pass over ring_vertex_indices) so a - // consecutive-duplicate compact index — from a literal repeated raw - // index, or from two distinct raw indices that dedup to the same - // compact vertex — can be skipped per ring, mirroring - // model_builder.cpp's IsConsecutiveDuplicate for the WKB path. Left - // uncollapsed, a same-vertex "edge" is a zero-length self-loop with no - // twin anywhere else in the model, which ValidateSolidModel would - // misreport as an open edge. This can shrink a ring's index count, so - // ring_vertex_offsets is rebuilt here rather than copied from boundaries. - model.ring_vertex_indices.reserve(boundaries.ring_vertex_indices.size()); - model.ring_vertex_offsets.reserve(boundaries.ring_vertex_offsets.size()); - model.ring_vertex_offsets.push_back(0); - if (!boundaries.ring_vertex_offsets.empty()) { - for (size_t r = 0; r + 1 < boundaries.ring_vertex_offsets.size(); r++) { - uint32_t raw_start = boundaries.ring_vertex_offsets[r]; - uint32_t raw_end = boundaries.ring_vertex_offsets[r + 1]; - bool has_prev = false; - uint32_t prev_compact = 0; - for (uint32_t k = raw_start; k < raw_end; k++) { - uint32_t compact_idx = ResolveCompactIndex(boundaries.ring_vertex_indices[k]); - if (has_prev && compact_idx == prev_compact) { - continue; // skip consecutive duplicate - } - model.ring_vertex_indices.push_back(compact_idx); - prev_compact = compact_idx; - has_prev = true; - } - model.ring_vertex_offsets.push_back(static_cast(model.ring_vertex_indices.size())); - } - } - - uint32_t face_count = model.FaceCount(); - model.face_triangle_offsets.resize(face_count + 1, 0); - - model.ComputeBBox(); - TriangulateSolidModel(model); - ValidateSolidModel(model); - - return model; -} - -SolidModel BuildSolidModelFromArrowNative(const ArrowNativeBoundaries &boundaries, - const std::vector &vertices, const GeometryMetadata &metadata) { - // No shells metadata: use the boundaries' own physical shell grouping - // as-is (matches model_builder.cpp's BuildSolidModel(surfaces) with no - // metadata). - if (metadata.shells.empty()) { - return BuildSolidModelFromArrowNative(boundaries, vertices); - } - - uint32_t solid_count = - boundaries.solid_shell_offsets.empty() ? 0 : static_cast(boundaries.solid_shell_offsets.size() - 1); - if (metadata.shells.size() != solid_count) { - throw std::runtime_error("arrow-native geometry_properties: shells solid count (" + - std::to_string(metadata.shells.size()) + ") does not match boundaries solid count (" + - std::to_string(solid_count) + ")"); - } - - // Regroup: a real producer (confirmed: cityparquet-rs's arrow_geom_write.rs) - // always pads each solid to exactly one physical shell, flattening real - // interior shells into a single face list exactly like the WKB path - // flattens them into one PolyhedralSurface — the real per-solid shell - // partition lives only in geometry_properties.shells. face_ring_offsets/ - // ring_vertex_offsets/ring_vertex_indices are untouched: regrouping only - // reinterprets which shell boundary markers apply to the SAME faces, - // never reorders them. Mirrors model_builder.cpp's BuildSolidModel(surfaces, - // metadata) exactly: 64-bit sum to avoid overflow tricks, a 0 entry is a - // fully-dropped shell creating no shell, at least one non-empty shell - // required per solid. - ArrowNativeBoundaries regrouped; - regrouped.face_ring_offsets = boundaries.face_ring_offsets; - regrouped.ring_vertex_offsets = boundaries.ring_vertex_offsets; - regrouped.ring_vertex_indices = boundaries.ring_vertex_indices; - regrouped.solid_shell_offsets.push_back(0); - regrouped.shell_face_offsets.push_back(0); - - uint32_t total_shells = 0; - for (uint32_t solid_idx = 0; solid_idx < solid_count; solid_idx++) { - uint32_t padded_start = boundaries.solid_shell_offsets[solid_idx]; - uint32_t padded_end = boundaries.solid_shell_offsets[solid_idx + 1]; - if (padded_end - padded_start != 1) { - throw std::runtime_error( - "arrow-native geometry: expected exactly one padded shell per solid when " - "geometry_properties.shells is present (a real producer flattens interior shells into it)"); - } - uint32_t face_start = boundaries.shell_face_offsets[padded_start]; - uint32_t face_end = boundaries.shell_face_offsets[padded_start + 1]; - - const auto &shell_counts = metadata.shells[solid_idx]; - uint64_t face_sum = 0; - uint32_t non_empty_shells = 0; - for (auto fc : shell_counts) { - face_sum += fc; - if (fc > 0) { - non_empty_shells++; - } - } - if (face_sum != face_end - face_start) { - throw std::runtime_error("arrow-native geometry_properties: shell face count mismatch for solid " + - std::to_string(solid_idx) + ": shells sum (" + std::to_string(face_sum) + - ") != boundaries face count (" + std::to_string(face_end - face_start) + ")"); - } - if (non_empty_shells == 0) { - throw std::runtime_error("arrow-native geometry_properties: solid " + std::to_string(solid_idx) + - " has no non-empty shell"); - } - - uint32_t cursor = face_start; - for (auto count : shell_counts) { - if (count == 0) { - continue; // dropped shell — contributes no faces and no shell entry - } - cursor += count; - total_shells++; - regrouped.shell_face_offsets.push_back(cursor); - } - regrouped.solid_shell_offsets.push_back(total_shells); - } - - return BuildSolidModelFromArrowNative(regrouped, vertices); -} - -GeomModel BuildGeomModelFromArrowNative(const ArrowNativeBoundaries &boundaries, - const std::vector &vertices) { - // Padding-dimension invariant (design doc): a surface value is a Solid - // value with solid-count and shell-count both padded to 1 — asserted, not - // branched on: callers commit to the surface family before calling this - // function (by calling ST_Geom3DFromArrowNative, not - // ST_3DFromArrowNative). - if (boundaries.solid_shell_offsets.size() != 2) { - throw std::runtime_error("arrow-native geometry: expected a padded (solid-count 1) surface-type value — " - "if this is a real multi-solid value, call BuildSolidModelFromArrowNative instead"); - } - if (boundaries.shell_face_offsets.size() != 2) { - throw std::runtime_error("arrow-native geometry: expected shell-count 1 (padding dimension)"); - } - - GeomModel model; - model.type = GeomType::MultiPolygon; - - uint32_t face_start = boundaries.shell_face_offsets[0]; - uint32_t face_end = boundaries.shell_face_offsets[1]; - - model.part_offsets.push_back(0); - uint32_t total_rings = 0; - - // GeomModel is NOT index-based (unlike SolidModel above) — ring indices - // are dereferenced and expanded into inline coordinates here, not copied. - for (uint32_t face_idx = face_start; face_idx < face_end; face_idx++) { - uint32_t ring_start = boundaries.face_ring_offsets[face_idx]; - uint32_t ring_end = boundaries.face_ring_offsets[face_idx + 1]; - - for (uint32_t ring_idx = ring_start; ring_idx < ring_end; ring_idx++) { - uint32_t idx_start = boundaries.ring_vertex_offsets[ring_idx]; - uint32_t idx_end = boundaries.ring_vertex_offsets[ring_idx + 1]; - - model.ring_offsets.push_back(static_cast(model.vertices.size())); - for (uint32_t k = idx_start; k < idx_end; k++) { - uint32_t raw = boundaries.ring_vertex_indices[k]; - if (raw >= vertices.size()) { - throw std::runtime_error("arrow-native geometry: vertex-pool index out of range"); - } - model.vertices.push_back(vertices[raw]); - } - total_rings++; - } - model.part_offsets.push_back(total_rings); - } - model.ring_offsets.push_back(static_cast(model.vertices.size())); - - model.ComputeBBox(); - return model; -} - -} // namespace duckdb_3d diff --git a/src/three_d_extension.cpp b/src/three_d_extension.cpp index 5d74668..c6930eb 100644 --- a/src/three_d_extension.cpp +++ b/src/three_d_extension.cpp @@ -27,7 +27,6 @@ static void LoadInternal(ExtensionLoader &loader) { RegisterGeomAccessorFunctions(loader, solid_3d_type, geom_3d_type); RegisterDistanceFunctions(loader, geom_3d_type); RegisterTransformFunctions(loader, solid_3d_type, geom_3d_type); - RegisterArrowNativeFunctions(loader, solid_3d_type, geom_3d_type); } void ThreeDExtension::Load(ExtensionLoader &loader) { diff --git a/test/cpp/CMakeLists.txt b/test/cpp/CMakeLists.txt index 138dd1a..f081fd9 100644 --- a/test/cpp/CMakeLists.txt +++ b/test/cpp/CMakeLists.txt @@ -28,8 +28,7 @@ set(KERNEL_SOURCES ${REPO_ROOT}/src/kernel/geom_construct.cpp ${REPO_ROOT}/src/kernel/geom_analysis.cpp ${REPO_ROOT}/src/kernel/geom_serialize.cpp - ${REPO_ROOT}/src/kernel/crs_transform.cpp - ${REPO_ROOT}/src/kernel/arrow_native_import.cpp) + ${REPO_ROOT}/src/kernel/crs_transform.cpp) # Test sources set(TEST_SOURCES @@ -51,8 +50,7 @@ set(TEST_SOURCES test_crs_transform.cpp test_wkb_export.cpp test_inner_shell.cpp - test_triangulation.cpp - test_arrow_native_import.cpp) + test_triangulation.cpp) add_executable(three_d_unit_tests ${TEST_SOURCES} ${KERNEL_SOURCES}) diff --git a/test/cpp/test_arrow_native_import.cpp b/test/cpp/test_arrow_native_import.cpp deleted file mode 100644 index 74cf0eb..0000000 --- a/test/cpp/test_arrow_native_import.cpp +++ /dev/null @@ -1,470 +0,0 @@ -#include "catch.hpp" -#include "kernel/arrow_native_import.hpp" -#include "kernel/measurements.hpp" -#include "kernel/metadata_parser.hpp" -#include "kernel/model_builder.hpp" -#include "kernel/triangulation.hpp" -#include "kernel/validation.hpp" -#include "kernel/wkb_parser.hpp" -#include -#include - -using namespace duckdb_3d; - -// Arrow-native ingestion (arrow-native-type branch): boundaries/vertices arrive -// as DuckDB nested LIST/STRUCT Vectors in three_d_extension.cpp, which flattens -// them into the plain ArrowNativeBoundaries CSR form below before calling into -// this DuckDB-free kernel — these tests exercise only that flattened form, no -// DuckDB dependency needed (test/cpp stays fast/no-DuckDB per this repo's -// existing architecture). - -TEST_CASE("BuildSolidModelFromArrowNative reconstructs a two-shell solid", "[arrow_native_import]") { - // 1 solid, 2 shells, 1 triangular face each, sharing a 6-vertex pool. - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 2}; // 1 solid: 2 shells - boundaries.shell_face_offsets = {0, 1, 2}; // shell 0: 1 face, shell 1: 1 face - boundaries.face_ring_offsets = {0, 1, 2}; // face 0: 1 ring, face 1: 1 ring - boundaries.ring_vertex_offsets = {0, 3, 6}; // ring 0: 3 indices, ring 1: 3 indices - boundaries.ring_vertex_indices = {0, 1, 2, 3, 4, 5}; - - std::vector vertices = {{0, 0, 0}, {1, 0, 0}, {0, 1, 0}, {0, 0, 1}, {1, 0, 1}, {0, 1, 1}}; - - auto model = BuildSolidModelFromArrowNative(boundaries, vertices); - REQUIRE(model.vertices.size() == 6); - REQUIRE(model.SolidCount() == 1); - REQUIRE(model.ShellCount() == 2); - REQUIRE(model.FaceCount() == 2); -} - -TEST_CASE("BuildSolidModelFromArrowNative rejects an out-of-range vertex-pool index", "[arrow_native_import]") { - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; - boundaries.shell_face_offsets = {0, 1}; - boundaries.face_ring_offsets = {0, 1}; - boundaries.ring_vertex_offsets = {0, 3}; - boundaries.ring_vertex_indices = {0, 1, 3}; // only 3 vertices (indices 0..2) exist - - std::vector vertices = {{0, 0, 0}, {1, 0, 0}, {0, 1, 0}}; - - REQUIRE_THROWS_WITH(BuildSolidModelFromArrowNative(boundaries, vertices), Catch::Contains("out of range")); -} - -TEST_CASE("BuildSolidModelFromArrowNative deduplicates coordinate-equal vertices at distinct pool indices", - "[arrow_native_import]") { - // A buggy/naive writer emits the same point {0,0,1} twice, at indices 3 - // and 6 — a real triangulated tetrahedron's two faces sharing that apex, - // each referencing "their own" copy instead of a shared index. Without - // dedup, ValidateSolidModel would see 2 distinct vertex indices for what - // is topologically one shared vertex, and misreport an open edge. - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; - boundaries.shell_face_offsets = {0, 4}; - boundaries.face_ring_offsets = {0, 1, 2, 3, 4}; - boundaries.ring_vertex_offsets = {0, 3, 6, 9, 12}; - boundaries.ring_vertex_indices = { - 0, 2, 1, // base (reversed for outward winding) - 0, 1, 3, // side using apex copy #1 (index 3) - 1, 2, 6, // side using apex copy #2 (index 6) — same coordinates as index 3 - 2, 0, 3, // side using apex copy #1 again - }; - - std::vector vertices = { - {0, 0, 0}, {2, 0, 0}, {1, 2, 0}, // base triangle - {1, 1, 2}, // apex, copy #1 (index 3) - {0, 0, 0}, {2, 0, 0}, // unused filler to make index 6 land on the duplicate apex - {1, 1, 2}, // apex, copy #2 (index 6) — coordinate-identical to index 3 - }; - - auto model = BuildSolidModelFromArrowNative(boundaries, vertices); - REQUIRE(model.vertices.size() == 4); // deduped: 3 base + 1 apex, not 7 - REQUIRE(model.ShellCount() == 1); - REQUIRE(model.FaceCount() == 4); - REQUIRE(model.validation.is_closed); - REQUIRE(model.validation.is_manifold); -} - -TEST_CASE("BuildSolidModelFromArrowNative skips a consecutive-duplicate compact index within a ring", - "[arrow_native_import]") { - // A validated closed/manifold tetrahedron (the same face set - // test_metadata.cpp's MakeTwoShellWKB uses: A=v0,B=v1,C=v2,D=v3, faces - // ACB/ABD/BCD/CAD), but face0's ring carries an extra raw index (4) that - // is coordinate-identical to C (index 2) and placed immediately adjacent - // to it: A,C,C',B instead of A,C,B. Without collapsing this into A,C,B - // (mirroring model_builder.cpp's IsConsecutiveDuplicate for the WKB - // path), the C-to-C' "edge" is a zero-length self-loop with no twin - // anywhere else in the model, and the solid misreports as open. - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; - boundaries.shell_face_offsets = {0, 4}; - boundaries.face_ring_offsets = {0, 1, 2, 3, 4}; - boundaries.ring_vertex_offsets = {0, 4, 7, 10, 13}; - boundaries.ring_vertex_indices = { - 0, 2, 4, 1, // face0 (A,C,C'-dup,B) — the injected consecutive duplicate - 0, 1, 3, // face1 (A,B,D) - 1, 2, 3, // face2 (B,C,D) - 2, 0, 3, // face3 (C,A,D) - }; - - std::vector vertices = { - {0, 0, 0}, {2, 0, 0}, {1, 2, 0}, {1, 1, 2}, // A, B, C, D - {1, 2, 0}, // C' — coordinate-identical to C (index 2) - }; - - auto model = BuildSolidModelFromArrowNative(boundaries, vertices); - REQUIRE(model.ring_vertex_indices.size() == 12); // 13 raw indices, 1 consecutive duplicate removed - REQUIRE(model.validation.is_closed); - REQUIRE(model.validation.is_manifold); -} - -TEST_CASE("BuildSolidModelFromArrowNative excludes an unreferenced pool vertex from the model", - "[arrow_native_import]") { - // The vertex pool carries a 4th entry (index 3) that no ring ever - // references. It must not appear in model.vertices at all — in - // particular it must not pollute ComputeBBox with a point far outside - // the actual triangle's extent. - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; - boundaries.shell_face_offsets = {0, 1}; - boundaries.face_ring_offsets = {0, 1}; - boundaries.ring_vertex_offsets = {0, 3}; - boundaries.ring_vertex_indices = {0, 1, 2}; // never references index 3 - - std::vector vertices = { - {0, 0, 0}, - {1, 0, 0}, - {0, 1, 0}, // the referenced triangle - {1000, 1000, 1000}, // unreferenced — must be excluded - }; - - auto model = BuildSolidModelFromArrowNative(boundaries, vertices); - REQUIRE(model.vertices.size() == 3); - REQUIRE(model.bbox.max_x == 1.0); - REQUIRE(model.bbox.max_y == 1.0); - REQUIRE(model.bbox.max_z == 0.0); -} - -TEST_CASE("BuildSolidModelFromArrowNative flags a degenerate (zero-length) ring, not a crash", - "[arrow_native_import]") { - // A face whose only ring has 0 indices (offset delta 0) — passes straight - // through to the same TriangulateSolidModel/ValidateSolidModel the WKB - // path already uses, so it must be flagged as degenerate exactly like a - // degenerate WKB ring is (DESIGN_DOC.md §8.1), not throw or crash here. - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; - boundaries.shell_face_offsets = {0, 1}; - boundaries.face_ring_offsets = {0, 1}; - boundaries.ring_vertex_offsets = {0, 0}; // one ring, zero vertices - boundaries.ring_vertex_indices = {}; - - std::vector vertices = {{0, 0, 0}, {1, 0, 0}, {0, 1, 0}}; - - auto model = BuildSolidModelFromArrowNative(boundaries, vertices); - REQUIRE(model.FaceCount() == 1); - REQUIRE(model.validation.degenerate_face_count >= 1); -} - -TEST_CASE("BuildGeomModelFromArrowNative strips padding and dereferences into inline coordinates", - "[arrow_native_import]") { - // Same physical shape as a Solid with 1 shell, 1 face, but semantically a - // MultiSurface (single triangular surface) — the padding dimensions - // (solid-count 1, shell-count 1) carry no meaning here (design doc). - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; // padding: exactly 1 solid - boundaries.shell_face_offsets = {0, 1}; // padding: exactly 1 shell - boundaries.face_ring_offsets = {0, 1}; // 1 face: 1 ring - boundaries.ring_vertex_offsets = {0, 3}; // 1 ring: 3 indices - boundaries.ring_vertex_indices = {0, 1, 2}; - - std::vector vertices = {{0, 0, 0}, {1, 0, 0}, {0, 1, 0}}; - - auto model = BuildGeomModelFromArrowNative(boundaries, vertices); - REQUIRE(model.type == GeomType::MultiPolygon); - REQUIRE(model.vertices.size() == 3); // GeomModel is NOT index-based — inline coordinates - REQUIRE(model.part_offsets.size() == 2); // 1 part (this face) -> part_offsets = [0, 1] - REQUIRE(model.ring_offsets.size() == 2); // 1 ring -> ring_offsets = [0, 3] - REQUIRE(model.vertices[0] == Vertex3D {0, 0, 0}); - REQUIRE(model.vertices[1] == Vertex3D {1, 0, 0}); - REQUIRE(model.vertices[2] == Vertex3D {0, 1, 0}); -} - -TEST_CASE("BuildGeomModelFromArrowNative rejects a non-padded (real multi-shell) value", "[arrow_native_import]") { - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 2}; // 2 shells -> not a padded surface value - boundaries.shell_face_offsets = {0, 1, 2}; - boundaries.face_ring_offsets = {0, 1, 2}; - boundaries.ring_vertex_offsets = {0, 3, 6}; - boundaries.ring_vertex_indices = {0, 1, 2, 0, 1, 2}; - - std::vector vertices = {{0, 0, 0}, {1, 0, 0}, {0, 1, 0}}; - - REQUIRE_THROWS_WITH(BuildGeomModelFromArrowNative(boundaries, vertices), Catch::Contains("padding")); -} - -TEST_CASE("BuildGeomModelFromArrowNative rejects an out-of-range vertex-pool index", "[arrow_native_import]") { - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; - boundaries.shell_face_offsets = {0, 1}; - boundaries.face_ring_offsets = {0, 1}; - boundaries.ring_vertex_offsets = {0, 3}; - boundaries.ring_vertex_indices = {0, 1, 3}; // only 3 vertices (indices 0..2) exist - - std::vector vertices = {{0, 0, 0}, {1, 0, 0}, {0, 1, 0}}; - - REQUIRE_THROWS_WITH(BuildGeomModelFromArrowNative(boundaries, vertices), Catch::Contains("out of range")); -} - -namespace { - -// Cross-encoding parity fixture: the same hollow-cube topology -// test_inner_shell.cpp's WKB-path tests already pin (outer cube [0,4]^3, -// inner cube [1,3]^3, 6 quad faces per shell, inner wound opposite the -// outer), built two ways — once as WKB bytes (this repo's existing ground -// truth: volume 56, surface area 120) and once as already-flattened -// arrow-native boundaries/vertices sharing one 16-vertex pool (indices 0-7 -// outer corners, 8-15 inner corners) — to prove the two ingestion paths -// agree on identical input geometry, not just each in isolation. - -class WKBBuilder { -public: - std::vector buffer; - void u8(uint8_t v) { - buffer.push_back(v); - } - void u32(uint32_t v) { - buffer.push_back(v & 0xFF); - buffer.push_back((v >> 8) & 0xFF); - buffer.push_back((v >> 16) & 0xFF); - buffer.push_back((v >> 24) & 0xFF); - } - void f64(double v) { - uint8_t b[8]; - std::memcpy(b, &v, 8); - buffer.insert(buffer.end(), b, b + 8); - } - void byteOrder() { - u8(1); - } - void geomType(WKBGeometryType t) { - u32(static_cast(t)); - } - void polyHeader(uint32_t num_rings) { - byteOrder(); - geomType(WKBGeometryType::PolygonZ); - u32(num_rings); - } - void ring(const std::vector &pts) { - u32(static_cast(pts.size()) + 1); - for (auto &p : pts) { - f64(p.x); - f64(p.y); - f64(p.z); - } - f64(pts[0].x); - f64(pts[0].y); - f64(pts[0].z); - } -}; - -//! Append the six faces of an axis-aligned cube [lo,hi]^3 — identical corner -//! ordering to three_d_extension.cpp's WkbCubeFaces() and -//! test_inner_shell.cpp's AppendCubeFaces(). -void AppendCubeFaces(WKBBuilder &b, double lo, double hi, bool reversed) { - Vertex3D v000 = {lo, lo, lo}, v100 = {hi, lo, lo}, v110 = {hi, hi, lo}, v010 = {lo, hi, lo}; - Vertex3D v001 = {lo, lo, hi}, v101 = {hi, lo, hi}, v111 = {hi, hi, hi}, v011 = {lo, hi, hi}; - auto face = [&](std::vector r) { - if (reversed) { - std::reverse(r.begin(), r.end()); - } - b.polyHeader(1); - b.ring(r); - }; - face({v000, v010, v110, v100}); - face({v001, v101, v111, v011}); - face({v000, v100, v101, v001}); - face({v010, v011, v111, v110}); - face({v000, v001, v011, v010}); - face({v100, v110, v111, v101}); -} - -SolidModel BuildHollowCubeFromWKB() { - WKBBuilder b; - b.byteOrder(); - b.geomType(WKBGeometryType::PolyhedralSurfaceZ); - b.u32(12); - AppendCubeFaces(b, 0.0, 4.0, /*reversed=*/false); // outer shell, faces 0..5 - AppendCubeFaces(b, 1.0, 3.0, /*reversed=*/true); // inner shell, faces 6..11 (opposite winding) - - auto surfaces = ParseWKB(b.buffer.data(), b.buffer.size()); - GeometryMetadata meta; - meta.type = "Solid"; - meta.shells = {{6, 6}}; - auto model = BuildSolidModel(surfaces, meta); - TriangulateSolidModel(model); - ValidateSolidModel(model); - return model; -} - -SolidModel BuildHollowCubeFromArrowNative() { - // Shared 16-vertex pool: outer corners at indices 0-7, inner at 8-15 — - // same corner-to-coordinate assignment AppendCubeFaces()/WkbCubeFaces() - // use (v000,v100,v110,v010,v001,v101,v111,v011 in that order). - std::vector vertices = { - {0, 0, 0}, {4, 0, 0}, {4, 4, 0}, {0, 4, 0}, {0, 0, 4}, {4, 0, 4}, {4, 4, 4}, {0, 4, 4}, - {1, 1, 1}, {3, 1, 1}, {3, 3, 1}, {1, 3, 1}, {1, 1, 3}, {3, 1, 3}, {3, 3, 3}, {1, 3, 3}, - }; - - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 2}; // 1 solid: 2 shells - boundaries.shell_face_offsets = {0, 6, 12}; // 6 faces per shell - boundaries.face_ring_offsets.resize(13); - for (int i = 0; i <= 12; i++) { - boundaries.face_ring_offsets[i] = static_cast(i); // 1 ring per face - } - boundaries.ring_vertex_offsets.resize(13); - for (int i = 0; i <= 12; i++) { - boundaries.ring_vertex_offsets[i] = static_cast(i * 4); // 4 indices per ring - } - - // Outer shell (outward winding, matching AppendCubeFaces(reversed=false)): - // bottom, top, front, back, left, right — corners v000=0,v100=1,v110=2, - // v010=3,v001=4,v101=5,v111=6,v011=7. - std::vector outer = { - 0, 3, 2, 1, // bottom: v000,v010,v110,v100 - 4, 5, 6, 7, // top: v001,v101,v111,v011 - 0, 1, 5, 4, // front: v000,v100,v101,v001 - 3, 7, 6, 2, // back: v010,v011,v111,v110 - 0, 4, 7, 3, // left: v000,v001,v011,v010 - 1, 2, 6, 5, // right: v100,v110,v111,v101 - }; - // Inner shell: same face order, indices offset by 8, each ring reversed - // (opposite winding, matching AppendCubeFaces(reversed=true)). - std::vector inner = { - 9, 10, 11, 8, // bottom reversed - 15, 14, 13, 12, // top reversed - 12, 13, 9, 8, // front reversed - 10, 14, 15, 11, // back reversed - 11, 15, 12, 8, // left reversed - 13, 14, 10, 9, // right reversed - }; - - boundaries.ring_vertex_indices.reserve(96); - boundaries.ring_vertex_indices.insert(boundaries.ring_vertex_indices.end(), outer.begin(), outer.end()); - boundaries.ring_vertex_indices.insert(boundaries.ring_vertex_indices.end(), inner.begin(), inner.end()); - - return BuildSolidModelFromArrowNative(boundaries, vertices); -} - -} // namespace - -TEST_CASE("Arrow-native and WKB ingestion agree on the hollow-cube fixture", "[arrow_native_import][parity]") { - auto wkb_model = BuildHollowCubeFromWKB(); - auto arrow_model = BuildHollowCubeFromArrowNative(); - - REQUIRE(arrow_model.SolidCount() == wkb_model.SolidCount()); - REQUIRE(arrow_model.ShellCount() == wkb_model.ShellCount()); - REQUIRE(arrow_model.FaceCount() == wkb_model.FaceCount()); - REQUIRE(arrow_model.validation.is_closed == wkb_model.validation.is_closed); - REQUIRE(arrow_model.validation.is_manifold == wkb_model.validation.is_manifold); - REQUIRE(arrow_model.validation.is_oriented == wkb_model.validation.is_oriented); - REQUIRE(ComputeVolume(arrow_model) == Approx(ComputeVolume(wkb_model)).epsilon(1e-12)); - REQUIRE(ComputeSurfaceArea(arrow_model) == Approx(ComputeSurfaceArea(wkb_model)).epsilon(1e-12)); - REQUIRE(ComputeVolume(arrow_model) == Approx(56.0).epsilon(1e-12)); - REQUIRE(ComputeSurfaceArea(arrow_model) == Approx(120.0).epsilon(1e-12)); -} - -namespace { - -//! Same hollow-cube topology, but shaped exactly as a real producer -//! (cityparquet-rs's arrow_geom_write.rs, confirmed by reading it) actually -//! emits it: the solid padded to exactly ONE physical shell holding all 12 -//! faces flattened together, with the real 2-shell partition recoverable -//! only from geometry_properties.shells — never from the boundaries' own -//! "shell" nesting level. -SolidModel BuildPaddedHollowCubeFromArrowNative() { - std::vector vertices = { - {0, 0, 0}, {4, 0, 0}, {4, 4, 0}, {0, 4, 0}, {0, 0, 4}, {4, 0, 4}, {4, 4, 4}, {0, 4, 4}, - {1, 1, 1}, {3, 1, 1}, {3, 3, 1}, {1, 3, 1}, {1, 1, 3}, {3, 1, 3}, {3, 3, 3}, {1, 3, 3}, - }; - - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; // padded: exactly 1 physical shell - boundaries.shell_face_offsets = {0, 12}; // all 12 faces flattened into it - boundaries.face_ring_offsets.resize(13); - for (int i = 0; i <= 12; i++) { - boundaries.face_ring_offsets[i] = static_cast(i); - } - boundaries.ring_vertex_offsets.resize(13); - for (int i = 0; i <= 12; i++) { - boundaries.ring_vertex_offsets[i] = static_cast(i * 4); - } - - std::vector outer = { - 0, 3, 2, 1, 4, 5, 6, 7, 0, 1, 5, 4, 3, 7, 6, 2, 0, 4, 7, 3, 1, 2, 6, 5, - }; - std::vector inner = { - 9, 10, 11, 8, 15, 14, 13, 12, 12, 13, 9, 8, 10, 14, 15, 11, 11, 15, 12, 8, 13, 14, 10, 9, - }; - boundaries.ring_vertex_indices.reserve(96); - boundaries.ring_vertex_indices.insert(boundaries.ring_vertex_indices.end(), outer.begin(), outer.end()); - boundaries.ring_vertex_indices.insert(boundaries.ring_vertex_indices.end(), inner.begin(), inner.end()); - - GeometryMetadata metadata; - metadata.type = "Solid"; - metadata.shells = {{6, 6}}; // the real per-solid shell partition - - return BuildSolidModelFromArrowNative(boundaries, vertices, metadata); -} - -} // namespace - -TEST_CASE("BuildSolidModelFromArrowNative regroups shells from geometry_properties.shells " - "when the boundaries are padded to one physical shell", - "[arrow_native_import]") { - auto model = BuildPaddedHollowCubeFromArrowNative(); - REQUIRE(model.SolidCount() == 1); - REQUIRE(model.ShellCount() == 2); // recovered from metadata, not the padded boundaries - REQUIRE(model.FaceCount() == 12); - REQUIRE(model.validation.is_closed); - REQUIRE(model.validation.is_manifold); - REQUIRE(model.validation.is_oriented); // CheckInteriorShellWinding actually ran - REQUIRE(ComputeVolume(model) == Approx(56.0).epsilon(1e-12)); - REQUIRE(ComputeSurfaceArea(model) == Approx(120.0).epsilon(1e-12)); -} - -TEST_CASE("BuildSolidModelFromArrowNative with empty shells metadata keeps the boundaries' own grouping", - "[arrow_native_import]") { - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 2}; - boundaries.shell_face_offsets = {0, 1, 2}; - boundaries.face_ring_offsets = {0, 1, 2}; - boundaries.ring_vertex_offsets = {0, 3, 6}; - boundaries.ring_vertex_indices = {0, 1, 2, 3, 4, 5}; - std::vector vertices = {{0, 0, 0}, {1, 0, 0}, {0, 1, 0}, {0, 0, 1}, {1, 0, 1}, {0, 1, 1}}; - - GeometryMetadata metadata; // shells left empty - auto model = BuildSolidModelFromArrowNative(boundaries, vertices, metadata); - REQUIRE(model.ShellCount() == 2); -} - -TEST_CASE("BuildSolidModelFromArrowNative rejects a shells/boundaries face count mismatch", "[arrow_native_import]") { - ArrowNativeBoundaries boundaries; - boundaries.solid_shell_offsets = {0, 1}; - boundaries.shell_face_offsets = {0, 12}; // 12 faces, padded - boundaries.face_ring_offsets.resize(13); - for (int i = 0; i <= 12; i++) { - boundaries.face_ring_offsets[i] = static_cast(i); - } - boundaries.ring_vertex_offsets.resize(13); - for (int i = 0; i <= 12; i++) { - boundaries.ring_vertex_offsets[i] = static_cast(i * 3); - } - boundaries.ring_vertex_indices.assign(36, 0); - - GeometryMetadata metadata; - metadata.type = "Solid"; - metadata.shells = {{6, 5}}; // sums to 11, not 12 - - REQUIRE_THROWS_WITH( - BuildSolidModelFromArrowNative(boundaries, std::vector(1, Vertex3D {0, 0, 0}), metadata), - Catch::Contains("face count")); -} diff --git a/test/sql/st_3d_from_arrow_native.test b/test/sql/st_3d_from_arrow_native.test deleted file mode 100644 index 7cc04b0..0000000 --- a/test/sql/st_3d_from_arrow_native.test +++ /dev/null @@ -1,258 +0,0 @@ -# name: test/sql/st_3d_from_arrow_native.test -# description: ST_3DFromArrowNative / ST_3DTryFromArrowNative ingest nested-List/Struct solids, dispatching per-row on geometry_properties.type -# group: [sql] - -require three_d - -require-env THREE_D_TEST_FIXTURES - -# Fixture: 1 solid, 2 shells, 1 triangular face each (mirrors -# test/cpp/test_arrow_native_import.cpp's basic two-shell case) — not a -# closed/valid solid, just enough to prove the ingestion pipeline binds and -# reconstructs the right topology end to end. -query III -SELECT ST_3DNumSolids(s), ST_3DNumShells(s), ST_3DNumFaces(s) -FROM ( - SELECT ST_3DFromArrowNative( - [[[[ [0,1,2] ]], [[ [3,4,5] ]]]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}, - {'x':0.0,'y':0.0,'z':1.0}, {'x':1.0,'y':0.0,'z':1.0}, {'x':0.0,'y':1.0,'z':1.0}], - '{"type": "Solid"}' - ) AS s -) q; ----- -1 2 2 - -# Fixture: the hollow-cube topology (outer cube [0,4]^3, inner cube [1,3]^3, -# 6 quad faces per shell, opposite winding) — the exact same corner ordering -# WkbCubeFaces() (three_d_extension.cpp) uses for ST_AsWKBHollowCube(), -# expressed as arrow-native boundaries/vertices (a shared 16-vertex pool, -# indices 0-7 outer corners, 8-15 inner corners) instead of WKB, to prove -# ST_3DVolume/ST_3DSurfaceArea agree with the WKB path's known values -# (56.0 / 120.0 — see ST_AsWKBHollowCube's own doc comment). -query II -SELECT ST_3DVolume(s), ST_3DSurfaceArea(s) -FROM ( - SELECT ST_3DFromArrowNative( - [[ - [ [[0,1,2,3]], [[4,5,6,7]], [[0,3,5,4]], [[1,7,6,2]], [[0,4,7,1]], [[3,2,6,5]] ], - [ [[11,10,9,8]], [[15,14,13,12]], [[12,13,11,8]], [[10,14,15,9]], [[9,15,12,8]], [[13,14,10,11]] ] - ]], - [{'x':0,'y':0,'z':0}, {'x':0,'y':4,'z':0}, {'x':4,'y':4,'z':0}, {'x':4,'y':0,'z':0}, - {'x':0,'y':0,'z':4}, {'x':4,'y':0,'z':4}, {'x':4,'y':4,'z':4}, {'x':0,'y':4,'z':4}, - {'x':1,'y':1,'z':1}, {'x':1,'y':3,'z':1}, {'x':3,'y':3,'z':1}, {'x':3,'y':1,'z':1}, - {'x':1,'y':1,'z':3}, {'x':3,'y':1,'z':3}, {'x':3,'y':3,'z':3}, {'x':1,'y':3,'z':3}], - '{"type": "Solid"}' - ) AS s -) q; ----- -56.0 120.0 - -# NULL propagation: NULL boundaries, vertices, or geometry_properties -> NULL result. -query I -SELECT ST_3DFromArrowNative(NULL, [{'x':0.0,'y':0.0,'z':0.0}], '{"type": "Solid"}') IS NULL; ----- -true - -query I -SELECT ST_3DFromArrowNative([[[[[0,1,2]]]]], NULL, '{"type": "Solid"}') IS NULL; ----- -true - -query I -SELECT ST_3DFromArrowNative([[[[[0,1,2]]]]], [{'x':0.0,'y':0.0,'z':0.0}], NULL) IS NULL; ----- -true - -# ST_3DTryFromArrowNative: out-of-range vertex-pool index returns NULL rather -# than raising (only 1 vertex provided, index 2 is out of range). -query I -SELECT ST_3DTryFromArrowNative([[[[[0,1,2]]]]], [{'x':0.0,'y':0.0,'z':0.0}], '{"type": "Solid"}') IS NULL; ----- -true - -# ST_3DFromArrowNative: the same out-of-range case raises in non-TRY mode. -statement error -SELECT ST_3DFromArrowNative([[[[[0,1,2]]]]], [{'x':0.0,'y':0.0,'z':0.0}], '{"type": "Solid"}'); ----- -out of range - -# ────────────────────────────────────────────────────────────── -# Per-row type dispatch (design doc "critical invariant": dispatch on -# geometry_properties.type, never on physical shape — a single column may -# mix Solid-family and surface-family rows sharing the identical padded -# shape). ST_3DFromArrowNative must reject a surface-family row even though -# its physical shape (solid-count 1, shell-count 1) looks like a valid -# degenerate solid. -# ────────────────────────────────────────────────────────────── - -# A MultiSurface-typed row rejected by ST_3DTryFromArrowNative (NULL), even -# though the padded shape is physically identical to a valid single-shell Solid. -query I -SELECT ST_3DTryFromArrowNative( - [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "MultiSurface"}' -) IS NULL; ----- -true - -statement error -SELECT ST_3DFromArrowNative( - [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "MultiSurface"}' -); ----- -not a solid-family type - -# MultiSolid/CompositeSolid are also accepted (solid-family, not just "Solid"). -query I -SELECT ST_3DTryFromArrowNative( - [[[[ [0,1,2] ]], [[ [3,4,5] ]]]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}, - {'x':0.0,'y':0.0,'z':1.0}, {'x':1.0,'y':0.0,'z':1.0}, {'x':0.0,'y':1.0,'z':1.0}], - '{"type": "MultiSolid"}' -) IS NOT NULL; ----- -true - -# The actual mixed-column proof: one query, two rows sharing the identical -# padded (solid-count 1, shell-count 1) physical shape, differing only in -# geometry_properties.type — ST_3DFromArrowNative must ingest the Solid row -# and reject the MultiSurface row, in the same SELECT. -query II -SELECT id, ST_3DTryFromArrowNative(boundaries, vertices, geometry_properties) IS NOT NULL AS ingested -FROM ( - VALUES - (1, [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "Solid"}'), - (2, [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "MultiSurface"}') -) t(id, boundaries, vertices, geometry_properties) -ORDER BY id; ----- -1 true -2 false - -# ────────────────────────────────────────────────────────────── -# Nullability invariants (design doc: "within a non-null geometry cell, no -# nested list element is itself null — every solid/shell/face/ring entry and -# every vertex index is present"; "no Struct entry is null and none of -# x/y/z is null within a present entry"). A NULL nested element must be -# rejected deterministically (NULL in TRY mode, a stable error otherwise) — -# not silently reinterpreted as an empty ring, and not dependent on whatever -# unspecified bytes happen to sit behind the null slot. -# ────────────────────────────────────────────────────────────── - -# A NULL ring (shell 0's only face's only ring) is rejected, not silently -# treated as an empty ring. -query I -SELECT ST_3DTryFromArrowNative( - [[[[NULL]], [[ [3,4,5] ]]]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}, - {'x':0.0,'y':0.0,'z':1.0}, {'x':1.0,'y':0.0,'z':1.0}, {'x':0.0,'y':1.0,'z':1.0}], - '{"type": "MultiSolid"}' -) IS NULL; ----- -true - -statement error -SELECT ST_3DFromArrowNative( - [[[[NULL]], [[ [3,4,5] ]]]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}, - {'x':0.0,'y':0.0,'z':1.0}, {'x':1.0,'y':0.0,'z':1.0}, {'x':0.0,'y':1.0,'z':1.0}], - '{"type": "MultiSolid"}' -); ----- -null - -# A NULL x coordinate in the vertex pool is rejected, not read as garbage. -query I -SELECT ST_3DTryFromArrowNative( - [[[[ [0,1,2] ]]]], - [{'x': NULL, 'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "Solid"}' -) IS NULL; ----- -true - -statement error -SELECT ST_3DFromArrowNative( - [[[[ [0,1,2] ]]]], - [{'x': NULL, 'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "Solid"}' -); ----- -null - -# ────────────────────────────────────────────────────────────── -# STRUCT geometry_properties overload (cityparquet-rs's real -# geometry_properties_lod* column type — confirmed STRUCT, same as its WKB -# rows) must bind directly, without an explicit to_json() cast. -# ────────────────────────────────────────────────────────────── - -query III -SELECT ST_3DNumSolids(s), ST_3DNumShells(s), ST_3DNumFaces(s) -FROM ( - SELECT ST_3DFromArrowNative( - [[[[ [0,1,2] ]], [[ [3,4,5] ]]]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}, - {'x':0.0,'y':0.0,'z':1.0}, {'x':1.0,'y':0.0,'z':1.0}, {'x':0.0,'y':1.0,'z':1.0}], - {'type': 'Solid', 'surfaces': NULL, 'face_semantics': NULL, 'shells': NULL} - ) AS s -) q; ----- -1 2 2 - -# ────────────────────────────────────────────────────────────── -# Shells regrouping (design doc + confirmed cityparquet-rs behavior): -# real producers pad a Solid's boundaries to exactly one physical shell, -# flattening interior shells into geometry_properties.shells — the same -# hollow-cube fixture as test/cpp's parity test, expressed in that padded -# shape, must still recover 2 shells / volume 56 / area 120, and the winding -# check must actually run (proven by st_3d_hollow_solid.test's WKB-side -# sibling case, not re-litigated here — this only proves shell recovery). -# ────────────────────────────────────────────────────────────── - -query III -SELECT ST_3DNumShells(s), ROUND(ST_3DVolume(s), 6), ROUND(ST_3DSurfaceArea(s), 6) -FROM ( - SELECT ST_3DFromArrowNative( - [[ - [ [[0,1,2,3]], [[4,5,6,7]], [[0,3,5,4]], [[1,7,6,2]], [[0,4,7,1]], [[3,2,6,5]], - [[11,10,9,8]], [[15,14,13,12]], [[12,13,11,8]], [[10,14,15,9]], [[9,15,12,8]], [[13,14,10,11]] ] - ]], - [{'x':0,'y':0,'z':0}, {'x':0,'y':4,'z':0}, {'x':4,'y':4,'z':0}, {'x':4,'y':0,'z':0}, - {'x':0,'y':0,'z':4}, {'x':4,'y':0,'z':4}, {'x':4,'y':4,'z':4}, {'x':0,'y':4,'z':4}, - {'x':1,'y':1,'z':1}, {'x':1,'y':3,'z':1}, {'x':3,'y':3,'z':1}, {'x':3,'y':1,'z':1}, - {'x':1,'y':1,'z':3}, {'x':3,'y':1,'z':3}, {'x':3,'y':3,'z':3}, {'x':1,'y':3,'z':3}], - '{"type": "Solid", "shells": [[6, 6]]}' - ) AS s -) q; ----- -2 56.0 120.0 - -# The same regrouping through the STRUCT geometry_properties overload, whose -# `shells` child binds as INTEGER[][] — the arm of struct_metadata.cpp's -# per-row face-count reader that the (BLOB, ANY) HUGEINT[][] normalisation -# never reaches, and the JSON-text case above bypasses entirely. -query II -SELECT ST_3DNumShells(s), ST_3DNumFaces(s) -FROM ( - SELECT ST_3DFromArrowNative( - [[ - [ [[0,1,2,3]], [[4,5,6,7]], [[0,3,5,4]], [[1,7,6,2]], [[0,4,7,1]], [[3,2,6,5]], - [[11,10,9,8]], [[15,14,13,12]], [[12,13,11,8]], [[10,14,15,9]], [[9,15,12,8]], [[13,14,10,11]] ] - ]], - [{'x':0,'y':0,'z':0}, {'x':0,'y':4,'z':0}, {'x':4,'y':4,'z':0}, {'x':4,'y':0,'z':0}, - {'x':0,'y':0,'z':4}, {'x':4,'y':0,'z':4}, {'x':4,'y':4,'z':4}, {'x':0,'y':4,'z':4}, - {'x':1,'y':1,'z':1}, {'x':1,'y':3,'z':1}, {'x':3,'y':3,'z':1}, {'x':3,'y':1,'z':1}, - {'x':1,'y':1,'z':3}, {'x':3,'y':1,'z':3}, {'x':3,'y':3,'z':3}, {'x':1,'y':3,'z':3}], - {'type': 'Solid', 'surfaces': NULL, 'face_semantics': NULL, 'shells': [[6, 6]]} - ) AS s -) q; ----- -2 12 diff --git a/test/sql/st_3d_metadata.test b/test/sql/st_3d_metadata.test index 48a53dd..99ca0f0 100644 --- a/test/sql/st_3d_metadata.test +++ b/test/sql/st_3d_metadata.test @@ -60,8 +60,7 @@ SELECT ST_3DTryFromWKB(NULL, '{}') IS NULL; true # ────────────────────────────────────────────────────────────── -# STRUCT geometry_properties overload (Task 1, arrow-native-type): -# duckdb-cityjson's arrow-native-type branch (commit d334b26) types +# STRUCT geometry_properties overload: CityParquet types # geometry_properties_lod* as STRUCT("type" VARCHAR, surfaces JSON, # face_semantics INTEGER[], shells INTEGER[][]), not VARCHAR JSON text. # ST_3DFromWKB/ST_3DTryFromWKB must bind directly against that STRUCT diff --git a/test/sql/st_geom3d_from_arrow_native.test b/test/sql/st_geom3d_from_arrow_native.test deleted file mode 100644 index dd68432..0000000 --- a/test/sql/st_geom3d_from_arrow_native.test +++ /dev/null @@ -1,154 +0,0 @@ -# name: test/sql/st_geom3d_from_arrow_native.test -# description: ST_Geom3DFromArrowNative / ST_Geom3DTryFromArrowNative ingest a surface-family nested-List/Struct value, dispatching per-row on geometry_properties.type -# group: [sql] - -require three_d - -# Fixture: a single triangle (MultiSurface), the same physical shape as a -# padded Solid (solid-count 1, shell-count 1) — mirrors -# test/cpp/test_arrow_native_import.cpp's basic GeomModel case. -query II -SELECT ST_3DGeometryType(g), ST_3DNumGeometries(g) -FROM ( - SELECT ST_Geom3DFromArrowNative( - [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "MultiSurface"}' - ) AS g -) q; ----- -ST_MultiPolygon 1 - -# Area of the single triangular face: 0.5 * |(1,0,0) x (0,1,0)| = 0.5. -query I -SELECT ST_3DFootprintArea(ST_Geom3DFromArrowNative( - [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "MultiSurface"}' -)); ----- -0.5 - -# CompositeSurface is also accepted (surface-family, not just "MultiSurface"). -query I -SELECT ST_Geom3DTryFromArrowNative( - [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "CompositeSurface"}' -) IS NOT NULL; ----- -true - -# NULL propagation. -query I -SELECT ST_Geom3DFromArrowNative(NULL, [{'x':0.0,'y':0.0,'z':0.0}], '{"type": "MultiSurface"}') IS NULL; ----- -true - -query I -SELECT ST_Geom3DFromArrowNative([[[[[0,1,2]]]]], NULL, '{"type": "MultiSurface"}') IS NULL; ----- -true - -query I -SELECT ST_Geom3DFromArrowNative([[[[[0,1,2]]]]], [{'x':0.0,'y':0.0,'z':0.0}], NULL) IS NULL; ----- -true - -# ST_Geom3DTryFromArrowNative: a non-padded (real 2-shell) value mistakenly -# typed as MultiSurface returns NULL rather than raising (the kernel's own -# padding-dimension assertion catches a producer inconsistency defensively, -# behind the type check). -query I -SELECT ST_Geom3DTryFromArrowNative( - [[[[ [0,1,2] ]], [[ [3,4,5] ]]]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}, - {'x':0.0,'y':0.0,'z':1.0}, {'x':1.0,'y':0.0,'z':1.0}, {'x':0.0,'y':1.0,'z':1.0}], - '{"type": "MultiSurface"}' -) IS NULL; ----- -true - -# ST_Geom3DFromArrowNative: the same non-padded case raises in non-TRY mode. -statement error -SELECT ST_Geom3DFromArrowNative( - [[[[ [0,1,2] ]], [[ [3,4,5] ]]]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}, - {'x':0.0,'y':0.0,'z':1.0}, {'x':1.0,'y':0.0,'z':1.0}, {'x':0.0,'y':1.0,'z':1.0}], - '{"type": "MultiSurface"}' -); ----- -padding - -# ────────────────────────────────────────────────────────────── -# Per-row type dispatch (design doc "critical invariant"): a Solid-typed row -# is rejected by ST_Geom3DFromArrowNative even though its padded shape is -# physically identical to a valid MultiSurface — mirrors -# st_3d_from_arrow_native.test's symmetric case. -# ────────────────────────────────────────────────────────────── - -query I -SELECT ST_Geom3DTryFromArrowNative( - [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "Solid"}' -) IS NULL; ----- -true - -statement error -SELECT ST_Geom3DFromArrowNative( - [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "Solid"}' -); ----- -not a surface-family type - -# The mixed-column proof, mirrored: same physical shape, ST_Geom3DFromArrowNative -# ingests the MultiSurface row and rejects the Solid row, in the same SELECT. -query II -SELECT id, ST_Geom3DTryFromArrowNative(boundaries, vertices, geometry_properties) IS NOT NULL AS ingested -FROM ( - VALUES - (1, [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "MultiSurface"}'), - (2, [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - '{"type": "Solid"}') -) t(id, boundaries, vertices, geometry_properties) -ORDER BY id; ----- -1 true -2 false - -# STRUCT geometry_properties overload (cityparquet-rs's real column type) -# must bind directly, without an explicit to_json() cast. -query I -SELECT ST_3DFootprintArea(ST_Geom3DFromArrowNative( - [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - {'type': 'MultiSurface', 'surfaces': NULL, 'face_semantics': NULL, 'shells': NULL} -)); ----- -0.5 - -# Row-varying STRUCT metadata through the arrow-native surface path: two rows -# (MultiSurface and CompositeSurface) in one chunk, exercising non-constant -# struct/list vectors in the positionally-bound STRUCT overload. -query II -SELECT id, ST_3DGeometryType(ST_Geom3DFromArrowNative(boundaries, vertices, geometry_properties)) AS gtype -FROM ( - VALUES - (1, [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - {'type': 'MultiSurface', 'surfaces': NULL, 'face_semantics': NULL, 'shells': NULL}), - (2, [[ [ [[0,1,2]] ] ]], - [{'x':0.0,'y':0.0,'z':0.0}, {'x':1.0,'y':0.0,'z':0.0}, {'x':0.0,'y':1.0,'z':0.0}], - {'type': 'CompositeSurface', 'surfaces': NULL, 'face_semantics': NULL, 'shells': NULL}) -) t(id, boundaries, vertices, geometry_properties) -ORDER BY id; ----- -1 ST_MultiPolygon -2 ST_MultiPolygon diff --git a/test/sql/wkb_from_geometry.test b/test/sql/wkb_from_geometry.test new file mode 100644 index 0000000..9389714 --- /dev/null +++ b/test/sql/wkb_from_geometry.test @@ -0,0 +1,86 @@ +# name: test/sql/wkb_from_geometry.test +# description: the WKB constructors accept a core GEOMETRY value directly, with no ST_AsWKB bridge +# group: [sql] + +require three_d + +require-env THREE_D_TEST_FIXTURES + +# CityParquet annotates its GeoParquet-legal geometry columns with the Parquet +# GEOMETRY logical type, so DuckDB hands them to a query as GEOMETRY rather than +# BLOB. Reading such a column used to need ST_AsWKB in between. These overloads +# remove that step; the bytes and the answers are identical either way. +# +# Only the footprint constructor has a reachable positive case. DuckDB v1.5.4's +# geometry model has no polyhedral surface, so ST_GeomFromWKB raises +# `Unsupported geometry type in WKB` on solid bytes and a solid cannot become a +# GEOMETRY at all; the kernel in turn takes a GeometryCollection Z only when its +# members are PolyhedralSurface Z. The solid constructors therefore carry the +# overload for the shape of the family and for foreign GeoParquet columns, where +# MultiPolygon Z is common, and are exercised here through their null and error +# contracts. + +# ST_Geom3DFromWKB accepts GEOMETRY. +query I +SELECT ST_3DGeometryType(ST_Geom3DFromWKB(ST_GeomFromWKB(ST_AsWKBPolygonZ()))); +---- +ST_Polygon + +# ...and agrees with the ST_AsWKB bridge byte for byte. +query I +SELECT ST_Geom3DFromWKB(ST_GeomFromWKB(ST_AsWKBPolygonZ())) + = ST_Geom3DFromWKB(ST_AsWKB(ST_GeomFromWKB(ST_AsWKBPolygonZ()))); +---- +true + +# Measurement straight off the GEOMETRY matches the bridged route. +query I +SELECT ST_3DFootprintArea(ST_Geom3DFromWKB(ST_GeomFromWKB(ST_AsWKBPolygonZ()))) + = ST_3DFootprintArea(ST_Geom3DFromWKB(ST_AsWKB(ST_GeomFromWKB(ST_AsWKBPolygonZ())))); +---- +true + +# ST_3DTryFromWKB accepts GEOMETRY and keeps its TRY contract — a lone Polygon Z +# is not a solid, so it yields NULL rather than raising. +query I +SELECT ST_3DTryFromWKB(ST_GeomFromWKB(ST_AsWKBPolygonZ())) IS NULL; +---- +true + +# The strict form binds the same argument and raises on it, as it does for BLOB. +statement error +SELECT ST_3DFromWKB(ST_GeomFromWKB(ST_AsWKBPolygonZ())); +---- +Unsupported WKB geometry type for SOLID_3D import: type code 1003 + +# NULL in, NULL out, on all three. +query III +SELECT ST_Geom3DFromWKB(NULL::GEOMETRY) IS NULL, + ST_3DFromWKB(NULL::GEOMETRY) IS NULL, + ST_3DTryFromWKB(NULL::GEOMETRY) IS NULL; +---- +true true true + +# An untyped NULL still binds. It is castable to both BLOB and GEOMETRY at equal +# cost, so a second overload would have made it ambiguous; the single ANY +# candidate dispatched at bind time keeps it unambiguous. +query III +SELECT ST_Geom3DFromWKB(NULL) IS NULL, + ST_3DFromWKB(NULL) IS NULL, + ST_3DTryFromWKB(NULL) IS NULL; +---- +true true true + +# A GEOMETRY carrying a CRS binds the same way — the overload is declared on the +# bare type, which a CRS-parameterised value still matches. +query I +SELECT ST_3DGeometryType(ST_Geom3DFromWKB(ST_SetCRS(ST_GeomFromWKB(ST_AsWKBPolygonZ()), 'EPSG:7415'))); +---- +ST_Polygon + +# The BLOB overloads are untouched. +query II +SELECT ST_3DVolume(ST_3DFromWKB(ST_AsWKBHollowCube())), + ST_3DGeometryType(ST_Geom3DFromWKB(ST_AsWKBPolygonZ())); +---- +56.0 ST_Polygon