postgrespro / pgsphere

PgSphere provides spherical data types, functions, operators, and indexing for PostgreSQL.
https://pgsphere.org
BSD 3-Clause "New" or "Revised" License
16 stars 14 forks source link

[ISSUE #19] Make HEALPix/MOC support optional #26

Closed esabol closed 1 year ago

esabol commented 1 year ago

This PR address issue #19 and makes support for HEALPix/MOC functionality optional. It is enabled by default, but it can be disabled by including USE_HEALPIX=0 as an argument to the make commands. The README.pg_sphere file is updated to reflect this change (and other recent changes to the build process).

Also, when upgrading my installation from 1.2.1 to 1.2.2, I was getting the error:

# ALTER EXTENSION pg_sphere UPDATE TO '1.2.2';
ERROR:  function g_spoint3_fetch(internal, internal, internal) does not exist

I've included a fix for that in this PR (the function is just g_spoint3_fetch(internal)). Let me know if you prefer that I submit that as a separate PR.

esabol commented 1 year ago

make test fails, but I don't think it worked before this PR either? Does it work for you? Does anyone care? make crushtest works fine...

vitcpp commented 1 year ago

@esabol Nice catch! You've found g_spoint3_fetch issue. Once we do not change the version number I'm ok to do it in this PR. Thank you!

vitcpp commented 1 year ago

@esabol Concerning make test. It fails without your PR. I will create an issue for it.

esabol commented 1 year ago

With the help of Travis CI, I got make test working again, both with HEALPix and without. Some of the line numbers were just off by one. I've included these changes in this PR since the expected output for make test is different with USE_HEALPIX=0 and without, so I think it's related. This work addresses issue #28 as a result. I think we should keep init_test.sql. I think it's meant to be used to test the build before you install it. So perhaps the README should recommend that the procedure should be make; make test; make install; make installcheck; make crushtest?

I've also added make test and make crushtest to the Travis CI and added a separate USE_HEALPIX=0 build and it's testing. All this adds approximately 1 minute to each CI job or about 6 minutes for all builds in PR. I mention this since I know Travis CI costs money. These changes address issue #27.

That just leaves the minimum PostgreSQL version mentioned in the README.pg_sphere file. I've changed it to version 9.6 in the most recent commits. Other than that, this PR is ready to merge if you like what you see. Thanks!

vitcpp commented 1 year ago

@esabol You did great work! Give me please some time to review the changes. Thank you!

vitcpp commented 1 year ago

@esabol I tested your changes. It works for me with and without USE_HEALPIX. Thank you!

esabol commented 1 year ago

Thank you, @vitcpp !