public inbox for [email protected]  
help / color / mirror / Atom feed
From: Masahiko Sawada <[email protected]>
To: Andrei Lepikhov <[email protected]>
Cc: PostgreSQL mailing lists <[email protected]>
Subject: Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
Date: Thu, 16 Apr 2026 10:58:40 -0700
Message-ID: <CAD21AoAit-Qp9OriVbb9c_Qebi=Cgxz0AQJu+zxQpp=_1Lt2dQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAD21AoBVEcC5stzLr80RgaWuBh0EoyRQys_aeOz0ceogMVREcQ@mail.gmail.com>
	<[email protected]>
	<CAD21AoC86nuoxy4r6G3_Ysb2y3K+0sybznRkjVq=Yc4URZUN4g@mail.gmail.com>
	<[email protected]>

On Thu, Apr 16, 2026 at 1:26 AM Andrei Lepikhov <[email protected]> wrote:
>
> On 16/04/2026 10:11, Masahiko Sawada wrote:
> > On Thu, Apr 16, 2026 at 12:13 AM Andrei Lepikhov <[email protected]> wrote:
> > -- Random TIDs test. The offset numbers are randomized and must be --
> > unique and ordered. INSERT INTO hideblocks (blockno) SELECT
> > do_set_block_offsets(blkno, array_agg(DISTINCT greatest((random() *
> > :maxoffset)::int, 1))::int2[]) FROM generate_series(1, 100)
> > num_offsets, generate_series(1000, 1100, 1) blkno GROUP BY blkno;
>
> Alright, I used an explicit sort in reverse order to make sure the test is
> stable. I usually create modules that may change different paths, costs, and
> orders, and using random can make things unpredictable. But for this specific
> test, I don't see any risk.
>
> >
> > While I agree that we need to sort the offset numbers, I think it
> > would be better to make sure the offset numbers in the array to be
> > sorted in a test_tidstore.sql file where required, instead of doing so
> > for all cases.
>
> I'm not sure I follow. Are you saying that do_set_block_offsets shouldn't sort
> the incoming offsets?

No, I wanted to mean that if we sort the given array in
do_set_block_offsets() as the proposed patch does, we end up always
sorting arrays even if the sorting is no actually required (e.g., when
executing "SELECT do_set_block_offsets(1,
array[1,2,3,4,100]::int2[]);"). So an alternative idea to stabilize
the regression test would be to create a SQL function to return a list
of sorted offsets and use it where it's required. While the patch gets
a little bigger, It would also help simplify the tests somewhat by
removing the redundant codes. I've attached the patch for this idea.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com


Attachments:

  [application/octet-stream] generate_random_offsets.patch (4.5K, 2-generate_random_offsets.patch)
  download | inline diff:
diff --git a/src/test/modules/test_tidstore/expected/test_tidstore.out b/src/test/modules/test_tidstore/expected/test_tidstore.out
index cbcacfd26e1..ad92041bf35 100644
--- a/src/test/modules/test_tidstore/expected/test_tidstore.out
+++ b/src/test/modules/test_tidstore/expected/test_tidstore.out
@@ -6,6 +6,11 @@ CREATE TEMP TABLE hideblocks(blockno bigint);
 -- The maximum number of heap tuples (MaxHeapTuplesPerPage) in 8kB block is 291.
 -- We use a higher number to test tidstore.
 \set maxoffset 512
+CREATE FUNCTION generate_random_offsets(noffs int) RETURNS int2[]
+BEGIN ATOMIC
+  SELECT array_agg(DISTINCT val ORDER BY val)
+  FROM (SELECT ((random() * :maxoffset)::int + 1) as val FROM generate_series(1, noffs));
+END;
 SELECT test_create(false);
  test_create 
 -------------
