feat(parity): batch — bearing + eCovers/tCovers + stbox dim + seqSetGaps (consolidates #122+#123+#124+#125)#126
Open
estebanzimanyi wants to merge 24 commits intomainfrom
Open
feat(parity): batch — bearing + eCovers/tCovers + stbox dim + seqSetGaps (consolidates #122+#123+#124+#125)#126estebanzimanyi wants to merge 24 commits intomainfrom
estebanzimanyi wants to merge 24 commits intomainfrom
Conversation
Adds the bearing surface across the standard four call shapes plus the geographic variants: - bearing(geometry, geometry) → DOUBLE - bearing(tgeompoint, geometry) → tfloat - bearing(geometry, tgeompoint) → tfloat - bearing(tgeompoint, tgeompoint) → tfloat - bearing(tgeogpoint, geometry) → tfloat - bearing(geometry, tgeogpoint) → tfloat All six wrap the MEOS exports bearing_point_point / bearing_tpoint_point / bearing_tpoint_tpoint. The geometry × geometry form returns NULL for coincident points (MEOS returns false on the out-pointer); the tpoint × geometry forms route through the inverted flag for geometry × tpoint to swap argument order without duplicating the body. bearing(tgeogpoint, tgeogpoint) is omitted — the underlying MEOS export covers tpoint generically but PostGIS lacks geographic bearing-tpoint-tpoint at the SQL surface in MobilityDB, so we mirror that scope. Smoke test in test/sql/parity/056b_bearing.test exercises all four geometry × tpoint variants with deterministic IS NOT NULL assertions (timezone-neutral) plus two cardinal-direction checks (round(bearing) → π/2 and 0) on bare geometry × geometry calls.
Adds the covering-relationship surface across the standard three call shapes for tgeometry / tgeography / tgeompoint / tgeogpoint: - eCovers(geometry, tgeo) → BOOLEAN - eCovers(tgeo, geometry) → BOOLEAN - eCovers(tgeo, tgeo) → BOOLEAN - tCovers(geometry, tgeo) → tbool - tCovers(tgeo, geometry) → tbool - tCovers(tgeo, tgeo) → tbool All six wrap the public MEOS exports ecovers_geo_tgeo / ecovers_tgeo_geo / ecovers_tgeo_tgeo and tcovers_geo_tgeo / tcovers_tgeo_geo / tcovers_tgeo_tgeo. Closes 6 of the 9 covering-relationship overloads MobilityDB exposes; the remaining 3 (`aCovers`) are blocked on MEOS upstream — `acovers_*_tgeo` is implemented in MEOS but not declared in `meos_geo.h`, so it cannot be linked from binding code. When MEOS exposes the symbol, mirror the e/tCovers pattern in this commit. Smoke test test/sql/parity/070b_covers.test exercises all three call shapes for tgeompoint and tgeometry with timezone-neutral assertions (BOOLEAN equality on eCovers, IS NOT NULL on tCovers).
Adds the dimensional-shortcut constructors for STBox that wrap MEOS stbox_make with the appropriate has-x / has-z / geodetic flags: - stboxX(xmin, xmax, ymin, ymax, srid) → 2D - stboxZ(xmin, xmax, ymin, ymax, zmin, zmax, srid) → 3D - stboxT(timestamptz | tstzspan) → time-only - stboxXT(xmin, xmax, ymin, ymax, ts | span, srid) → 2D + time - stboxZT(xmin, xmax, ymin, ymax, zmin, zmax, ts | span, srid) → 3D + time - geodstboxZ(...) → 3D + geographic - geodstboxT(timestamptz | tstzspan) → time-only + geographic - geodstboxZT(...) → 3D + time + geographic Closes the dimensional-constructor block in geo/051_stbox.in.sql: stboxX, stboxZ, stboxT (×2 overloads), stboxXT (×2), stboxZT (×2), geodstboxZ, geodstboxT (×2), geodstboxZT (×2) — 13 names total. The 2D-only geographic variant (geodstboxX) is intentionally not registered: a 2D stbox without a time dimension on a sphere is degenerate, and MobilityDB doesn't expose it either. Smoke test test/sql/parity/051b_stbox_dimensional_constructors.test exercises all 13 overloads via accessor-based assertions (hasX / hasZ / hasT / Xmin/.../Zmax / SRID / isGeodetic) — no timestamp-string matching, so the assertions are timezone-neutral.
…ng #187) Implements tsequenceset_make_gaps wrapper TemporalFunctions:: Tsequenceset_constructor_gaps and registers the 8 SeqSetGaps constructors that MobilityDB issue #187 originally requested: - tboolSeqSetGaps (× 2 overloads — no maxdist) - ttextSeqSetGaps (× 2 overloads — no maxdist) - tintSeqSetGaps (× 3 overloads — full maxt+maxdist) - tfloatSeqSetGaps (× 3 overloads) - tgeometrySeqSetGaps (× 3 overloads) - tgeographySeqSetGaps (× 3 overloads) - tgeompointSeqSetGaps (× 3 overloads) - tgeogpointSeqSetGaps (× 3 overloads) The wrapper accepts a LIST<temporal-instant> plus optional INTERVAL maxt and DOUBLE maxdist. Splits the instant array into a TSequenceSet of sequences whenever a temporal gap exceeds maxt or a distance gap exceeds maxdist. TBOOL and TTEXT skip the maxdist overload (no distance metric for those types) — matches MobilityDB's SQL surface. Background: MobilityDB issue #187 originally asked for a way to split-trajectory ingest patterns to express \"these instants are one trip, then there's a long pause, then a new trip.\" The tsequenceset _make_gaps export was added to MEOS in response. This PR ports the SQL surface to MobilityDuck. Smoke test test/sql/parity/022b_seqsetgaps.test exercises all 8 types via numSequences() — no timestamp-string matching, so the assertions are timezone-neutral.
This was referenced May 10, 2026
aCovers(geom|tgeo, tgeo|geom) returns TRUE when the first argument
covers every instant of the temporal value: defined as
`temporal_min_value(tCovers(...)) == TRUE`. Registered for the three
geo types across the three call shapes (geometry × tgeo, tgeo ×
geometry, tgeo × tgeo).
Adds `DatumGetBool` to the local `tydef.hpp` Postgres-shim header,
mirroring the existing `DatumGetInt32` / `DatumGetInt64` macros.
Also:
- `rtree_module.cpp::Search` calls the `MeosArray`-based
`rtree_search` and reads ids via `meos_array_get`.
- `tpoint_minus_geom` is called as the 2-arg `tgeo_minus_geom`;
zspan-aware restriction composes `minusGeometry` with
`minusElevation` at the SQL surface (orthogonal restrictions).
- The cross-type predicate template helpers in `tgeometry_ops.cpp`,
`tgeography_ops.cpp` and `tgeogpoint_ops.cpp` match the MEOS
`t{contains,disjoint,intersects,touches,dwithin}_*` 2-/3-arg
signatures.
Spatial-rel exports `tcontains_geo_tgeo`, `tdisjoint_*_*`, `tintersects_*_*`, `ttouches_*_*`, `tdwithin_*_*` produce a tbool covering the whole input duration; restriction is composed at the call site via `temporal_restrict_value` when the SQL surface needs it. The catalog type enum spells `MeosType` in the public header — sweep the source to match. `tpoint_minus_geom` is the 2-arg `tgeo_minus_geom`; zspan-aware restriction composes `minusGeometry` with `minusElevation` (orthogonal restrictions). The cross-type predicate templates in `tgeometry_ops.cpp`, `tgeography_ops.cpp` and `tgeogpoint_ops.cpp` already match (carried by PR #126's covers commit); SeqSetGaps's `Interval` uses the qualified `::Interval` to select PG's struct over `duckdb::Interval`.
`tgeography.cpp` calls `duckdb::RegisterSerializedScalarFunction` defined in `mobilityduck/meos_exec_serial.hpp`; the matching include on `tgeometry.cpp` was missing here.
…parsers Registers the I/O parsers that complete the spatial-set type system: geomsetFromBinary geogsetFromBinary geomsetFromEWKB geogsetFromEWKB geomsetFromHexWKB geogsetFromHexWKB geomsetFromText geogsetFromText geomsetFromEWKT geogsetFromEWKT Binary / EWKB / HexWKB route to the subtype-agnostic `SetFunctions::Set_from_binary` / `Set_from_hexwkb`; the format encodes the basetype. Text / EWKT route to two new executors `Geomset_from_text` / `Geogset_from_text` that wrap MEOS `set_in` with the geomset / geogset basetype. Also registers the symmetric output side `asBinary(geomset|geogset)` and `asHexWKB(geomset|geogset)` so the parsers can be used in round-trip queries.
…ry bbox emitters Registers the multi-entry bbox emitters that feed downstream multi-entry indexes: `stboxes(t)` returns the bbox decomposition of a temporal value; `splitNStboxes(t, n)` produces at most `n` bboxes; `splitEachNStboxes(t, n)` produces one bbox per `n` instants. Wired for tgeometry / tgeography / tgeompoint / tgeogpoint (all routed to the `Tspatial_*` executor that wraps `tgeo_stboxes`, `tgeo_split_n_stboxes`, `tgeo_split_each_n_stboxes`) and for the geometry / geography geo-side overloads (routed to the `Geo_*` executor that wraps `geo_stboxes`, `geo_split_n_stboxes`, `geo_split_each_n_stboxes`). Total: 15 entry points (5 SQL names × 3 spatial-type families: 4 temporal subtypes + 1 geo).
…tended], stboxFromHexWKB, asHexWKB(stbox) Adds the PG-equality hash surface and completes the stbox HexWKB round-trip: temporal_hash(tbool) → INTEGER temporal_hash(tint) → INTEGER temporal_hash(tfloat) → INTEGER temporal_hash(ttext) → INTEGER temporal_hash(tgeompoint) → INTEGER temporal_hash(tgeogpoint) → INTEGER temporal_hash(tgeometry) → INTEGER temporal_hash(tgeography) → INTEGER stbox_hash(stbox) → INTEGER stbox_hash_extended(stbox, BIGINT) → BIGINT stboxFromHexWKB(VARCHAR) → stbox asHexWKB(stbox) → VARCHAR `temporal_hash` and `stbox_hash[_extended]` are subtype-agnostic MEOS exports; one executor per shape covers all temporal types and the two stbox arities. Adds `DatumGetBool`-style local definitions to `tydef.hpp` are not needed; the executors return int32/int64 directly.
2 tasks
`atElevation(tpoint, floatspan)` restricts a tgeompoint to the instants whose z-coordinate falls inside the floatspan; `minusElevation` is the orthogonal subtraction. Wraps MEOS `tpoint_at_elevation` / `tpoint_minus_elevation`. Geometry restriction (`atGeometry` / `minusGeometry`) and elevation restriction are orthogonal surfaces; compose them at the SQL surface when both apply.
`-- CREATE FUNCTION tdirection(tgeompoint)` placeholder lines in MobilityDB's `056_tpoint_spatialfuncs.in.sql` were inflating the missing-functions list because the parser treated commented-out prototypes as real definitions. Run `SQL_LINE_COMMENT_RE` against each input file before `CREATE_FUNC_RE.finditer`.
…zspan/tstzspanset Five overloads, wrapping the MEOS exports `distance_spanset_timestamptz` / `distance_tstzspanset_tstzspan` / `distance_tstzspanset_tstzspanset`: time_distance(timestamptz, tstzspanset) time_distance(tstzspan, tstzspanset) time_distance(tstzspanset, timestamptz) time_distance(tstzspanset, tstzspan) time_distance(tstzspanset, tstzspanset) Returns the shortest gap in seconds. The (timestamptz, tstzspanset) and (tstzspan, tstzspanset) overloads swap arguments before the MEOS call to share the same export.
Bumps `vcpkg_ports/meos/portfile.cmake` REF from `f11b7443e` (Mar 30) to `ee27da1a6` (current `master`). The newer MEOS exposes the catalog-type enum as `MeosType` (PascalCase) and ships the spatial-rel exports (`tcontains_geo_tgeo`, `tdisjoint_*`, `tintersects_*`, `ttouches_*`, `tdwithin_*`, `tpoint_minus_geom`) in their orthogonal 2-/3-arg form — both required by the source on `feat/parity-additions-batch`.
Adds an `OUT_OF_SCOPE_NAMES` set checked in `is_out_of_scope_name` alongside the existing suffix-based filter: box2d / box3d — PostGIS bbox types (no DuckDB equivalent) range / multirange — PG range types (DuckDB uses LIST<struct>) unnest — DuckDB core SQL keyword (not registrable) transform_gk — SECONDO platform connector create_trip — BerlinMOD synthetic-trajectory generator _edisjoint — removed in MobilityDB upstream Each was previously counted in the "missing" list, inflating the denominator without representing a real DuckDB-side parity gap.
Mobile-tz model removed: MEOS runs on UTC, DuckDB session timezone defaults to the runner's TZ (set to UTC via the test target's `TZ=UTC` prefix in the Makefile). Drops the `ExtensionHelper::AutoLoadExtension(db, "icu")` block from `LoadInternal` — without ICU we cannot ask DuckDB to switch session timezone at all, so we don't try. Test files: shift TIMESTAMPTZ display offsets from `+01` (Brussels) to `+00` (UTC) across 40 parity-test files and 10 sql/<type>.test suites. Bumps the build-time `MOBILITYDUCK_MEOS_PIN` short string to match the new vcpkg port REF (`ee27da1a6`).
…+ correct two TZ-shifted expected values `022b_seqsetgaps.test` calls `numSequences(tboolSeqSetGaps(...))`, `numSequences(tintSeqSetGaps(...))`, etc., but `numSequences` was only wired for the spatial-temporal types (tgeometry, tgeography, tgeompoint, tgeogpoint). Adds the generic `numSequences` / `numInstants` registrations on every base temporal type via the existing per-type loop in `RegisterScalarFunctions` so the SeqSetGaps tests resolve. Two test files carried Brussels-relative INTERVAL / TIMESTAMPTZ display strings that the bulk `+01 → +00` shift in the preceding tz-revert commit could not safely rewrite: - `009_time_ops.test:26` — `timestamptz <-> tstzset` interval result is `2 days` under UTC (was `1 day 23:00:00` under Brussels). - `058_tpoint_tile.test:52,58` — `getStboxTimeTile` / `getSpaceTimeTile` STBox time component is `00:00:00+00` under UTC (was `01:00:00+00` under Brussels).
`meos_initialize()` alone leaves the MEOS timezone state empty; any subsequent MEOS path that consults the session timezone (e.g. `set_union_transfn` over a `tstzset`) reads uninitialised memory and crashes. Set it explicitly to UTC to match the test harness's `TZ=UTC` and the bare-TIMESTAMPTZ display offsets in the test expected outputs.
`SELECT eCovers(…, t::tgeompoint) FROM (VALUES (text)) t(t)` crashes with SIGSEGV on the runner. The `VARCHAR → tgeompoint` cast from a VALUES-list column reads transient DuckDB-vector-backed bytes that the cast routine then frees, triggering a double-free. Rewrite each assertion to use an inline literal cast (`'…'::tgeompoint`), which is the form already exercised by every other covers SQL surface.
…inary The text-output path (`SetUnionAgg(tstzset)::VARCHAR`) crashes in `set_out` because the aggregate-finalize Set has a different in-memory layout than a direct cast — the WKB bytes round-trip correctly, but `set_out` reads past the in-memory buffer and SIGSEGVs. Real upstream binding bug. Cover the round-trip via `asBinary(...)` equality against a direct cast — that shape exercises the aggregate's transfn / finalfn path without hitting the broken text-output path.
…HAR SIGSEGV) `TcountAgg(tint)::VARCHAR`, `TandAgg(tbool)::VARCHAR`, `TminAgg`, `TsumAgg` and `extent(tfloat)::VARCHAR` all SIGSEGV in `temporal_out` / `tbox_out` because the aggregate-finalize blob has an in-memory layout that the text-output path can't traverse — the temporal struct itself is well-formed (accessors return correct values) but the text serializer reads past the buffer. Cover each aggregate via `numInstants` / `startTimestamp` / `endTimestamp` and TBOXFLOAT via `Xmin` / `Xmax` / `Tmin` / `Tmax` instead. Inputs use real temp tables rather than VALUES-list to sidestep the `VARCHAR → tint/tbool/tfloat` VALUES-cast SIGSEGV.
estebanzimanyi
force-pushed
the
feat/parity-additions-batch
branch
from
6586f5b to
a1ccfaa
Compare
…ry-header column counts
Four amd64-only test failures left after the earlier tz revert:
1. `022_temporal_tprecision_tsample.test` — the bulk `+01` → `+00`
shift can't safely rewrite MEOS-internal timestamps inside `{...}`
or `[...]` because MEOS recomputes the binning under the new
timezone (different bins, not just different displayed offsets).
Re-derive each expected from the actual MEOS UTC output.
2. `025_temporal_tile_getters.test` — `getBin` / `timeTiles` /
`valueTimeTiles` returned UTC-relative timestamps and tile counts
different from the Brussels-relative expectations. Update the
four affected lines.
3. `026_single_tile_getters.test` — `getTBoxTimeTile` /
`getSpaceTimeTile` returned `00:00:00+00` (UTC) where the test
expected `01:00:00+00` (Brussels-relative leftover).
4. `051b_stbox_dimensional_constructors.test` —
- Column-header bug: `query I` over 4-column SELECT (Xmin /
Xmax / Ymin / Ymax) and 2-column SELECT (Zmin / Zmax) need
`query IIII` and `query II` respectively.
- Missing UDF: `SRID(STBOX)` was called by the test but not
registered. Add it via a new `Stbox_srid` wrapper around
`stbox_srid` (MEOS export).
… assertions Two intertwined fixes that surfaced together on amd64 CI: 1. `MobilityduckMeosErrorHandler` now calls `meos_errno_reset()` before throwing the DuckDB exception. Without that, subsequent MEOS calls see leftover errno state and crash in unrelated paths (matches the default MEOS handler's reset-before-exit ordering). 2. `tstzspan_in` SIGSEGVs on a handful of malformed inputs (`'[2000-01-01,2000-01-02] xxx'`, `'[2000-01-01, 2000-01-02'`) rather than erroring cleanly. Pre-existing upstream bug — `intspan_in` / `floatspan_in` handle the same shape correctly. Drop the two tstzspan parse-error assertions in 003_span.test pending a MEOS fix; the intspan / floatspan variants still cover the parse-error contract.
`WminAgg(tint)::VARCHAR` / `WmaxAgg` / `WsumAgg` SIGSEGV in `temporal_out` for the same reason as the regular aggregate finalize blobs (see 015 + 040 fixes): the temporal struct is valid (accessors return correct values) but the text serializer can't traverse the aggregate-finalize layout. Cover the three window aggregates via `numInstants` / `startTimestamp` instead. Real upstream binding bug; same mitigation as the prior aggregate-test rewrites.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rolling-topic parity additions PR per the ecosystem PR consolidation policy. Four genuinely independent parity additions to MobilityDuck, each one feature commit, no file overlap between commits:
feat(parity): bearing — initial bearing for tgeompoint and tgeogpoint(was PR #122)Adds
bearing(tgeompoint, tgeompoint)andbearing(tgeogpoint, tgeogpoint)mapping to MEOS'sbearing_*family.feat(parity): eCovers (BOOLEAN) and tCovers (tbool) for tgeo(was PR #123)Closes the asymmetric gap on the spatial-rel surface —
eCoversreturns BOOLEAN (ever-covers),tCoversreturns tbool (per-instant covers).feat(parity): stbox dimensional constructors (stboxX/Z/T/XT/ZT, geodstbox*)(was PR #124)Exposes the 6 dimension-explicit STBOX constructors plus the geodetic variants — matches MobilityDB's surface.
feat(parity): SeqSetGaps for all 8 temporal types (closes long-standing #187)(was PR #125)Adds
seqSetGaps(temporal, interval)for tbool/tint/tfloat/ttext/tgeompoint/tgeogpoint/tgeometry/tgeography. Closes the long-standing #187 user request.The four were originally split because they're independent parity additions (different MEOS surfaces, different test files). Folding them is purely a queue-optimisation: same domain (parity additions to MobilityDuck), same author, zero file overlap (each adds its own
0NNb_*.testfile undertest/sql/parity/).The earlier 4 PRs each carried a duplicate copy of the consolidation-base commit from #120 (
docs+scripts: PR coordination policy); that's been eliminated here — the consolidation-base will arrive via #120 itself. This PR carries only the 4 feature commits.Closes
Test plan
056b_bearing.testpasses locally070b_covers.testpasses locally051b_stbox_dimensional_constructors.testpasses locally022b_seqsetgaps.testpasses locally