Subject: Re: Parallel CREATE INDEX for BRIN indexes

From: Andres Freund <[email-masked]>
Date: Sat, 13 Apr 2024 01:36:35 -0700

Hi,

While preparing a differential code coverage report between 16 and HEAD, one
thing that stands out is the parallel brin build code. Neither on
coverage.postgresql.org nor locally is that code reached during our tests.

https://coverage.postgresql.org/src/backend/access/brin/brin.c.gcov.html#2333

Greetings,

Andres Freund

---

From: Tomas Vondra <[email-masked]>
Date: Sat, 13 Apr 2024 11:19:21 +0200

> coverage.postgresql.org nor locally is that code reached during our tests.

Thanks for pointing this out, it's definitely something that I need to
improve (admittedly, should have been part of the patch). I'll also look
into eliminating the difference between BTREE and BRIN parallel builds,
mentioned in my last message in this thread.

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

---

From: Tomas Vondra <[email-masked]>
Date: Sat, 13 Apr 2024 23:04:33 +0200

> mentioned in my last message in this thread.

Here's a couple patches adding a test for the parallel CREATE INDEX with
BRIN. The actual test is 0003/0004 - I added the test to pageinspect,
because that allows cross-checking the index to one built without
parallelism, which I think is better than just doing CREATE INDEX
without properly testing it produces correct results.

It's not entirely trivial because for some opclasses (e.g. minmax-multi)
the results depend on the order in which values are added, and order in
which summaries from different workers are merged.

Funnily enough, while adding the test, I ran into two pre-existing bugs.
One is that brin_bloom_union forgot to update the number of bits set in
the bitmap, another one is that 6bcda4a721 changes PG_DETOAST_DATUM to
the _PACKED version, which however does the wrong thing. Both of which
are mostly harmless - it only affects the output function, which is
unused outside pageinspect. No impact on query correctness etc.

The test needs a bit more work to make sure it works on 32-bit machines
etc. which I think may affect available space on a page, which in turn
might affect the minmax-multi summaries. But I'll take care this early
next week.

Funnily

From 450f992c12d43c14aa1b4b72cfd9d3ce0251eed1 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email-masked]>
Date: Sat, 13 Apr 2024 22:15:26 +0200
Subject: [PATCH v20240413 1/4] Update nbits_set in brin_bloom_union

When merging BRIN bloom summaries, it's not enough to merge the bitmaps,
we need to update the nbits_set counter too.

This is mostly harmless, as it does not affect correctness - as the
counter is used only in the out function, and that's used only in
pageinspect to show basic info about the summary.

Backpatch-through: 14
---
 src/backend/access/brin/brin_bloom.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c
index ebf33016279..9e3bc567303 100644
--- a/src/backend/access/brin/brin_bloom.c
+++ b/src/backend/access/brin/brin_bloom.c
@@ -687,10 +687,20 @@ brin_bloom_union(PG_FUNCTION_ARGS)
 
 	nbytes = (filter_a->nbits) / 8;
 
+	filter_a->nbits_set = 0;
+
 	/* simply OR the bitmaps */
 	for (i = 0; i < nbytes; i++)
+	{
 		filter_a->data[i] |= filter_b->data[i];
 
+		for (int bit = 0; bit < 8; bit++)
+		{
+			if (filter_a->data[i] & (0x01 << bit))
+				filter_a->nbits_set++;
+		}
+	}
+
 	PG_RETURN_VOID();
 }
 
-- 
2.44.0

From 07739713893bb0f6be5853324e8897159cf2ad07 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email-masked]>
Date: Sat, 13 Apr 2024 22:18:37 +0200
Subject: [PATCH v20240413 2/4] Use correct PG_DETOAST_DATUM macro in BRIN

Commit 6bcda4a721 replaced PG_DETOAST_DATUM with PG_DETOAST_DATUM_PACKED
in two places, in output function for minmax-multi and bloom. But this
is incorrect - the structs backing both summaries include 4B header, so
we need to detoast them fully. With _PACKED we may end up with 1B
header, so the fields will be shifted and have incorrect values.

Backpatch-through: 16
---
 src/backend/access/brin/brin_bloom.c        | 2 +-
 src/backend/access/brin/brin_minmax_multi.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c
index 9e3bc567303..1c2437cef74 100644
--- a/src/backend/access/brin/brin_bloom.c
+++ b/src/backend/access/brin/brin_bloom.c
@@ -807,7 +807,7 @@ brin_bloom_summary_out(PG_FUNCTION_ARGS)
 	StringInfoData str;
 
 	/* detoast the data to get value with a full 4B header */