@@ -39,8 +44,7 @@ SELECT do_set_block_offsets(501, array[greatest((random() * :maxoffset)::int, 1)
                   501
 (1 row)
 
-SELECT do_set_block_offsets(502, array_agg(DISTINCT greatest((random() * :maxoffset)::int, 1))::int2[])
-  FROM generate_series(1, 3);
+SELECT do_set_block_offsets(502, generate_random_offsets(3));
  do_set_block_offsets 
 ----------------------
                   502
@@ -202,8 +206,7 @@ SELECT do_set_block_offsets(501, array[greatest((random() * :maxoffset)::int, 1)
                   501
 (1 row)
 
-SELECT do_set_block_offsets(502, array_agg(DISTINCT greatest((random() * :maxoffset)::int, 1))::int2[])
-  FROM generate_series(1, 3);
+SELECT do_set_block_offsets(502, generate_random_offsets(3));
  do_set_block_offsets 
 ----------------------
                   502
@@ -212,9 +215,8 @@ SELECT do_set_block_offsets(502, array_agg(DISTINCT greatest((random() * :maxoff
 -- Random TIDs test. The offset numbers are randomized and must be
 -- unique and ordered.
 INSERT INTO hideblocks (blockno)
-SELECT do_set_block_offsets(blkno, array_agg(DISTINCT greatest((random() * :maxoffset)::int, 1))::int2[])
-  FROM generate_series(1, 100) num_offsets,
-  generate_series(1000, 1100, 1) blkno
+SELECT do_set_block_offsets(blkno, generate_random_offsets(100))
+  FROM generate_series(1000, 1100, 1) blkno
 GROUP BY blkno;
 -- Check TIDs we've added to the store.
 SELECT check_set_block_offsets();
@@ -231,3 +233,4 @@ SELECT test_destroy();
 (1 row)
 
 DROP TABLE hideblocks;
+DROP FUNCTION generate_random_offsets(integer);
diff --git a/src/test/modules/test_tidstore/sql/test_tidstore.sql b/src/test/modules/test_tidstore/sql/test_tidstore.sql
index a29e4ec1c55..38f08d23501 100644
--- a/src/test/modules/test_tidstore/sql/test_tidstore.sql
+++ b/src/test/modules/test_tidstore/sql/test_tidstore.sql
@@ -9,6 +9,12 @@ CREATE TEMP TABLE hideblocks(blockno bigint);
 -- We use a higher number to test tidstore.
 \set maxoffset 512
 
+CREATE FUNCTION generate_random_offsets(noffs int) RETURNS int2[]
+BEGIN ATOMIC
+  SELECT array_agg(DISTINCT val ORDER BY val)
+  FROM (SELECT ((random() * :maxoffset)::int + 1) as val FROM generate_series(1, noffs));
+END;
+
 SELECT test_create(false);
 
 -- Test on empty tidstore.
@@ -25,8 +31,7 @@ SELECT do_set_block_offsets(blk, array_agg(off)::int2[])
 
 -- Test offsets embedded in the bitmap header.
 SELECT do_set_block_offsets(501, array[greatest((random() * :maxoffset)::int, 1)]::int2[]);
-SELECT do_set_block_offsets(502, array_agg(DISTINCT greatest((random() * :maxoffset)::int, 1))::int2[])
-  FROM generate_series(1, 3);
+SELECT do_set_block_offsets(502, generate_random_offsets(3));
 
 -- Add enough TIDs to cause the store to appear "full", compared
 -- to the allocated memory it started out with. This is easier
@@ -67,15 +72,13 @@ SELECT test_create(true);
 
 -- Test offsets embedded in the bitmap header.
 SELECT do_set_block_offsets(501, array[greatest((random() * :maxoffset)::int, 1)]::int2[]);
-SELECT do_set_block_offsets(502, array_agg(DISTINCT greatest((random() * :maxoffset)::int, 1))::int2[])
-  FROM generate_series(1, 3);
+SELECT do_set_block_offsets(502, generate_random_offsets(3));
 
 -- Random TIDs test. The offset numbers are randomized and must be
 -- unique and ordered.
 INSERT INTO hideblocks (blockno)
-SELECT do_set_block_offsets(blkno, array_agg(DISTINCT greatest((random() * :maxoffset)::int, 1))::int2[])
-  FROM generate_series(1, 100) num_offsets,
-  generate_series(1000, 1100, 1) blkno
+SELECT do_set_block_offsets(blkno, generate_random_offsets(100))
+  FROM generate_series(1000, 1100, 1) blkno
 GROUP BY blkno;
 
 -- Check TIDs we've added to the store.
@@ -84,3 +87,4 @@ SELECT check_set_block_offsets();
 -- cleanup
 SELECT test_destroy();
 DROP TABLE hideblocks;
+DROP FUNCTION generate_random_offsets(integer);


reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
  In-Reply-To: <CAD21AoAit-Qp9OriVbb9c_Qebi=Cgxz0AQJu+zxQpp=_1Lt2dQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox