| From: | Tomas Vondra <tomas(at)vondra(dot)me> |
|---|---|
| To: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
| Cc: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Parallel CREATE INDEX for GIN indexes |
| Date: | 2026-01-16 18:03:47 |
| Message-ID: | 4c7153b5-e9fc-4ce1-986e-4e52661fe771@vondra.me |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 1/15/26 06:08, Kirill Reshke wrote:
> ...
>>>
>>> I think tuplesort_begin_index_gin() has the same issue. It does this to
>>> look up the comparison function:
>>>
>>> /*
>>> * Look for an ordering for the index key data type, and then the sort
>>> * support function.
>>> */
>>> typentry = lookup_type_cache(att->atttypid, TYPECACHE_LT_OPR);
>>> PrepareSortSupportFromOrderingOp(typentry->lt_opr, sortKey);
>>>
>>> That also looks up the key type's b-tree operator rather than the GIN
>>> opclass's compare function.
>>>
>>
>> Thanks for noticing this, I'll get this fixed next week.
>>
>> Funny, you noticed that almost exactly one year after I fixed the other
>> incorrect place in the patch ;-)
>>
The attached 0001 should fix this. I'm wondering what kinds of issues it
might cause, and if we need to mention that in release notes. AFAICS it
would cause trouble if (a) there's no b-tree opclass, in which case the
tuplesort_begin_index_gin() errors out, or (b) the btree/gin opclasses
disagree on ordering (or rather equality).
AFAIK we haven't heard anything about index builds failing on 18, and
with both btree/gin opclasses it seems unlikely they'd define equality
differently. Maybe I'm missing something.
>
>
> I was looking at code coverage for GIN indexes [1] and noticed that
> Parallel GIN build is not covered in the regression test. Btree
> parallel build (_bt_begin_parallel function for example at[0]) is
> covered. I did not find any btree-parallel-build dedicated test, looks
> like this is covered by accident, from write_paralle, partition_prune
> and other regression tests.
>
> So, maybe add some tests here? Also, I wonder what regression sql file
> to use, or maybe create a new one.
>
Fair point. Someone pinged me about this coverage issue a while back,
but it completely slipped my mind. 0002 tweaks two places in regression
tests to build the GIN index in parallel. Which for me seems to improve
the coverage quite a bit. It's not perfect, because it's still not a lot
of data, which means some code is impossible to hit (e.g. it'll not hit
trimming).
regards
--
Tomas Vondra
| Attachment | Content-Type | Size |
|---|---|---|
| 0002-Exercise-parallel-GIN-builds-in-regression-tests.patch | text/x-patch | 3.4 KB |
| 0001-Lookup-the-correct-ordering-for-parallel-GIN-builds.patch | text/x-patch | 3.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jacob Champion | 2026-01-16 18:14:19 | Re: pg_plan_advice |
| Previous Message | Jacob Champion | 2026-01-16 17:52:11 | Re: Custom oauth validator options |