Skip to content

feat(geography): explicit GEOMETRY <-> GEOGRAPHY casts#175

Open
estebanzimanyi wants to merge 9 commits into
MobilityDB:mainfrom
estebanzimanyi:feat/geography-casts
Open

feat(geography): explicit GEOMETRY <-> GEOGRAPHY casts#175
estebanzimanyi wants to merge 9 commits into
MobilityDB:mainfrom
estebanzimanyi:feat/geography-casts

Conversation

@estebanzimanyi
Copy link
Copy Markdown
Member

Summary

Two cast functions for the GEOGRAPHY LogicalType (registered in #169), each reusing the same GSERIALIZED with the geodetic flag toggled:

Cast Mechanism
GEOMETRY -> GEOGRAPHY GeometryToGSerialized lifts the sgl GEOMETRY to a GSERIALIZED; the geodetic flag is set explicitly via MEOS_FLAGS_SET_GEODETIC(gs->gflags, true).
GEOGRAPHY -> GEOMETRY The GEOGRAPHY BLOB is deserialized to GSERIALIZED; the geodetic flag is cleared; GSerializedToGeometry emits sgl GEOMETRY.

SRID handling

DuckDB-Spatial GEOMETRY has no SRID slot, so a round-trip via GEOMETRY drops the SRID prefix. The EWKT / EWKB round-trip (covered by step 1's ST_AsText and ST_AsBinary) preserves SRID.

TGEOGPOINT(GEOGRAPHY, …) is transparent

The existing TGEOGPOINT(GEOMETRY, …) constructor overloads transparently accept GEOGRAPHY arguments via the new implicit cast — no extra explicit overloads are needed.

Step in the GEOGRAPHY roadmap

This is step 2 of 4 in doc/geography-boundary.md:

  1. I/O UDFs (4 functions) — feat(geography): ST_GeogFromText / ST_AsText / ST_AsBinary / ST_GeogFromBinary #174
  2. GEOMETRY <-> GEOGRAPHY casts (this PR) — 2 casts
  3. Operations: ST_Length / ST_Area / eIntersects / nearestApproachDistance over GEOGRAPHY — ~50 LoC
  4. Full test matrix — ~200 LoC

Tests

test/sql/geography.test now covers:

  • Type alias registration (column type, NULL handling)
  • EWKT round-trip on POINT and LINESTRING
  • EWKB round-trip on POINT
  • GEOMETRY -> GEOGRAPHY explicit cast
  • GEOGRAPHY -> GEOMETRY -> GEOGRAPHY round-trip (SRID dropped, value preserved)

Local: 9 assertions pass.

Depends on

Adds doc/geography-boundary.md as the canonical write-up of how
MobilityDuck represents geodetic geography values across the
MEOS<->DuckDB columnar boundary.

Covers:
- The closed-algebra property in MEOS and why it doesn't survive
  the columnar boundary without a dedicated LogicalType.
- The GEOGRAPHY LogicalType registration: a BLOB alias carrying
  MEOS-WKB with the geodetic flag preserved in the type tag, with
  no dependence on a DuckDB upstream change or on a third-party
  duckdb-geography extension.
- The I/O surface (ST_GeogFromText, ST_AsText, ST_AsBinary,
  ST_GeogFromBinary), all thin shims over existing MEOS exports.
- The operation surface (length, area, eIntersects, etc.) — every
  call delegates to a MEOS function that takes geodetic input and
  returns the correct type; DuckDB never sees a non-geodetic
  representation of a geodetic value during a computation.
- The complete inter-type cast matrix (GEOMETRY / GEOGRAPHY /
  TGEOGPOINT / TGEOMPOINT), mirroring the MobilityDB-on-Postgres
  surface.
- TemporalParquet round-trip preservation via the footer JSON's
  base_type / geodetic / srid fields.
- Pitfalls a binding implementation must avoid (using
  ST_GeomFromText to construct a GEOGRAPHY value, reusing DuckDB
  Spatial Cartesian functions on a GEOGRAPHY blob, stripping the
  geodetic flag in Parquet output, etc.).
- Current state of the implementation and the bounded pending
  work (~430 LoC, single PR) to register the LogicalType, the
  I/O UDFs, the casts, and the tests.

README updated with a single-paragraph pointer in the
parity-gaps neighbourhood so adopters land here when looking for
geography semantics on the DuckDB side.
…yDB#168 design)

Foundation for the DuckDB GEOGRAPHY boundary documented in MobilityDB#168.
Registers the LogicalType alias (BLOB carrying MEOS-WKB with the
geodetic flag preserved in the type tag) so DuckDB columns can be
declared as `GEOGRAPHY`.

Files:

- src/include/geo/geography.hpp — GeographyType struct declaration
  (LogicalType getter + RegisterType entry point), mirroring the
  shape of StboxType / TgeogpointType.

- src/geo/geography.cpp — implementation: BLOB with SetAlias
  ("GEOGRAPHY") and loader.RegisterType("GEOGRAPHY", GEOGRAPHY()).

- src/mobilityduck_extension.cpp — Load() wires GeographyType::RegisterType
  after StboxType.

- CMakeLists.txt — adds src/geo/geography.cpp to EXTENSION_SOURCES.

- test/sql/geography.test — CREATE TABLE / INSERT NULL / SELECT
  round-trip sanity test verifying the alias is registered.

Follow-up PRs build on this:

  - I/O UDFs:    ST_GeogFromText, ST_AsText, ST_AsBinary,
                 ST_GeogFromBinary (~150 LoC)
  - Casts:       GEOMETRY <-> GEOGRAPHY, GEOGRAPHY <-> TGEOGPOINT
                 (~80 LoC)
  - Operations: length / area / eIntersects / nearestApproachDistance
                 on GEOGRAPHY columns dispatching to MEOS geog_*
                 functions
  - Tests:      round-trip, cast-matrix, numeric checks against
                MEOS-on-Postgres ground truth (~200 LoC)

Total target surface (from MobilityDB#168): ~430 LoC across the four follow-ups.
Each follow-up is independently mergeable on top of this skeleton.

Stacks on MobilityDB#168 (geography boundary design doc).
… PR MobilityDB#161)

Cherry-picked from open PR MobilityDB#161 so this PR's CI compiles against the
vcpkg-installed MEOS, which exposes 'meosType' (pre-consolidation)
not 'MeosType'.  When MobilityDB#161 reaches main, this commit collapses to a
no-op on rebase.
…MobilityDB#136)

Cherry-picked from open PR MobilityDB#136 so this PR's amd64 Linux test phase
goes green before MobilityDB#136 lands.  When MobilityDB#136 reaches main, this rebase
collapses to a no-op.
…en PR MobilityDB#140)

Cherry-picked from open PR MobilityDB#140 so this PR's osx_amd64 / osx_arm64 /
wasm builds compile.  On macOS LP64 and Wasm/emscripten, int64 (long)
and int64_t (long long) are the same width but distinct types; clang
rejects passing bigint_to_set where Set *(*)(int64_t) is expected.
The cast is a no-op on Linux.  When MobilityDB#140 reaches main, this rebase
collapses to a no-op.
The first geography.cpp landing had a 'reference to Interval is ambiguous'
arm64 build error because the duckdb header chain pulled in duckdb::Interval
before meos.h's extern "C" block scoped MEOS's Interval to global linkage.
Reorder includes to put meos_wrapper_simple.hpp first, mirroring the
existing static-type pattern in stbox.cpp / tgeogpoint.cpp.
…e_ptr<FunctionData> in Copy()

GCC + DuckDB 1.4.4's unique_ptr does not implicitly convert
derived->base, so 'return r;' in BinsBindData::Copy() fails to compile:

  error: could not convert 'r' from 'unique_ptr<duckdb::{anonymous}::BinsBindData,...>'
                                to 'unique_ptr<duckdb::FunctionData,...>'

Use duckdb's unique_ptr_cast helper (from duckdb/common/helper.hpp) to
do the conversion explicitly, matching the canonical pattern used by
DuckDB core (e.g. table_scan.hpp's TableScanBindData::Copy()).  No
behaviour change; the move is exactly what the implicit conversion
would have done if the compiler accepted it.
…romBinary

Four I/O UDFs over the GEOGRAPHY LogicalType, each a thin shim over a
MEOS export:

  - ST_GeogFromText(VARCHAR)  -> GEOGRAPHY   via geog_in
  - ST_AsText(GEOGRAPHY)      -> VARCHAR     via geo_as_ewkt
  - ST_AsBinary(GEOGRAPHY)    -> BLOB        via geo_as_ewkb
  - ST_GeogFromBinary(BLOB)   -> GEOGRAPHY   via geo_from_ewkb

The BLOB payload of a GEOGRAPHY value is the raw GSERIALIZED struct
(varlena VARSIZE bytes), which preserves the geodetic flag in the type
tag across the DuckDB columnar boundary; standard EWKB does not carry
the flag, so ST_GeogFromBinary re-sets it explicitly via
MEOS_FLAGS_SET_GEODETIC.

test/sql/geography.test covers EWKT round-trip + EWKB round-trip on
POINT and LINESTRING.

Step 1 of the ~430-LoC GEOGRAPHY follow-up roadmap.  Casts
(GEOMETRY <-> GEOGRAPHY, GEOGRAPHY <-> TGEOGPOINT) and operations
(length / area / eIntersects / nearestApproachDistance) land in
follow-up PRs.

See doc/geography-boundary.md.
Two cast functions, both reusing the same GSERIALIZED with the geodetic
flag toggled:

  - GEOMETRY -> GEOGRAPHY: lift via GeometryToGSerialized, set the
    geodetic flag on the resulting GSERIALIZED, store as GEOGRAPHY BLOB.
  - GEOGRAPHY -> GEOMETRY: deserialize the GEOGRAPHY BLOB, clear the
    geodetic flag, emit sgl GEOMETRY via GSerializedToGeometry.

DuckDB-Spatial GEOMETRY has no SRID slot, so a round-trip via GEOMETRY
drops the SRID prefix; an EWKT / EWKB round-trip preserves SRID. The
EWKT and EWKB paths are already tested in test/sql/geography.test.

The existing TGEOGPOINT(GEOMETRY, …) constructors transparently accept
GEOGRAPHY arguments via the new implicit cast — no extra overloads
needed for the temporal geographic family.

Step 2 of the GEOGRAPHY follow-up roadmap.  Operations (length / area /
eIntersects / nearestApproachDistance) and the full test matrix land in
steps 3 and 4.

test/sql/geography.test: 9 assertions pass locally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant