Skip to content

SP-GiST support#43

Merged
zachasme merged 8 commits intomainfrom
spgist
Jan 3, 2025
Merged

SP-GiST support#43
zachasme merged 8 commits intomainfrom
spgist

Conversation

@zachasme
Copy link
Contributor

Replaces #7. Part of #5.

I have closed the old PR which contained code for both GiST and SP-GiST, splitting it into two separate PRs.

Currently the tests do not pass, and work is focused GiST, see #42.

@zachasme zachasme added enhancement 🚀 New feature or request help wanted ⛏️ Extra attention is needed labels Aug 24, 2020
@zachasme zachasme changed the base branch from master to breaking-v4 June 7, 2022 11:35
@zachasme zachasme force-pushed the breaking-v4 branch 2 times, most recently from d63f74b to b1a395b Compare August 23, 2022 12:07
Base automatically changed from breaking-v4 to master August 24, 2022 11:37
@zachasme zachasme marked this pull request as draft October 10, 2022 08:31
@zachasme zachasme changed the title WIP: SP-GiST support SP-GiST support Oct 10, 2022
@BielStela
Copy link
Contributor

BielStela commented Sep 29, 2023

Hi! First and foremost, thank you for doing all this! It is really nice to have h3 integration in postgres :). I've been wondering that the h3 index structure fits perfectly the sp-gist indexes and found this PR. I tried to rebase this branch so it has the latest updates in main to give it a try and work on it. But it is a bit hell of a rebase 😁 and don't think I'm familiarized enough with your code to make it correctly.
I feel weird asking you to rebase this so I can play around with the sp-gist thing (without any success I'm afraid) but it would be awesome if so happens! Thank you!

@zachasme zachasme force-pushed the spgist branch 2 times, most recently from bc7870d to e8aae9e Compare October 2, 2023 07:18
@zachasme
Copy link
Contributor Author

zachasme commented Oct 2, 2023

Hi @BielStela! Any help on this is appreciated, I've gone ahead and rebased the branch on main.

You should be able to run the tests using the script at scripts/develop, and I've put some preliminary tests in h3/test/sql/opclass_spgist.sql.

As far as I can tell the last test query is currently crashing the server.

@BielStela
Copy link
Contributor

Hi, @zachasme, I managed to pass all the test by finding that the line

https://github.com/zachasme/h3-pg/blob/e8aae9eeebb9b708ae3bb46e43e3b738542a3a2b/h3/src/opclass_spgist.c#L294

was the one causing the segfault. I removed the pointer definition and fixed pass by reference here and there of parent and test pass smoothly. The issue is that the index is as slow a not having the index so I think some deeper tweaks in the implementation are needed. I'm learning all this on the way so don't expect amazing news anytime soon 😬

@zachasme
Copy link
Contributor Author

Thank you for the fix!

Yes, I'm afraid the current branch is simply the result of me smashing something together that can run on a PostgreSQL server.

The next (big) step is figuring out how to best implement picksplit and choose, such that the implementation is both correct and performant.

May I ask how you tested that the index is as slow as not having the index?

@zachasme zachasme force-pushed the spgist branch 2 times, most recently from 80409b9 to 374c773 Compare January 3, 2025 09:03
@zachasme zachasme marked this pull request as ready for review January 3, 2025 12:29
@zachasme zachasme self-assigned this Jan 3, 2025
@zachasme
Copy link
Contributor Author

zachasme commented Jan 3, 2025

I'm going to wrap this up for 4.2.0 and release it with an _experimental prefix (and not marked default).

@zachasme zachasme merged commit f8aca22 into main Jan 3, 2025
34 checks passed
@zachasme zachasme deleted the spgist branch January 3, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement 🚀 New feature or request help wanted ⛏️ Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants