Regenerate functions.java against the MEOS 1.4 split-header layout#15
Regenerate functions.java against the MEOS 1.4 split-header layout#15estebanzimanyi wants to merge 9 commits into
Conversation
Amalgamate current MEOS 1.4 public headers (postgres_ext_defs.in.h +
postgres_int_defs.h + meos.h + meos_geo.h + meos_cbuffer.h +
meos_npoint.h + meos_pose.h + meos_rgeo.h) into the generator input,
add RTreeSearchOp and error_handler_fn type mappings, then regenerate
src/main/java/functions/functions.java.
Net change in functions.java surface:
+767 new bindings (MEOS 1.4 rename targets + cbuffer/npoint/pose/rgeo/
tspatial families + tile/split/spatialset accessors)
-179 removed (MEOS 1.4 rename sources, e.g. tpoint_* → tspatial_*)
Coverage of MobilitySpark's MeosNative.java supplement: 122 of 133 (91.7%);
residual 11 symbols sit in private headers (meos_internal.h,
meos_internal_geo.h, temporal/temporal.h, temporal/meos_catalog.h) and
are intentionally not picked up here because they have Datum-typed
parameters that the current generator does not lower.
Out of scope (follow-up):
- utils/ConversionUtils.java + types/{boxes,collections,temporal}/*.java
still reference removed names (pgis_geometry_in, pg_timestamptz_in,
pg_interval_in, geoset_*, geo_get_srid, …) — the JMEOS API uplift
against MEOS 1.4 will need its own pass.
Mechanical 1:1 renames across utils/, types/, and test/ for the
removed-or-renamed symbols listed in PR description: pg_*, pgis_*,
geoset_*, tpoint_* (most → tspatial_*/tgeo_*), distance_* (→ tdistance_*),
ever_/always_eq_tpoint_point (→ ever_/always_eq_tgeo_geo), the 5 spatial-
predicate callsites that dropped trailing (false,false) atvalue/restr args,
TInterpolation.toString() → .getValue() for temporal_to_tsequence{,set},
and the 2-arg meos_initialize → 0-arg + (deferred) error-handler split.
Stub methods (UnsupportedOperationException) for symbols with no MEOS 1.4
equivalent:
- TGeomPoint.to_geographic / TGeogPoint.to_geometric
(no tgeompoint_to_tgeogpoint / tgeogpoint_to_tgeompoint;
callers must convert per-instant via geom_to_geog / geog_to_geom)
Replaced symbols where a newer MEOS API supersedes:
- TPoint.expand: tpoint_expand_space → tspatial_to_stbox + stbox_expand_space
- TPoint.transform: lwproj_transform + tpoint_transform_pj → tspatial_transform
- TPoint.is_spatially_contained_in / disjoint / intersects / touches:
drop trailing (false,false) restr/atvalue args
- Temporal.append: insert leading interp arg (LINEAR) for the new
temporal_append_tinstant signature
Build artefacts:
jar/JMEOS.jar refreshed (476 KB, libmeos.so + functions.class with
256 KB of bindings — the regenerated MEOS 1.4 surface)
mvn package -Dmaven.test.skip=false -DskipTests: BUILD SUCCESS
4e34a61 to
0671917
Compare
Two small changes that, combined, eliminate the entire MobilitySpark
MeosNative.java supplement (was 81 lines / 10 declarations after the
prior trim).
1. FunctionsGenerator equivalentTypes additions:
Datum -> long (= uint64; opaque payload — callers pack via
Double.doubleToLongBits / .longValue())
MeosType -> int (enum)
Effect on the existing public surface: one additional binding
(trgeo_restrict_value, the lone Datum-using extern in current
meos.h that was previously skipped). No other changes.
2. Append 10 extern declarations to the amalgamated meos.h. These
functions are exported from libmeos.so but live in MEOS private
headers (meos_internal.h / meos_internal_geo.h /
temporal/temporal.h / temporal/meos_catalog.h) — they are
well-known to MobilitySpark and other JVM consumers but JMEOS
could not pick them up without either lowering the private-header
types or asking the consumer to re-bind them via JNR-FFI.
The 10 added decls:
mobilitydb_version mobilitydb_full_version
temporal_mem_size temptype_basetype
temporal_values_p set_make_free
tnumber_value_split tnumber_value_time_split
tnumber_value_time_boxes tbox_get_value_time_tile
3. Rebuilt jar/JMEOS.jar (478 KB).
The post-regen wrapper-fix script is also extended to cover all 28
broken bool-out-param wrappers (was 18 stbox/tbox accessors only).
The classification keeps the 10 'T **result' wrappers as INDIR
(still does getPointer(0)) and rewrites the 18 'T *result' wrappers
to return the value buffer directly:
bearing_point_point bigintset_value_n
dateset_value_n datespanset_date_n
floatset_value_n geom_azimuth
intset_value_n tbool_value_n
temporal_timestamptz_n tfloat_value_n
tint_value_n tpoint_direction
tstzset_value_n tstzspanset_timestamptz_n
tboxfloat_xmax/_xmin tboxint_xmax/_xmin
The 18 stbox/tbox accessors fixed in the prior commit are unchanged
(same patch criterion).
MobilitySpark consumer test (under estebanzimanyi/MobilitySpark):
907 / 907 green; MeosNative.java deleted.
…erns
This commit promotes the regen pipeline from tribal knowledge in a
session memory note to checked-in scripts, and adds a JUnit test that
would have caught yesterday's getPointer(0) wrapper bug.
scripts/amalgamate_meos_h.sh
Builds src/main/java/builder/resources/meos.h by concatenating the 8
current MEOS public headers and appending extern decls for symbols
that exist in libmeos.so but live in private headers (meos_internal.h,
temporal/temporal.h, temporal/meos_catalog.h) the amalgam excludes on
purpose. Two previous decls dropped because the symbols are not in
current libmeos.so:
- acovers_tgeo_tgeo: declared 'inline int' in tgeo_spatialrels.c so
the compiler does not emit it as an exported symbol
- meos_initialize_noexit_error_handler: removed from MEOS source in
ae43d2f4a (the JSONB integration commit); MobilitySpark uses a
reflective fallback to call it when present
scripts/post_regen_patch.py
Idempotent post-process for the auto-generated functions.java:
1. rtree_search / rtree_search_temporal — the generator emits a
malformed wrapper for these (the C signature is int-return with
MeosArray *result, not bool-return with a value out-param);
rewrite both to a straight delegation taking a caller-supplied
Pointer for the result.
2. bool foo(args, T *result) vs bool foo(args, T **result) — the
generator emits the same wrapper shape for both, with a spurious
getPointer(0) indirection at the end. For DIRECT (T *result, 18
cases) the indirection turns the value buffer into garbage
(caller's getDouble(0) reads the address as IEEE bits and
crashes Unsafe_GetDouble with SIGSEGV). For INDIR (T **result,
10 cases) the indirection is correct. Classify by name and flip
the DIRECT cases to return the buffer directly.
scripts/regenerate.sh
End-to-end orchestrator: amalgamate -> mvn compile -> extractor ->
generator -> patcher. Run as
scripts/regenerate.sh /path/to/MobilityDB
Intentionally NOT wired into the default mvn lifecycle — most users
consume JMEOS without regenerating; folding the pipeline into 'mvn
package' would force every consumer to clone MobilityDB plus run a
python script.
src/test/java/regen/RegenWrapperSanityTest.java
Two end-to-end smoke tests, one DIRECT (stbox_xmin returns 1.5 for
STBOX X((1.5,2.5),(3.5,4.5))) and one INDIR (ttext_value_n returns
the text* the caller can pass to text_out). If a future MEOS bump
adds a new bool-out function and the patcher classifies it wrongly
(or misses it), one of these two cases will fail.
Runs only with -DskipTests=false (matches existing test suite
convention which keeps surefire skipped by default for libmeos.so
reasons). Verified manually with a standalone main against the
rebuilt jar:
DIRECT stbox_xmin → 1.500: OK
INDIR ttext_value_n → "hello": OK
Refreshed jar/JMEOS.jar with the new amalgam + post-regen patches.
490ca07 to
abcb28f
Compare
Pulls together the per-script docstrings into one document covering: the one-liner workflow, what each script does, why the pipeline is not wired into 'mvn package' by default, how to run the smoke test, and a checklist for adding a new MEOS function (including the inline-export gotcha that MobilityDB PR #939 fixes systematically).
The Install PROJ step has been failing for the last two runs because apt-get install pulls from a stale package list that points at the prior libcurl4-gnutls-dev .deb, which the mirror has since rotated out (404). Adding apt-get update first re-fetches the index against the live mirror state, mirroring the fix that closed PR MobilityDB#14, and the -y flag on the dependent installs keeps them non-interactive.
MobilityDB's top-level CMakeLists.txt calls find_package(GSL) at line 274 and the CI runner's stock package set ships none of GSL_INCLUDE_DIR / GSL_LIBRARY / GSL_CBLAS_LIBRARY. Adding libgsl-dev to the apt steps mirrors the closed PR MobilityDB#14 fix that addressed the same gap and lets the Install MobilityDB step proceed to the build phase.
MEOS 1.4 added an explicit `interp` argument to temporal_append_tinstant (1.3 inferred it internally from the temporal type's continuity). The 1.4 regen wired the JNR-FFI signature through but the high-level `Temporal.append_instant` convenience hardcoded TInterpolation.LINEAR -- which MEOS rejects for the step-only base types (Boolean, Integer, String). Restore the 1.3 semantics: derive the argument from the base type's continuity (read from the existing customType field that the concrete TBool / TInt / TText / TFloat / TGeomPoint / ... subclasses already declare) -- LINEAR for continuous types, STEPWISE for the step-only ones -- exactly what MEOS 1.3 inferred internally from the temporal type before the argument existed. Mirrors PyMEOS fcbbce7 and GoMEOS b623659 (the reference implementations of the same MEOS-1.4 signature-change consequence). Verified: mvn -DskipTests compile clean against Java 21.
Mirrors PyMEOS's TestTGeogPointTemporalSpatialOperations
(tests/main/tgeogpoint_test.py, PyMEOS commit 74bd797): a
parametrized JUnit 5 fixture exercising the geodetic temporal
within_distance / intersects / disjoint / touches surface on
TGeogPointInst / Seq / SeqSet. This closes a coverage-parity gap
flagged during the cross-binding geodetic-fixture audit -- prior to
this commit JMEOS exercised constructors / accessors / boxes / IO on
TGeogPoint but not these spatial relationships, so regressions in
either direction (geodetic working or breaking) were silently
unobserved.
Behaviour assumed by the assertions:
* within_distance / intersects / disjoint with a static geometry
argument: relies on MobilityDB#1088 (one geodetic distance kernel;
intersects / disjoint derived from tdwithin(0)). Pre-#1088 the
geodetic path raises ("Only planar coordinates supported"); a
Assumptions.abort runtime guard skips the assertion in that
state so the suite stays green until #1088 lands in the MEOS
this binding builds against. Post-#1088 the assertions activate
automatically and catch regressions.
* Pointwise (Instant / Discrete Sequence) expecteds use the
geodesically-correct truths (distance POINT(1 1)->POINT(2 2) is
~156876 m, far beyond the 2 m threshold). Continuous
(Sequence / SequenceSet) expecteds record the current
planar-approximate continuous turning point
(tpointsegm_tdwithin_turnpt) tracked in MobilityDB#1087.
* touches asserts the documented planar-only behaviour
(DE-9IM topological predicate, not dwithin(0); tracked in #1087).
No production-code changes. mvn test-compile clean (Java 21);
JMEOS's pom hard-codes skipTests=true so live execution is gated to
the docker-based CI, where the runtime guards keep it green until
#1088 propagates and then activate.
|
CONFL diagnosis: rebase blocked on the structural migration done by the merged PR #9 (multi-module restructure). Specifically:
Path forward (the rebase needs manual re-targeting):
Option 3 is likely cleanest given the multi-module restructure also changed the generator's input/output assumptions. The regenerate-script commits at the top of this branch ( The 9 commits on this branch above |
|
Superseded by #19 — the regen intent retargeted onto the post-#9 multi-module layout. PR #19 uses the canonical MEOS-API → Local verification on #19: 87/87 codegen tests pass; jmeos-core compiles green; This PR can be closed once #19 is reviewed. |
Regenerates src/main/java/functions/functions.java against the current MEOS 1.4 split-header layout, a net +767/-179 symbol change that picks up the 1.4 renames (tpoint_* to tspatial_, pgis_geometry_in to geom_in, pg_timestamptz_in to timestamptz_in, geo_get_srid to geo_srid, geoset_ to spatialset_) and the new families the split exposes (cbuffer, npoint, pose, rgeo, the multidimensional tile and split surfaces, spatialset_ accessors). The extractor still reads a single builder/resources/meos.h, here the concatenation of postgres_ext_defs.in.h, postgres_int_defs.h, meos.h, meos_geo.h, meos_cbuffer.h, meos_npoint.h, meos_pose.h and meos_rgeo.h; meos_internal.h and meos_internal_geo.h are excluded because their Datum and MeosType parameters are not lowered by the current generator. Two small FunctionsGenerator patches map RTreeSearchOp to int and error_handler_fn to Pointer, and the rtree_search and rtree_search_temporal wrappers are hand-patched to straight delegation because the generator mishandles an int return combined with a MeosArray out-parameter.