-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add initial SpatiaLite ↔ PostGIS support #96
Conversation
e92dbcd
to
a9fe8b6
Compare
9addd08
to
3627495
Compare
@t-kataym . I am ready to discussing about this PR and testing environment setup. Look like the problems is near Ubuntu |
Hello @mkgrgis, thank you for your effort. In my understanding, if we want to enable PostGIS to be able to use its data types, we need to CREATE EXTENSION PostGIS.
Is there any reason for this creation? Why didn't you enable PostGIS by creating extension PostGIS? FYI, I tried to enable PostGIS by creating extension, executed your test file and got several error.
|
Hello, @son-phamngoc ! Thanks for code and tests reading!
No. My idea of initial experimental geospatial data types support based on
There are some reasons:
|
@mkgrgis Thanks for your answering.
I did. I commented out the SQL to create domain, create PostGIS extension, and then got the error in #96 (comment)
|
Yes. What about SpatiaLite versions on your system, @son-phamngoc ? How did you get the files?
|
@mkgrgis I used libspatialite-devel version 5.0.0-1. I installed it by command |
This should be normal environment. Your 5.0.0-1 SpatiaLite doesn't critically differs with my 5.0.1-3. I'll test with DBeaver visualisation and update this message, but no problems expected. |
Look like some problem is in |
@mkgrgis I think I found the problem. |
@son-phamngoc , I think such PostGIS functions as |
@mkgrgis |
@son-phamngoc, this PR should works also with PostGIS 3+. In PostGIS 2 was implemented current stable internal storage format EWKB. This storage format is used in all of PostGIS 2+ functions including all existed PostGIS 3+ versions. Now I am trying to test context of using of the format. As data source I use https://osmcode.org/osmium-tool/ as |
@son-phamngoc , some additional context PostGIS for testing
SpatiaLite tests For Ubuntu/Debian there is In my case SQLite listing with values from my PostGIS/SpatiaLite tests is also correct:
PostGIS shows some point near, |
@mkgrgis Could you try exactly the same as my example in #96 (comment) and give me the result in your PC (I think a screenshot or attachment of postgis.out result file is good for me)? Possibly, your inserted data is correct, but sqlite_fdw cannot display geometry data. If so, we need to fix more.
Do you mean the following description in your README is not correct? and this PR can also work with PostGIS 3+?
|
Yes. Now I have understood this is not enough. We should also test PostGIS output for the values.
Yes. On my real (not testing) system with installed experimental code and PostGIS and The main point is
Look like you are right after our diagnostics.
Yes. This is correct description because of common semantic version rules. Some examplees:
|
@mkgrgis Thanks for your understanding.
Could you investigate and fix this PR, considering above problem?
I understood. |
Yes, @son-phamngoc . Also I need help with testing environment. How can I enable PostGIS in testing databases? Just unpack PostGIS source to |
@son-phamngoc , after current fixed CI and
Have you trying to get available PostGIS on testing PostgreSQL installation? |
137a9e7
to
98e2cb1
Compare
@son-phamngoc , currently the main problem in CI is invisible |
Please add the following code to Makefile of sqlite_fdw. After that, you can execute
The reason is |
Thanks, @son-phamngoc , but this is not enough. Please read the scripts and error. In every PostgreSQL source code directory there is configured PostGIS source code in |
@son-phamngoc , current code works with both PostGIS and |
dc0ec17
to
09bb507
Compare
@son-phamngoc , now this PR contains universal code for both PostGIS data types and
|
@son-phamngoc , please try to reproduce our CRS destructor problem in current configuration of this PR. I am ready to try detailed debug if the problem is still reproducible. |
@mkgrgis I tried again with Postgres 17.0, Postgis 3.4.2 and the latest source code of sqlite_fdw in this branch. The same problem still occurred. |
Thanks, @son-phamngoc ! I'll continue research around of the problem. |
@son-phamngoc, please subscribe at OSGeo/PROJ#4361 Maybe PROJ team can help us to pin our memory allocating/freeing problem. |
@son-phamngoc , there is answer from |
@son-phamngoc , please check |
@mkgrgis I checked OSGeo/PROJ#4361 (comment) and checked my environment.
Built version:
I didn't build it with option |
@son-phamngoc , I think in your case SpatiaLite uses |
@mkgrgis Thanks for your support. It seems that's correct. I tried to install a clean environment, and then install only |
@mkgrgis This PR has many changes compared to the 1st review, so I would like to perform the 2nd review for this PR. Is this PR ready for review again? |
Yes, @son-phamngoc . I was want to propose this yesterday. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkgrgis Could you check my comments?
@son-phamngoc , I am ready to final review by @t-kataym and merging of this PR as squashed commit. I think in PR merge note we should add thanks list:
|
@t-kataym , are here any other problems or there was gone internal checks about code quality and productivity? |
@mkgrgis I confirmed and there is no further comment. |
Co-authored-by: mkgrgis <[email protected]>
+
bytea
data formats.blob
affinity.No need to add here PostGIS header files, because SpatiaLite has functions for EWKB data transformation.
In this PR:
libspatialite-dev
for compilation and tests.SQLITE_FDW_GIS_ENABLE
variable which can be not defined for compilation without spatial data types support.Oid
, only by names. Hence bind and convert functions in case of unknownOid
s should analyze name of data type.sqlite_fdw
because of no similar or translatable conceptions in SpatiaLite.bytea
domains having listed names and real PostGIS data types. If there is no PostGIS SpatiaLite spatial data will transformed tobytea
data, binary identical with usual PostGIS data format.sqlite_query.c
are improved.IMPORT FOREIGN SCHEMA
converts formalgeometry
andgeography
to similar names if the FDW was compiled with GIS support, otherwise this columns will bebytea
in PostgreSQL.README.md
.