Re: BUG #18692: Segmentation fault when extending a varchar column with a gist index with custom signal length

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>, nicolas.maus@bertelsmann.de, pgsql-bugs@lists.postgresql.org
Date: 2024-11-08T05:10:54Z
Lists: pgsql-bugs
Alexander Korotkov <aekorotkov@gmail.com> writes:
> On Fri, Nov 8, 2024 at 4:05 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think this is nonsense.  What about toasted datums, or even
>> just short-header ones?

> You are correct.  I quickly skim trough the sources and didn't find a
> function which compares detoasted contents of Datums excluding
> headers.

Right, there isn't one AFAIK.  For context, datumIsEqual() is mainly
meant to support cases such as comparison of Const nodes in equal(),
where

  a) false match is unacceptable, but false no-match will at worst
     cause some performance loss;
  b) the two inputs have probably come through similar paths (eg,
     both are fresh from the parser, or both were on-disk), so that
     any toasting effects will be the same.
Also
  c) for many of the callers, it'd be unsafe to perform disk
     access to allow accurate comparison of out-of-line values.

Problem (a) would be all right for this case, and (c) too,
but not so much (b).

You can certainly imagine writing a more aggressive routine
that can cope with detoasting, but I don't think we've yet
found a case where that seemed worth the trouble.  Partial
solutions such as "cope with short-header but not compressed
values" have even narrower possible use-cases.

> So, yes, comparison using C-collation seems the most
> reasonable option.

WFM.  But I'm concerned that you couldn't build a test case
where the comparison fails.  Seems like we need one, in view
of this bug having escaped detection for so long.

			regards, tom lane



Commits

  1. Fix arrays comparison in CompareOpclassOptions()