-	filter = (BloomFilter *) PG_DETOAST_DATUM_PACKED(PG_GETARG_DATUM(0));
+	filter = (BloomFilter *) PG_DETOAST_DATUM(PG_GETARG_DATUM(0));
 
 	initStringInfo(&str);
 	appendStringInfoChar(&str, '{');
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index c5962c00d64..e5d95de5d84 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -3023,7 +3023,7 @@ brin_minmax_multi_summary_out(PG_FUNCTION_ARGS)
 	 * Detoast to get value with full 4B header (can't be stored in a toast
 	 * table, but can use 1B header).
 	 */
-	ranges = (SerializedRanges *) PG_DETOAST_DATUM_PACKED(PG_GETARG_DATUM(0));
+	ranges = (SerializedRanges *) PG_DETOAST_DATUM(PG_GETARG_DATUM(0));
 
 	/* lookup output func for the type */
 	getTypeOutputInfo(ranges->typid, &outfunc, &isvarlena);
-- 
2.44.0

From e6c8c6f5d1d9420a5166512af25e7c3407a877d1 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email-masked]>
Date: Sat, 13 Apr 2024 22:25:35 +0200
Subject: [PATCH v20240413 3/4] Add regression tests for BRIN parallel builds

Adds regression test for parallel CREATE INDEX with a BRIN index. The
test is added to pageinspect, and which makes it easy to compare the
parallel index to an index built without parallelism.
---
 contrib/pageinspect/expected/brin.out | 106 ++++++++++++++++++++++++++
 contrib/pageinspect/sql/brin.sql      |  85 +++++++++++++++++++++
 2 files changed, 191 insertions(+)

diff --git a/contrib/pageinspect/expected/brin.out b/contrib/pageinspect/expected/brin.out
index 3f6e5174bc6..66b199dba8a 100644
--- a/contrib/pageinspect/expected/brin.out
+++ b/contrib/pageinspect/expected/brin.out
@@ -108,3 +108,109 @@ SELECT (COUNT(*) = (SELECT relpages FROM pg_class WHERE relname = 'test2')) AS r
 
 DROP TABLE test1;
 DROP TABLE test2;
+-- test that parallel index build produces the same BRIN index as serial build
+CREATE TABLE brin_parallel_test (a int, b text) WITH (fillfactor=25);
+-- a mix of NULLs and non-NULL values
+INSERT INTO brin_parallel_test
+SELECT (CASE WHEN (mod(i,231) = 0) OR (i BETWEEN 3500 AND 4000) THEN NULL ELSE i END),
+       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE md5(i::text) END)
+  FROM generate_series(1,10000) S(i);
+-- delete a couple pages, to make a range empty
+DELETE FROM brin_parallel_test WHERE a BETWEEN 1000 and 2000;
+-- vacuum to make the ranges actually empty
+VACUUM brin_parallel_test;
+-- We build index with different BRIN opclasses, but we can't use minmax-multi
+-- because the summary is not deterministic (i.e. the order of values affects
+-- how ranges get merged, and if workers see random subsets between runs, that
+-- affects the resulting summaries too). The summaries are equivalent in the
+-- sense that should give the same query results, but look different enough to
+-- not be very useful/convenient for this test.
+SET max_parallel_maintenance_workers = 0;
+CREATE INDEX brin_test_serial_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+-- XXX we have to set some parameters to force parallel build, both because
+-- there's a requirement for table size and plan_create_index_workers assumes
+-- each worker will use work_mem=32MB for sorting (which works for btree, but
+-- not really for BRIN)
+SET min_parallel_table_scan_size = 0;
+SET max_parallel_maintenance_workers = 4;
+SET maintenance_work_mem = '128MB';
+CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+SELECT relname, relpages FROM pg_class WHERE relname IN ('brin_test_serial_idx', 'brin_test_parallel_idx') ORDER BY relname;
+        relname         | relpages 
+------------------------+----------
+ brin_test_parallel_idx |        4
+ brin_test_serial_idx   |        4
+(2 rows)
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 2), 'brin_test_parallel_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 2), 'brin_test_serial_idx');
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value 
+------------+--------+--------+----------+----------+-------------+-------+-------
+(0 rows)
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 2), 'brin_test_serial_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 2), 'brin_test_parallel_idx');
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value 
+------------+--------+--------+----------+----------+-------------+-------+-------
+(0 rows)
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_test_parallel_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 3), 'brin_test_serial_idx');
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value 
+------------+--------+--------+----------+----------+-------------+-------+-------
+(0 rows)
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 3), 'brin_test_serial_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_test_parallel_idx');
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value 
+------------+--------+--------+----------+----------+-------------+-------+-------
+(0 rows)
+
+-- now do the same parallel build, but don't allow starting parallel workers
+-- to force fallback to serial build, and repeat the checks
+DROP INDEX brin_test_parallel_idx;
+SET max_parallel_workers = 0;
+CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+SELECT relname, relpages FROM pg_class WHERE relname IN ('brin_test_serial_idx', 'brin_test_parallel_idx') ORDER BY relname;
+        relname         | relpages 
+------------------------+----------
+ brin_test_parallel_idx |        4
+ brin_test_serial_idx   |        4
+(2 rows)
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 2), 'brin_test_parallel_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 2), 'brin_test_serial_idx');
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value 
+------------+--------+--------+----------+----------+-------------+-------+-------
+(0 rows)
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 2), 'brin_test_serial_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 2), 'brin_test_parallel_idx');
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value 
+------------+--------+--------+----------+----------+-------------+-------+-------
+(0 rows)
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_test_parallel_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 3), 'brin_test_serial_idx');
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value 
+------------+--------+--------+----------+----------+-------------+-------+-------
+(0 rows)
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 3), 'brin_test_serial_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_test_parallel_idx');
+ itemoffset | blknum | attnum | allnulls | hasnulls | placeholder | empty | value 
+------------+--------+--------+----------+----------+-------------+-------+-------
+(0 rows)
+
+DROP TABLE brin_parallel_test;
+RESET min_parallel_table_scan_size;
+RESET max_parallel_maintenance_workers;
+RESET maintenance_work_mem;
diff --git a/contrib/pageinspect/sql/brin.sql b/contrib/pageinspect/sql/brin.sql
index 50f260b8e1f..affd805bb27 100644
--- a/contrib/pageinspect/sql/brin.sql
+++ b/contrib/pageinspect/sql/brin.sql
@@ -54,3 +54,88 @@ SELECT (COUNT(*) = (SELECT relpages FROM pg_class WHERE relname = 'test2')) AS r
 
 DROP TABLE test1;
 DROP TABLE test2;
