From: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
---|---|
To: | Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> |
Cc: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, David Steele <david(at)pgmasters(dot)net>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] [PATCH] kNN for SP-GiST |
Date: | 2018-07-15 11:54:38 |
Message-ID: | 5A8DAE0D-E4E8-48A6-BC8C-048A240875A0@yandex-team.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi!
> 14 июля 2018 г., в 1:31, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> написал(а):
>
> Attached 6th version of the patches.
> ...
>> 2. The patch leaves contribs intact. Do extensions with sp-gist opclasses
>> need to update it's behavior somehow to be used as-is? Or to support new
>> functionality?
>
> There are no SP-GiST opclasses in contrib/, so there is nothing to change in
> contrib/. Moreover, existing extensions need to be updated only for support of
> new distance-ordered searches -- they need to implement distances[][] array
> calculation in their spgInnerConsistent() and spgLeafConsistent() support
> functions.
So, if extension will not update anything - it will keep all preexisting functionality, right?
I have some more nitpicks about naming
+ recheckQual = out.recheck;
+ recheckDist = out.recheckDistances;
Can we call this things somehow in one fashion?
Also, this my be a stupid question, but do we really need to differ this two recheck cases? It is certain that we cannot merge them?
This seems strange if-formatting
+ /* If allTheSame, they should all or none of 'em match */
+ if (innerTuple->allTheSame)
+ if (out.nNodes != 0 && out.nNodes != nNodes)
+ elog(ERROR, "inconsistent inner_consistent results for allTheSame inner tuple");
+
+ if (out.nNodes) // few lines before you compare with 0
Best regards, Andrey Borodin.
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2018-07-15 13:29:16 | Re: [HACKERS] PATCH: multivariate histograms and MCV lists |
Previous Message | Vik Fearing | 2018-07-15 11:10:54 | Re: Allow to specify a index name as ANALYZE parameter |