Bump vendored MEOS headers to 1.3 and adapt CGO call sites#1
Bump vendored MEOS headers to 1.3 and adapt CGO call sites#1estebanzimanyi wants to merge 2 commits into
Conversation
Adopts the unified spatial nomenclature (tpoint_* renamed to tspatial_* / tgeo_* depending on signature), the meos_initialize() split that moves the timezone to a separate call, and the temporal_append_tinstant interpType parameter. cast.h gains stddef.h so size_t resolves on hosts that do not transitively include it via meos.h.
Davichet-e
left a comment
Davichet-e
left a comment
There was a problem hiding this comment.
Looks good, only minor things
| // TPointSTBoxes Return an array of spatiotemporal boxes from the segments of a temporal point | ||
| func TPointSTBoxes[TP TPoint](tp TP, max_count int) ([]*STBox, error) { | ||
| var count C.int | ||
| _ = max_count |
There was a problem hiding this comment.
If max_count is not used anymore, we should remove it
There was a problem hiding this comment.
Done in 895af5d. Removed the unused max_count parameter from TPointSTBoxes and updated the only caller (example_main_tpoint_test.go).
| func GeographyFromText(input string, srid int) *Geom { | ||
| c_geom_str := C.CString(input) | ||
| defer C.free(unsafe.Pointer(c_geom_str)) | ||
| c_geom := C.geography_from_text(c_geom_str, C.int(srid)) | ||
| c_geom := C.geo_from_text(c_geom_str, C.int(srid)) | ||
| g := &Geom{_inner: c_geom} | ||
| return g | ||
| } | ||
|
|
||
| func GeometryFromText(input string, srid int) *Geom { | ||
| c_geom_str := C.CString(input) | ||
| defer C.free(unsafe.Pointer(c_geom_str)) | ||
| c_geom := C.geometry_from_text(c_geom_str, C.int(srid)) | ||
| c_geom := C.geo_from_text(c_geom_str, C.int(srid)) | ||
| g := &Geom{_inner: c_geom} | ||
| return g | ||
| } |
There was a problem hiding this comment.
This functions are now the same, we could collapse them into one single GeoFromText
There was a problem hiding this comment.
Done in 895af5d. Collapsed into a single GeoFromText; the two were byte-identical since geo_from_text is a single function under the 1.3 unified nomenclature, and there were no other callers.
| func TPointExpandSpace[TP TPoint](tp TP, other float64) *STBox { | ||
| box := C.tspatial_to_stbox(tp.Inner()) | ||
| return &STBox{ | ||
| _inner: C.tpoint_expand_space(tp.Inner(), C.double(other)), | ||
| _inner: C.stbox_expand_space(box, C.double(other)), | ||
| } |
There was a problem hiding this comment.
This is leaking memory since tspatial_to_stbox doesn't touch the input, it creates another stbox
Recommended:
func TPointExpandSpace[TP TPoint](tp TP, other float64) *STBox {
box := C.tspatial_to_stbox(tp.Inner())
defer C.free(unsafe.Pointer(box))
return &STBox{
_inner: C.stbox_expand_space(box, C.double(other)),
}
}There was a problem hiding this comment.
Done in 895af5d. Added defer C.free(unsafe.Pointer(box)). Verified against MEOS source: tspatial_to_stbox returns a fresh palloc'd STBox and stbox_expand_space(const STBox *) returns a separate stbox_copy, so freeing the intermediate is safe and does not touch _inner. This also matches the C.free(unsafe.Pointer(res)) convention already used elsewhere in the package.
…and-space box Collapse the now-identical GeographyFromText and GeometryFromText into a single GeoFromText, since geo_from_text is one function under the 1.3 unified spatial nomenclature. Remove the unused max_count parameter from TPointSTBoxes and update its caller. Free the intermediate STBox from tspatial_to_stbox in TPointExpandSpace; stbox_expand_space copies its input rather than adopting it, so the box was leaked.
|
Did we also run the tests to make sure we didn't break anything? Also maybe worth at some point adding a CI job |
|
Ran the suite. Against a MEOS 1.3 library (built from MobilityDB One caveat worth flagging: this PR pins MEOS 1.3 on purpose, and 1.4 changed several spatial-relationship signatures ( On CI: agreed, and it directly prevents this class of drift. The shape that works (just added to MEOS.NET PR #3) is a workflow that builds |
2 participants
Adopts the unified spatial nomenclature (tpoint_* renamed to tspatial_* / tgeo_* depending on signature), the meos_initialize() split that moves the timezone to a separate call, and the temporal_append_tinstant interpType parameter. cast.h gains stddef.h so size_t resolves on hosts that do not transitively include it via meos.h.