pgsql: Fix behavior of ~> (cube, int) operator

Started by Teodor Sigaevover 8 years ago5 messagescomitters
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

appliestests failedCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t207389
psql -h localhost -U postgres

Built from patchset v3 (message #3), September 20, 2026 at 07:50 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t207389_3 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t207389_3 && git checkout t207389_3

Patchset v3 (message #3) is on t207389_3

Jump to latest
#1Teodor Sigaev
teodor@sigaev.ru

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: /messages/by-id/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(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Teodor Sigaev (#1)
Re: pgsql: Fix behavior of ~> (cube, int) operator

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

#3Alexander Korotkov
aekorotkov@gmail.com
In reply to: Tom Lane (#2)
Re: pgsql: Fix behavior of ~> (cube, int) operator

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

Attachments:

t207389_3
cube_backpatch_fix.patchapplication/octet-stream; name=cube_backpatch_fix.patchDownload+0-5
#4Teodor Sigaev
teodor@sigaev.ru
In reply to: Alexander Korotkov (#3)
Re: pgsql: Fix behavior of ~> (cube, int) operator

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/

#5Teodor Sigaev
teodor@sigaev.ru
In reply to: Alexander Korotkov (#3)
Re: pgsql: Fix behavior of ~> (cube, int) operator

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/&gt;
The Russian Postgres Company

--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/