Re: [PATCH] kNN for btree

On Tue, Dec 3, 2019 at 3:00 AM Alexander Korotkov
 wrote:
> On Mon, Sep 9, 2019 at 11:28 PM Alexander Korotkov
>  wrote:
> > On Sun, Sep 8, 2019 at 11:35 PM Alexander Korotkov
> >  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@.  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