+
+-- test that parallel index build produces the same BRIN index as serial build
+CREATE TABLE brin_parallel_test (a int, b text) WITH (fillfactor=25);
+
+-- a mix of NULLs and non-NULL values
+INSERT INTO brin_parallel_test
+SELECT (CASE WHEN (mod(i,231) = 0) OR (i BETWEEN 3500 AND 4000) THEN NULL ELSE i END),
+       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE md5(i::text) END)
+  FROM generate_series(1,10000) S(i);
+
+-- delete a couple pages, to make a range empty
+DELETE FROM brin_parallel_test WHERE a BETWEEN 1000 and 2000;
+
+-- vacuum to make the ranges actually empty
+VACUUM brin_parallel_test;
+
+-- We build index with different BRIN opclasses, but we can't use minmax-multi
+-- because the summary is not deterministic (i.e. the order of values affects
+-- how ranges get merged, and if workers see random subsets between runs, that
+-- affects the resulting summaries too). The summaries are equivalent in the
+-- sense that should give the same query results, but look different enough to
+-- not be very useful/convenient for this test.
+SET max_parallel_maintenance_workers = 0;
+CREATE INDEX brin_test_serial_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+
+-- XXX we have to set some parameters to force parallel build, both because
+-- there's a requirement for table size and plan_create_index_workers assumes
+-- each worker will use work_mem=32MB for sorting (which works for btree, but
+-- not really for BRIN)
+SET min_parallel_table_scan_size = 0;
+SET max_parallel_maintenance_workers = 4;
+SET maintenance_work_mem = '128MB';
+CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+
+SELECT relname, relpages FROM pg_class WHERE relname IN ('brin_test_serial_idx', 'brin_test_parallel_idx') ORDER BY relname;
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 2), 'brin_test_parallel_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 2), 'brin_test_serial_idx');
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 2), 'brin_test_serial_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 2), 'brin_test_parallel_idx');
+
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_test_parallel_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 3), 'brin_test_serial_idx');
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 3), 'brin_test_serial_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_test_parallel_idx');
+
+-- now do the same parallel build, but don't allow starting parallel workers
+-- to force fallback to serial build, and repeat the checks
+
+DROP INDEX brin_test_parallel_idx;
+
+SET max_parallel_workers = 0;
+CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+
+SELECT relname, relpages FROM pg_class WHERE relname IN ('brin_test_serial_idx', 'brin_test_parallel_idx') ORDER BY relname;
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 2), 'brin_test_parallel_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 2), 'brin_test_serial_idx');
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 2), 'brin_test_serial_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 2), 'brin_test_parallel_idx');
+
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_test_parallel_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 3), 'brin_test_serial_idx');
+
+SELECT * FROM brin_page_items(get_raw_page('brin_test_serial_idx', 3), 'brin_test_serial_idx')
+EXCEPT
+SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_test_parallel_idx');
+
+
+DROP TABLE brin_parallel_test;
+RESET min_parallel_table_scan_size;
+RESET max_parallel_maintenance_workers;
+RESET maintenance_work_mem;
-- 
2.44.0

