h3-pg icon indicating copy to clipboard operation
h3-pg copied to clipboard

Add GiST and SP-GiST support

Open zachasme opened this issue 6 years ago • 6 comments
trafficstars

We should support GiST and SP-GiST indexes for operations like contains and contained by.

Ongoing work in separate PRs:

  • GiST in #42
  • SP-GiST in #43

zachasme avatar Aug 06 '19 12:08 zachasme

Hi, any news on this? The PR looks stalled as it's 1 year old, but there's no info on the blockers or the status.

How can we help? :hammer:

AbelVM avatar Aug 12 '20 09:08 AbelVM

If I recall correctly, the state of the branch (which hasn't been touched in a year :open_mouth:) is that we managed to get GiST and SP-GiST operator classes to compile and run. However, tests either returned wrong results or were significantly slower than no index. There probably is either a logical error or a misimplementation of (SP-)GiST.

Help is very much welcomed!

The branch should be rebased (or maybe it's easier to simply copy the opclass since so much has changed). Then, we need to figure out what is wrong with our implementation. My implementation is based off the SP-GiST docs and GiST docs but if anyone has some other reference implementation that could be helpful.

zachasme avatar Aug 14 '20 06:08 zachasme

Cool, we will take a look at it. I have some ideas I'd like to test :smiley:

But first, I have a question related to these functions (gist_cmp and common_ancestor, mainly) in the PR: You have defined a lot of bitwise logic in h3index.h, but then in the index related functions, you use high-level methods from the core API instead. Is there any reason for that? Looks like a lot of redundant (implicit) operations to me :thinking:

AbelVM avatar Aug 14 '20 07:08 AbelVM

That sounds great :smiley:

You are correct that the bitwise logic is mostly unused. I have directly copied h3index.h from the h3 repository since I needed the H3_GET_INDEX_DIGIT macro. I actually made this related issue https://github.com/uber/h3/issues/277 since it is not exposed to library consumers.

The branch is very much a work-in-progress, and it would probably better to only include the necessary macros.

zachasme avatar Aug 14 '20 09:08 zachasme

As you asked for some references about GiST:

And, regarding the bitwise version, I just made a PR instead of a codeblock in this comment :)

AbelVM avatar Aug 14 '20 14:08 AbelVM

I've split the old branch into two, so we can get GiST working and merged before moving on to SP-GiST.

zachasme avatar Aug 24 '20 15:08 zachasme

Hi, you mentioned above that using the GiST index can lead to slower results - is there more known about the causes of this? Are some kinds of queries more affected than others?

multi-spectral avatar Nov 06 '22 01:11 multi-spectral

It is not a problem with GiST generally, but a problem with my initial implementation. Specifically I need to come up with good algorithms for penalty and picksplit (see https://www.postgresql.org/docs/current/gist-extensibility.html). I just haven't yet been able to dedicate the time necessary to solve this.

zachasme avatar Nov 07 '22 09:11 zachasme

Discussion continued in:

  • GiST in #42
  • SP-GiST in #43

zachasme avatar Dec 01 '22 09:12 zachasme