Re: [PATCH] kNN for btree

On Sun, Dec 30, 2018 at 1:19 AM Alexander Korotkov
<a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> On Thu, Dec 27, 2018 at 5:46 AM Alexander Korotkov
> <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> > * 0006-Remove-distance-operators-from-btree_gist-v04.patch
> >
> > I see you provide btree_gist--1.6.sql and remove btree_gist--1.2.sql.
> > Note, that in order to better checking of extension migrations, we're
> > now providing just migration script to new version. So, everybody
> > installing new version will go through the migration. However, in
> > this particular case we've mass deletion of former extension objects.
> > So, I think this case should be an exception to the rules. And it's
> > good to provide new version of extension script in this case. Other
> > opinions?
>
> I also note that you've removed implementation of distance functions
> from btree_gist. But during pg_upgrade extensions are moved "as is".
> Not just CREATE EXTENSION command is dumped, but the whole extension
> content. pg_upgrade'd instances would have old version of extension
> metadata with new .so until ALTER EXTENSION UPDATE. So, user would get
> errors about missed function in .so until updates the extension.
>
> We're typically evade this by inclusion of old functions into new .so.
> Then user can work normally before extension update. In this
> particular case, we can leave the distance functions in the .so, but
> make them just wrappers over core functions.

I've run regression tests with patch applied and opr_sanity showed some errors:

1) date_dist_timestamptz(), timestamp_dist_timestamptz(),
timestamptz_dist_date(), timestamptz_dist_timestamp() should be
stable, not immutable. These functions use timezone during
conversion.

2) date_dist_timestamp(), date_dist_timestamptz(),
timestamp_dist_date(), timestamp_dist_timestamptz(),
timestamptz_dist_date(), timestamptz_dist_timestamp() should be not
leafproof. These functions perform conversion, which might fail in
corner case. So, this error should be considered as a leak.

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