From a79446bdc46c85ae9e9386e55bc3f7da43523106 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email-masked]>
Date: Sat, 13 Apr 2024 22:50:53 +0200
Subject: [PATCH v20240413 4/4] add minmax-multi to the regression test

---
 contrib/pageinspect/expected/brin.out | 25 ++++++++++++++-----------
 contrib/pageinspect/sql/brin.sql      | 25 ++++++++++++++-----------
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/contrib/pageinspect/expected/brin.out b/contrib/pageinspect/expected/brin.out
index 66b199dba8a..33e565f8300 100644
--- a/contrib/pageinspect/expected/brin.out
+++ b/contrib/pageinspect/expected/brin.out
@@ -109,24 +109,27 @@ SELECT (COUNT(*) = (SELECT relpages FROM pg_class WHERE relname = 'test2')) AS r
 DROP TABLE test1;
 DROP TABLE test2;
 -- test that parallel index build produces the same BRIN index as serial build
-CREATE TABLE brin_parallel_test (a int, b text) WITH (fillfactor=25);
+CREATE TABLE brin_parallel_test (a int, b text, c bigint) WITH (fillfactor=40);
 -- a mix of NULLs and non-NULL values
 INSERT INTO brin_parallel_test
 SELECT (CASE WHEN (mod(i,231) = 0) OR (i BETWEEN 3500 AND 4000) THEN NULL ELSE i END),
-       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE md5(i::text) END)
+       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE md5(i::text) END),
+       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE (i/40) + mod(i,8) END)
   FROM generate_series(1,10000) S(i);
 -- delete a couple pages, to make a range empty
 DELETE FROM brin_parallel_test WHERE a BETWEEN 1000 and 2000;
 -- vacuum to make the ranges actually empty
 VACUUM brin_parallel_test;
--- We build index with different BRIN opclasses, but we can't use minmax-multi
--- because the summary is not deterministic (i.e. the order of values affects
--- how ranges get merged, and if workers see random subsets between runs, that
--- affects the resulting summaries too). The summaries are equivalent in the
--- sense that should give the same query results, but look different enough to
--- not be very useful/convenient for this test.
+-- We build index with different BRIN opclasses - minmax, bloom and minmax-multi.
+-- With minmax-multi we need to be more careful, because the summary is not
+-- entirely deterministic - the order of values affects how ranges get merged,
+-- and if workers see random subsets between runs, that affects the resulting
+-- summaries too). The summaries would be equivalent in the sense that should
+-- give the same query results, but are not equal for EXCEPT to work. To work
+-- around that, we generate only small number of values to keep exact values,
+-- in which case the ordering does not matter.
 SET max_parallel_maintenance_workers = 0;
-CREATE INDEX brin_test_serial_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+CREATE INDEX brin_test_serial_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b, c int8_minmax_multi_ops) WITH (pages_per_range=7);
 -- XXX we have to set some parameters to force parallel build, both because
 -- there's a requirement for table size and plan_create_index_workers assumes
 -- each worker will use work_mem=32MB for sorting (which works for btree, but
@@ -134,7 +137,7 @@ CREATE INDEX brin_test_serial_idx ON brin_parallel_test USING brin (a int4_minma
 SET min_parallel_table_scan_size = 0;
 SET max_parallel_maintenance_workers = 4;
 SET maintenance_work_mem = '128MB';
-CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b, c int8_minmax_multi_ops) WITH (pages_per_range=7);
 SELECT relname, relpages FROM pg_class WHERE relname IN ('brin_test_serial_idx', 'brin_test_parallel_idx') ORDER BY relname;
         relname         | relpages 
 ------------------------+----------
@@ -174,7 +177,7 @@ SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_t
 -- to force fallback to serial build, and repeat the checks
 DROP INDEX brin_test_parallel_idx;
 SET max_parallel_workers = 0;
-CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b, c int8_minmax_multi_ops) WITH (pages_per_range=7);
 SELECT relname, relpages FROM pg_class WHERE relname IN ('brin_test_serial_idx', 'brin_test_parallel_idx') ORDER BY relname;
         relname         | relpages 
 ------------------------+----------
