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(at)gmail(dot)com> wrote:
> Alexander Korotkov <aekorotkov(at)gmail(dot)com> 于2024年11月7日周四 01:15写道：
>> On Wed, Nov 6, 2024 at 7:08 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> > PG Bug reporting form <noreply(at)postgresql(dot)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