Opened 7 years ago
Closed 5 years ago
#3979 closed patch (fixed)
[Re]introduce postgis_geos_noop() and postgis_sfcgal_noop()
Reported by: | lucasvr | Owned by: | pramsey |
---|---|---|---|
Priority: | medium | Milestone: | PostGIS 3.0.0 |
Component: | postgis | Version: | 2.4.x |
Keywords: | serialization deserialization performance | Cc: |
Description
PostGIS used to expose a SQL function named geosnoop(geometry) to test the cost of deserializing and reserializing from the PostgreSQL backend. The attached patch brings that function back (this time named as postgis_geos_noop(geometry)) together with its SFCGAL counterpart (postgis_sfcgal_noop(geometry)).
Attachments (2)
Change History (10)
by , 7 years ago
Attachment: | postgis-noop.patch added |
---|
comment:1 by , 7 years ago
comment:2 by , 7 years ago
for the bug itself it sounds like due to the fact you're not re-serializing the output (geometry_serialize)
comment:3 by , 7 years ago
Milestone: | PostGIS 2.4.3 → PostGIS 2.5.0 |
---|
Has to go in 2.5 since it adds to SQL API.
comment:4 by , 7 years ago
It looks like I've attached the first diff that I produced for review purposes. Sorry about that, it was indeed missing the re-serializing part!
I have submitted a new patch (v2) that includes the re-serializing and the regression tests. I've also updated the comments to reflect that this API is available from 2.5.0 onwards. I appreciate your review and guidance on missing bits (if any).
Thanks!
comment:5 by , 7 years ago
Sounds good. Only thing is (beside a missing TAB on line 792 of postgis/lwgeom_sfcgal.c) is I'd avoid the Buffer call in the test and give the geos_noop test a good label like the sfcgal one.
If you feel like doing more, ideally those noop tests would test not a single geometry but a range of geometry types and dimensions, ensuring for example that the SRID is retained upon round-trip and the Z value (GEOS does not support the M value, not sure about sfcgal, if it does it's worth testing that too) and the different types (not all types are supported by GEOS)
comment:6 by , 6 years ago
Milestone: | PostGIS 2.5.0 → PostGIS 3.0.0 |
---|
comment:7 by , 6 years ago
postgis_geos_noop was added, see #2902, so this ticket remains for postgis_sfcgal_noop. Are you willing to complete the patch to include in 3.0.0 lucasvr ?
Can you please also add tests under regress/ directory ? What i get is:
Other function (geos) is fine.
For tests I see also postgis_noop testing is missing, you could add that one in regress/regress.sql togheter with the postgis_geos_noop, and the geos_sfcgal_noop would go in regress_sfcgal.sql