Thread: pgsql: Fix behavior of ~> (cube, int) operator
Fix behavior of ~> (cube, int) operator ~> (cube, int) operator was especially designed for knn-gist search. However, it appears that knn-gist search can't work correctly with current behavior of this operator when dataset contains cubes of variable dimensionality. In this case, the same value of second operator argument can point to different dimension depending on dimensionality of particular cube. Such behavior is incompatible with gist indexing of cubes, and knn-gist doesn't work correctly for it. This patch changes behavior of ~> (cube, int) operator by introducing dimension numbering where value of second argument unambiguously identifies number of dimension. With new behavior, this operator can be correctly supported by knn-gist. Relevant changes to cube operator class are also included. Backpatch to v9.6 where operator was introduced. Since behavior of ~> (cube, int) operator is changed, depending entities must be refreshed after upgrade. Such as, expression indexes using this operator must be reindexed, materialized views must be rebuilt, stored procedures and client code must be revised to correctly use new behavior. That should be mentioned in release notes. Noticed by: Tomas Vondra Author: Alexander Korotkov Reviewed by: Tomas Vondra, Andrey Borodin Discussion: https://www.postgresql.org/message-id/flat/a9657f6a-b497-36ff-e56-482a2c7e3292@2ndquadrant.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/563a053bdd4b91c5e5560f4bf91220e562326f7d Modified Files -------------- contrib/cube/cube.c | 120 ++++++++++++--- contrib/cube/expected/cube.out | 317 ++++++++++++++++++++++++--------------- contrib/cube/expected/cube_2.out | 317 ++++++++++++++++++++++++--------------- contrib/cube/sql/cube.sql | 35 +++-- doc/src/sgml/cube.sgml | 9 +- 5 files changed, 512 insertions(+), 286 deletions(-)
Teodor Sigaev <teodor@sigaev.ru> writes: > Fix behavior of ~> (cube, int) operator This patch has caused Coverity to complain, correctly AFAICS, about dead code in cube_coord_llur in the back branches: 1628 /* Inverse value if needed */ 1629 if (inverse) >>> CID 1463943: Control flow issues (DEADCODE) >>> Execution cannot reach this statement: "result = -result;". 1630 result = -result; 1631 1632 PG_RETURN_FLOAT8(result); Seems to be due to sloppy division of changes between f50c80dbb (which was not back-patched) and 563a053bd. Please fix. regards, tom lane
On Sun, Jan 21, 2018 at 11:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Teodor Sigaev <teodor@sigaev.ru> writes:
> Fix behavior of ~> (cube, int) operator
This patch has caused Coverity to complain, correctly AFAICS, about
dead code in cube_coord_llur in the back branches:
1628 /* Inverse value if needed */
1629 if (inverse)
>>> CID 1463943: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "result = -result;".
1630 result = -result;
1631
1632 PG_RETURN_FLOAT8(result);
Seems to be due to sloppy division of changes between f50c80dbb (which
was not back-patched) and 563a053bd. Please fix.
Thank you for catching this. You diagnosis is right.
I propose to commit the attached patch to 10 and 9.6.
------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachment
inverse variable becomes unused, will remove Alexander Korotkov wrote: > Seems to be due to sloppy division of changes between f50c80dbb (which > was not back-patched) and 563a053bd. Please fix. > > > Thank you for catching this. You diagnosis is right. > I propose to commit the attached patch to 10 and 9.6. -- Teodor Sigaev E-mail: teodor@sigaev.ru WWW: http://www.sigaev.ru/
Pushed, thank you Alexander Korotkov wrote: > On Sun, Jan 21, 2018 at 11:20 PM, Tom Lane <tgl@sss.pgh.pa.us > <mailto:tgl@sss.pgh.pa.us>> wrote: > > Teodor Sigaev <teodor@sigaev.ru <mailto:teodor@sigaev.ru>> writes: > > Fix behavior of ~> (cube, int) operator > > This patch has caused Coverity to complain, correctly AFAICS, about > dead code in cube_coord_llur in the back branches: > > 1628 /* Inverse value if needed */ > 1629 if (inverse) > >>> CID 1463943: Control flow issues (DEADCODE) > >>> Execution cannot reach this statement: "result = -result;". > 1630 result = -result; > 1631 > 1632 PG_RETURN_FLOAT8(result); > > Seems to be due to sloppy division of changes between f50c80dbb (which > was not back-patched) and 563a053bd. Please fix. > > > Thank you for catching this. You diagnosis is right. > I propose to commit the attached patch to 10 and 9.6. > > ------ > Alexander Korotkov > Postgres Professional:http://www.postgrespro.com <http://www.postgrespro.com/> > The Russian Postgres Company -- Teodor Sigaev E-mail: teodor@sigaev.ru WWW: http://www.sigaev.ru/