Re: BUG #19641: Unexpected results on an SP-GiST indexed column with a non-deterministic collation - Mailing list pgsql-bugs

From Manu
Subject Re: BUG #19641: Unexpected results on an SP-GiST indexed column with a non-deterministic collation
Date
Msg-id 179019303108.297478.2662804185242466018@gmail.com
Whole thread
In response to BUG #19641: Unexpected results on an SP-GiST indexed column with a non-deterministic collation  (PG Bug reporting form <noreply@postgresql.org>)
List pgsql-bugs
Andrey Rachitskiy <pl0h0yp1@gmail.com> wrote:
> I have a draft of "A" and "B" ready, but I decided not to publish it
> until an agreement on the direction is reached.
>
> Thoughts?

Some data for that choice, all on master (374522aa63a).

First, the scope.  With the 340-row table from the report, the column
under the nondeterministic collation, and the same equality query, each
index type against a sequential scan (227 rows):

- SP-GiST text_ops: 113
- btree, hash, BRIN, GiST (btree_gist): 227

Every plan used its index, so SP-GiST's text_ops is the only one of
them that gets this wrong.  It also does so at any size, one row
included.

Second, what A does to indexes that already exist.  The precedent,
281039631, went in before v12's rc1, when no such index could exist
yet, while an SP-GiST index under a nondeterministic collation has been
accepted since v12.  So I built a prototype of A with the same check as
the pattern_ops one in index.c, for SP-GiST text_ops, created such an
index on an unpatched master cluster, and took it to the prototype:

- a pg_dump restored with psql loads the table (340 rows) and skips
  the index with one ERROR; psql exits with 0, so a restore script
  that does not stop on errors ends up without the index and says
  nothing;
- pg_upgrade fails, during the schema restore, with the same error.

So A cannot be back-patched, and in master it would need a pg_upgrade
check that reports these indexes before the upgrade, as pg_upgrade
does for other objects it cannot carry over.

B has neither problem.  If I read your description right, it changes
only how the scan uses the tree, not how the tree is built, so an
existing index returns correct results after a minor update, without a
REINDEX.  That seems to me the one that can go to all the branches.
Your point that B does not make the index a good accelerator for this
equality stands; that seems like something for the documentation to
say (a btree index serves it better), rather than a reason to break
existing schemas.

If you post the B draft, I am happy to test it on the back branches:
the existing-index case above, and the other text_ops operators under
the same collation.

Regards,
Manu



pgsql-bugs by date:

Previous
From: Manu
Date:
Subject: Re: BUG #19602: Vuln46: citext split_part silently returns NULL for a zero field position instead of raising core sp
Next
From: Manu
Date:
Subject: Re: BUG #19621: Unexpected results of JSON_VALUE with DEFAULT ON EMPTY