pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input.
Date: Sat, 12 Apr 2025 16:28:14 +0000
Message-ID: <E1u3diI-003vJ6-1U@gemulon.postgresql.org> (raw)

Fix GIN's shimTriConsistentFn to not corrupt its input.

Commit 0f21db36d made an assumption that GIN triConsistentFns
would not modify their input entryRes[] arrays.  But in fact,
the "shim" triConsistentFn that we use for opclasses that don't
supply their own did exactly that, potentially leading to wrong
answers from a GIN index search.  Through bad luck, none of the
test cases that we have for such opclasses exposed the bug.

One response to this could be that the assumption of consistency check
functions not modifying entryRes[] arrays is a bad one, but it still
seems reasonable to me.  Notably, shimTriConsistentFn is itself
assuming that with respect to the underlying boolean consistentFn,
so it's sure being self-centered in supposing that it gets to do so.

Fortunately, it's quite simple to fix shimTriConsistentFn to restore
the entry-time state of entryRes[], so let's do that instead.

This issue doesn't affect any core GIN opclasses, since they all
supply their own triConsistentFns.  It does affect contrib modules
btree_gin, hstore, and intarray.

Along the way, I (tgl) noticed that shimTriConsistentFn failed to
pick up on a "recheck" flag returned by its first call to the boolean
consistentFn.  This may be only a latent problem, since it would be
unlikely for a consistentFn to set recheck for the all-false case
and not any other cases.  (Indeed, none of our contrib modules do
that.)  Nonetheless, it's formally wrong.

Reported-by: Vinod Sridharan <vsridh90@gmail.com>
Author: Vinod Sridharan <vsridh90@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAFMdLD7XzsXfi1+DpTqTgrD8XU0i2C99KuF=5VHLWjx4C1pkcg@mail.gmail.com
Backpatch-through: 13

Branch
------
REL_14_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/1b47a112ac41b6414eadebcb3d5f3908bf440565

Modified Files
--------------
contrib/intarray/expected/_int.out | 42 ++++++++++++++++++++++++++++++++++++++
contrib/intarray/sql/_int.sql      |  7 +++++++
src/backend/access/gin/ginlogic.c  | 20 +++++++++++++-----
3 files changed, 64 insertions(+), 5 deletions(-)



view thread (6+ messages)  latest in thread

Message-ID: <E1u3diI-003vJ6-1U@gemulon.postgresql.org>
Permalink:  ../E1u3diI-003vJ6-1U@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1u3diI-003vJ6-1U@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Fix GIN's shimTriConsistentFn to not corrupt its input.
  In-Reply-To: <E1u3diI-003vJ6-1U@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox