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
From 5e229098a140c5f3361988f24526e3d6687ac2d5 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Fri, 16 Jan 2026 16:05:24 +0100 Subject: [PATCH 2/2] Exercise parallel GIN builds in regression tests Modify two places creating GIN indexes in regression tests, so that the build is parallel. This provides a basic test coverage, even if the amounts of data are fairly small. Reported-by: Kirill Reshke <[email protected]> Backpatch-through: 18 Discussion: https://postgr.es/m/CALdSSPjUprTj+vYp1tRKWkcLYzdy=N=o4cn4y_hoxnsqqwb...@mail.gmail.com --- src/test/regress/expected/jsonb.out | 3 ++- src/test/regress/expected/tsearch.out | 1 + src/test/regress/sql/jsonb.sql | 3 ++- src/test/regress/sql/tsearch.sql | 1 + 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out index 93535fd7dee..4e2467852db 100644 --- a/src/test/regress/expected/jsonb.out +++ b/src/test/regress/expected/jsonb.out @@ -3131,6 +3131,7 @@ SELECT count(*) FROM testjsonb WHERE j @? '$.bar'; 0 (1 row) +ALTER TABLE testjsonb SET (parallel_workers = 2); CREATE INDEX jidx ON testjsonb USING gin (j); SET enable_seqscan = off; SELECT count(*) FROM testjsonb WHERE j @> '{"wait":null}'; @@ -3507,7 +3508,7 @@ SELECT count(*) FROM testjsonb WHERE j = '{"pos":98, "line":371, "node":"CBA", " --gin path opclass DROP INDEX jidx; -CREATE INDEX jidx ON testjsonb USING gin (j jsonb_path_ops); +CREATE INDEX CONCURRENTLY jidx ON testjsonb USING gin (j jsonb_path_ops); SET enable_seqscan = off; SELECT count(*) FROM testjsonb WHERE j @> '{"wait":null}'; count diff --git a/src/test/regress/expected/tsearch.out b/src/test/regress/expected/tsearch.out index 9fad6c8b04b..9287c440709 100644 --- a/src/test/regress/expected/tsearch.out +++ b/src/test/regress/expected/tsearch.out @@ -870,6 +870,7 @@ RESET enable_seqscan; RESET enable_indexscan; RESET enable_bitmapscan; DROP INDEX wowidx; +ALTER TABLE test_tsvector SET (parallel_workers = 2); CREATE INDEX wowidx ON test_tsvector USING gin (a); SET enable_seqscan=OFF; -- GIN only supports bitmapscan, so no need to test plain indexscan diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql index 21db0db81d6..d28ed1c1e85 100644 --- a/src/test/regress/sql/jsonb.sql +++ b/src/test/regress/sql/jsonb.sql @@ -857,6 +857,7 @@ SELECT count(*) FROM testjsonb WHERE j @? '$'; SELECT count(*) FROM testjsonb WHERE j @? '$.public'; SELECT count(*) FROM testjsonb WHERE j @? '$.bar'; +ALTER TABLE testjsonb SET (parallel_workers = 2); CREATE INDEX jidx ON testjsonb USING gin (j); SET enable_seqscan = off; @@ -945,7 +946,7 @@ SELECT count(*) FROM testjsonb WHERE j = '{"pos":98, "line":371, "node":"CBA", " --gin path opclass DROP INDEX jidx; -CREATE INDEX jidx ON testjsonb USING gin (j jsonb_path_ops); +CREATE INDEX CONCURRENTLY jidx ON testjsonb USING gin (j jsonb_path_ops); SET enable_seqscan = off; SELECT count(*) FROM testjsonb WHERE j @> '{"wait":null}'; diff --git a/src/test/regress/sql/tsearch.sql b/src/test/regress/sql/tsearch.sql index fbd26cdba45..dc74aa0c889 100644 --- a/src/test/regress/sql/tsearch.sql +++ b/src/test/regress/sql/tsearch.sql @@ -222,6 +222,7 @@ RESET enable_bitmapscan; DROP INDEX wowidx; +ALTER TABLE test_tsvector SET (parallel_workers = 2); CREATE INDEX wowidx ON test_tsvector USING gin (a); SET enable_seqscan=OFF; -- 2.52.0
From 90c7cda42bcf40fea5766e1e0928ec825857bda1 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Fri, 16 Jan 2026 15:25:35 +0100 Subject: [PATCH 1/2] Lookup the correct ordering for parallel GIN builds When building a tuplesort during parallel GIN builds, the function incorrectly looked up the default B-Tree operator, not the function associated with the GIN opclass (through GIN_COMPARE_PROC). Fixed by using the same logic as initGinState(), and the other place in parallel GIN builds. This could cause two types of issues. First, a data type might not have a B-Tree opclass, in which case the PrepareSortSupportFromOrderingOp() fails with an ERROR. Second, a data type might have both B-Tree and GIN opclasses, defining order/equality in different ways. This could lead to logical corruption in the index. Backpatch to 18, where parallel GIN builds were introduced. Discussion: https://postgr.es/m/[email protected] Reported-by: Heikki Linnakangas <[email protected]> Backpatch-through: 18 --- src/backend/utils/sort/tuplesortvariants.c | 27 ++++++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/sort/tuplesortvariants.c b/src/backend/utils/sort/tuplesortvariants.c index e3e1142126e..2509ac3e3a4 100644 --- a/src/backend/utils/sort/tuplesortvariants.c +++ b/src/backend/utils/sort/tuplesortvariants.c @@ -20,6 +20,7 @@ #include "postgres.h" #include "access/brin_tuple.h" +#include "access/gin.h" #include "access/gin_tuple.h" #include "access/hash.h" #include "access/htup_details.h" @@ -28,6 +29,7 @@ #include "catalog/pg_collation.h" #include "executor/executor.h" #include "pg_trace.h" +#include "utils/builtins.h" #include "utils/datum.h" #include "utils/guc.h" #include "utils/lsyscache.h" @@ -615,7 +617,7 @@ tuplesort_begin_index_gin(Relation heapRel, { SortSupport sortKey = base->sortKeys + i; Form_pg_attribute att = TupleDescAttr(desc, i); - TypeCacheEntry *typentry; + Oid cmpFunc; sortKey->ssup_cxt = CurrentMemoryContext; sortKey->ssup_collation = indexRel->rd_indcollation[i]; @@ -629,11 +631,26 @@ tuplesort_begin_index_gin(Relation heapRel, sortKey->ssup_collation = DEFAULT_COLLATION_OID; /* - * Look for a ordering for the index key data type, and then the sort - * support function. + * If the compare proc isn't specified in the opclass definition, look + * up the index key type's default btree comparator. */ - typentry = lookup_type_cache(att->atttypid, TYPECACHE_LT_OPR); - PrepareSortSupportFromOrderingOp(typentry->lt_opr, sortKey); + cmpFunc = index_getprocid(indexRel, i + 1, GIN_COMPARE_PROC); + if (cmpFunc == InvalidOid) + { + TypeCacheEntry *typentry; + + typentry = lookup_type_cache(att->atttypid, + TYPECACHE_CMP_PROC_FINFO); + if (!OidIsValid(typentry->cmp_proc_finfo.fn_oid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("could not identify a comparison function for type %s", + format_type_be(att->atttypid)))); + + cmpFunc = typentry->cmp_proc_finfo.fn_oid; + } + + PrepareSortSupportComparisonShim(cmpFunc, sortKey); } base->removeabbrev = removeabbrev_index_gin; -- 2.52.0
