Re: [PATCH] kNN for btree

On Tue, Dec 3, 2019 at 3:00 AM Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> On Mon, Sep 9, 2019 at 11:28 PM Alexander Korotkov
> <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> > On Sun, Sep 8, 2019 at 11:35 PM Alexander Korotkov
> > <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> > > I'm going to push 0001 changing "attno >= 1" to assert.
> >
> > Pushed. Rebased patchset is attached. I propose to limit
> > consideration during this commitfest to this set of 7 remaining
> > patches. The rest of patches could be considered later. I made some
> > minor editing in preparation to commit. But I decided I've couple
> > more notes to Nikita.
> >
> > * 0003 extracts part of fields from BTScanOpaqueData struct into new
> > BTScanStateData struct. However, there is a big comment regarding
> > BTScanOpaqueData just before definition of BTScanPosItem. It needs to
> > be revised.
> > * 0004 adds "knnState" field to BTScanOpaqueData in addition to
> > "state" field. Wherein "knnState" might unused during knn scan if it
> > could be done in one direction. This seems counter-intuitive. Could
> > we rework this to have "forwardState" and "backwardState" fields
> > instead of "state" and "knnState"?
>
> I have reordered patchset into fewer more self-consistent patches.
>
> Besides comments, code beautification and other improvements, now
> there are dedicated fields for forward and backward scan states. The
> forward scan state is the pointer to data structure, which is used in
> ordinal unidirectional scan. So, it's mostly cosmetic change, but it
> improves the code readability.
>
> One thing bothers me. We have to move scalar distance operators from
> btree_gist to core. btree_gist extension upgrade script have to
> qualify operators with schema in order to distinguish core and
> extension implementations. So, I have to use @extschema(at)(dot) But it's
> forbidden for relocatable extensions. For now, I marken btree_gist as
> non-relocatable. Our comment in extension.c says "For a relocatable
> extension, we needn't do this. There cannot be any need for
> @extschema@, else it wouldn't be relocatable.". Is it really true? I
> think now we have pretty valid example for relocatable extension,
> which needs @extschema@ in upgrade script. Any thoughts?

I've rebased the patchset to the current master and made some
refactoring. I hope it would be possible to bring it to committable
shape during this CF. This need more refactoring though.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company