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

On Thu, Nov 7, 2024 at 8:47 AM Tender Wang <tndrwang@gmail.com> wrote:
> Alexander Korotkov <aekorotkov@gmail.com> 于2024年11月7日周四 01:15写道：
>> On Wed, Nov 6, 2024 at 7:08 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> > PG Bug reporting form <noreply@postgresql.org> writes:
>> > > When extending a varchar column with a gist index with a custom signal
>> > > length the Postgres server crashes with a segmentation fault.
>> >
>> > What this shows is that CompareOpclassOptions has never been tested
>> > at all except with null inputs.  It's trying to call array_eq
>> > without providing an FmgrInfo.
>>
>
> Based on Tom's analysis, I  provide a POC patch. I'm not sure if it is right
> to use DEFAULT_COLLATION_OID in the patch.

Thank you.  But I'm not sure about DEFAULT_COLLATION_OID.  The default
collation could threaten binary different text'es as the same.
However, they could still be different from opclass point of view.
So, I think it would be safer to do binary comparison of text'es.
Also, array_eq already assumes the order of elements to be the same.
Therefore, we can compare two text[] just with datumIsEqual().
Attached patch implements this.

I've added the check similar what is reported to pg_trgm regression
test.  I also tried to produce the situation when
CompareOpclassOptions() returns false.  However, I didn't manage to do
so.  Currently, the only usage scenario writes oplcass options to a
query string using get_reloptions(), then parses this query and
reconstructs text[] using transformRelOptions().  AFAICS,
CompareOpclassOptions() should always return true in the current usage
scenario is our quoting/dequoting of strings works correctly.

------
Regards,
Alexander Korotkov
Supabase