diff --git a/contrib/pageinspect/sql/brin.sql b/contrib/pageinspect/sql/brin.sql
index affd805bb27..f113de39c5d 100644
--- a/contrib/pageinspect/sql/brin.sql
+++ b/contrib/pageinspect/sql/brin.sql
@@ -56,12 +56,13 @@ DROP TABLE test1;
 DROP TABLE test2;
 
 -- test that parallel index build produces the same BRIN index as serial build
-CREATE TABLE brin_parallel_test (a int, b text) WITH (fillfactor=25);
+CREATE TABLE brin_parallel_test (a int, b text, c bigint) WITH (fillfactor=40);
 
 -- a mix of NULLs and non-NULL values
 INSERT INTO brin_parallel_test
 SELECT (CASE WHEN (mod(i,231) = 0) OR (i BETWEEN 3500 AND 4000) THEN NULL ELSE i END),
-       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE md5(i::text) END)
+       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE md5(i::text) END),
+       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE (i/40) + mod(i,8) END)
   FROM generate_series(1,10000) S(i);
 
 -- delete a couple pages, to make a range empty
@@ -70,14 +71,16 @@ DELETE FROM brin_parallel_test WHERE a BETWEEN 1000 and 2000;
 -- vacuum to make the ranges actually empty
 VACUUM brin_parallel_test;
 
--- We build index with different BRIN opclasses, but we can't use minmax-multi
--- because the summary is not deterministic (i.e. the order of values affects
--- how ranges get merged, and if workers see random subsets between runs, that
--- affects the resulting summaries too). The summaries are equivalent in the
--- sense that should give the same query results, but look different enough to
--- not be very useful/convenient for this test.
+-- We build index with different BRIN opclasses - minmax, bloom and minmax-multi.
+-- With minmax-multi we need to be more careful, because the summary is not
+-- entirely deterministic - the order of values affects how ranges get merged,
+-- and if workers see random subsets between runs, that affects the resulting
+-- summaries too). The summaries would be equivalent in the sense that should
+-- give the same query results, but are not equal for EXCEPT to work. To work
+-- around that, we generate only small number of values to keep exact values,
+-- in which case the ordering does not matter.
 SET max_parallel_maintenance_workers = 0;
-CREATE INDEX brin_test_serial_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+CREATE INDEX brin_test_serial_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b, c int8_minmax_multi_ops) WITH (pages_per_range=7);
 
 -- XXX we have to set some parameters to force parallel build, both because
 -- there's a requirement for table size and plan_create_index_workers assumes
@@ -86,7 +89,7 @@ CREATE INDEX brin_test_serial_idx ON brin_parallel_test USING brin (a int4_minma
 SET min_parallel_table_scan_size = 0;
 SET max_parallel_maintenance_workers = 4;
 SET maintenance_work_mem = '128MB';
-CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b, c int8_minmax_multi_ops) WITH (pages_per_range=7);
 
 SELECT relname, relpages FROM pg_class WHERE relname IN ('brin_test_serial_idx', 'brin_test_parallel_idx') ORDER BY relname;
 
@@ -113,7 +116,7 @@ SELECT * FROM brin_page_items(get_raw_page('brin_test_parallel_idx', 3), 'brin_t
 DROP INDEX brin_test_parallel_idx;
 
 SET max_parallel_workers = 0;
-CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b) WITH (pages_per_range=7);
+CREATE INDEX brin_test_parallel_idx ON brin_parallel_test USING brin (a int4_minmax_ops, a int4_bloom_ops, b, c int8_minmax_multi_ops) WITH (pages_per_range=7);
 
 SELECT relname, relpages FROM pg_class WHERE relname IN ('brin_test_serial_idx', 'brin_test_parallel_idx') ORDER BY relname;
 
-- 
2.44.0

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 15 Aug 2024 15:48:19 +0200

> without properly testing it produces correct results.

These pageinspect tests added a new use of the md5() function.  We got
rid of those in the tests for PG17.  You could write the test case with
something like

  SELECT (CASE WHEN (mod(i,231) = 0) OR (i BETWEEN 3500 AND 4000) THEN NULL ELSE i END),
-       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE md5(i::text) END),
+       (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3750 AND 4250) THEN NULL ELSE encode(sha256(i::text::bytea), 'hex') END),
         (CASE WHEN (mod(i,233) = 0) OR (i BETWEEN 3850 AND 4500) THEN NULL ELSE (i/100) + mod(i,8) END)

But this changes the test output slightly and I'm not sure if this gives
you the data distribution that you need for you test.  Could your check
this please?