Subject: Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"

From: Andrei Lepikhov <[email-masked]>
Date: Thu, 16 Apr 2026 09:13:41 +0200

On 15/04/2026 22:50, Masahiko Sawada wrote:
> On Wed, Apr 15, 2026 at 5:48 AM Andrei Lepikhov <[email-masked]> wrote:
> Could you provide the reproducer of the assertion failure? IIRC there
> have not been such reports on the community so far and the test should
> be included in the patch anyway.
Sure! See in attachment.

-- 
regards, Andrei Lepikhov,
pgEdge

From 2b04d6476c1d4e75e0caab57dbe61ed817a1fac2 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email-masked]>
Date: Wed, 15 Apr 2026 14:41:51 +0200
Subject: [PATCH v1] Sort offsets in test_tidstore's do_set_block_offsets().

TidStoreSetBlockOffsets() requires its offsets[] argument to be
strictly ascending, and asserts the precondition.

The test harness was forwarding the OffsetNumber array straight from
SQL, which works only because the current planner happens to present
array_agg() input in the order the VALUES clause was written.  That
ordering is not a SQL guarantee: array_agg() without ORDER BY is
unordered by specification, and any plan change that reshuffles the
Cartesian product used in test_tidstore.sql -- parallel aggregation,
a different join order, or a randomised planner -- can deliver the
offsets out of order and cause the Assert.

Fix by sorting offs in place inside do_set_block_offsets() before
calling TidStoreSetBlockOffsets(). Duplicates are intentionally left to fail
the strict-inequality Assert.
---
 .../test_tidstore/expected/test_tidstore.out     |  8 ++++++++
 .../modules/test_tidstore/sql/test_tidstore.sql  |  4 ++++
 src/test/modules/test_tidstore/test_tidstore.c   | 16 ++++++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/src/test/modules/test_tidstore/expected/test_tidstore.out b/src/test/modules/test_tidstore/expected/test_tidstore.out
index cbcacfd26e1..0ba1ec76f4c 100644
--- a/src/test/modules/test_tidstore/expected/test_tidstore.out
+++ b/src/test/modules/test_tidstore/expected/test_tidstore.out
@@ -32,6 +32,14 @@ SELECT do_set_block_offsets(blk, array_agg(off)::int2[])
     (VALUES (0), (1), (:maxblkno / 2), (:maxblkno - 1), (:maxblkno)) AS blocks(blk),
     (VALUES (1), (2), (:maxoffset / 2), (:maxoffset - 1), (:maxoffset)) AS offsets(off)
   GROUP BY blk;
+SELECT do_set_block_offsets(1, array_agg(off ORDER BY off DESC)::int2[])
+  FROM
+    (VALUES (1), (2)) AS offsets(off);
+ do_set_block_offsets 
+----------------------
+                    1
+(1 row)
+
 -- Test offsets embedded in the bitmap header.
 SELECT do_set_block_offsets(501, array[greatest((random() * :maxoffset)::int, 1)]::int2[]);
  do_set_block_offsets 
diff --git a/src/test/modules/test_tidstore/sql/test_tidstore.sql b/src/test/modules/test_tidstore/sql/test_tidstore.sql
index a29e4ec1c55..f1b184656c4 100644
--- a/src/test/modules/test_tidstore/sql/test_tidstore.sql
+++ b/src/test/modules/test_tidstore/sql/test_tidstore.sql
@@ -23,6 +23,10 @@ SELECT do_set_block_offsets(blk, array_agg(off)::int2[])
     (VALUES (1), (2), (:maxoffset / 2), (:maxoffset - 1), (:maxoffset)) AS offsets(off)
   GROUP BY blk;
 
+SELECT do_set_block_offsets(1, array_agg(off ORDER BY off DESC)::int2[])
+  FROM
+    (VALUES (1), (2)) AS offsets(off);
+
 -- 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[])
diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c
index c9a035fa494..bfc40bce24c 100644
--- a/src/test/modules/test_tidstore/test_tidstore.c
+++ b/src/test/modules/test_tidstore/test_tidstore.c
@@ -75,6 +75,19 @@ itemptr_cmp(const void *left, const void *right)
 	return 0;
 }
 
+static int
+offsetnumber_cmp(const void *a, const void *b)
+{
+	OffsetNumber l = *(const OffsetNumber *) a;
+	OffsetNumber r = *(const OffsetNumber *) b;
+
+	if (l < r)
+		return -1;
+	else if (l > r)
+		return 1;
+	return 0;
+}
+
 /*
  * Create a TidStore. If shared is false, the tidstore is created
  * on TopMemoryContext, otherwise on DSA. Although the tidstore
@@ -178,6 +191,9 @@ do_set_block_offsets(PG_FUNCTION_ARGS)
 	noffs = ArrayGetNItems(ARR_NDIM(ta), ARR_DIMS(ta));
 	offs = ((OffsetNumber *) ARR_DATA_PTR(ta));
 
+	/* TidStoreSetBlockOffsets() requires offsets to be strictly ascending. */
+	qsort(offs, noffs, sizeof(OffsetNumber), offsetnumber_cmp);
+
 	/* Set TIDs in the store */
 	TidStoreLockExclusive(tidstore);
 	TidStoreSetBlockOffsets(tidstore, blkno, offs, noffs);
-- 
2.53.0

---

From: Masahiko Sawada <[email-masked]>
Date: Thu, 16 Apr 2026 01:11:47 -0700

> > be included in the patch anyway.

Thank you for updating the patch.

IIUC the assertion failure could happen only where we do random TIDs
test like below because it's not guaranteed that the offset numbers in
the array after applying DISTICT are sorted.

-- 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;

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.

Regards,

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

---

From: Andrei Lepikhov <[email-masked]>
Date: Thu, 16 Apr 2026 10:26:01 +0200

> 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.

> sorted in a test_tidstore.sql file where required, instead of doing so

I'm not sure I follow. Are you saying that do_set_block_offsets shouldn't sort
the incoming offsets? I made this change mainly to meet the
TidStoreSetBlockOffsets contract. Since this is just a simple test, performance
isn't really a concern.

---

From: Masahiko Sawada <[email-masked]>
Date: Thu, 16 Apr 2026 10:58:40 -0700

On Thu, Apr 16, 2026 at 1:26 AM Andrei Lepikhov <[email-masked]> wrote:
> 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,

---

From: Andrei Lepikhov <[email-masked]>
Date: Fri, 17 Apr 2026 23:26:45 +0200

> removing the redundant codes. I've attached the patch for this idea.

Ok. No objections. Both changes are just test routines registered by the
test_tidstore module.

I decided to add C code, mostly following the idea that we reuse examples from
the Postgres codebase when writing our patches/extensions. An explicit
demonstration of the sort contract on the TidStoreSetBlockOffsets() call might
help developers who don't read function comments each time.

---

From: Masahiko Sawada <[email-masked]>
Date: Wed, 22 Apr 2026 09:51:18 -0700

> help developers who don't read function comments each time.

Understood. After more thoughts, I think your idea would be better.

One thing still unclear to me is in which situation the query inthe
test produces an array of unsorted offset numbers. While I understand
it's not guaranteed that the DISTINCT clause returns the sorted
result, doing DISTINCT in an aggregation function is using sort-based
deduplication. I'd like to confirm that the queries in the test could
end up producing the results that violate the assertion. Is it
possible to do that by changing GUC parameters or something?

Regards,

---

From: Andrei Lepikhov <[email-masked]>
Date: Wed, 22 Apr 2026 19:23:39 +0200

> possible to do that by changing GUC parameters or something?
No, this is part of ongoing research into Postgres Optimizer vulnerabilities. I
used two tools: pg_pathcheck [1] and pg-chaos-mode [2]. The first tool finds
hidden dangling pointers in pathlists, which we are currently discussing in
another thread. The second is a patch that makes the cost-based decision random
to help uncover hidden or unwritten coding contracts.
Both tools are experimental and not meant for core use; they are only used to
trigger potential issues. In this case, I think the query picked a costly sorted
path, which led to the crash.

[1] https://github.com/danolivo/pg_pathcheck
[2] https://github.com/danolivo/pg-chaos-test

---

From: Masahiko Sawada <[email-masked]>
Date: Fri, 24 Apr 2026 16:23:46 -0700

> to help uncover hidden or unwritten coding contracts.

Thank you for the clarification!

> path, which led to the crash.

Does this imply that array_agg() could return unsorted results
depending on the plan the optimizer chooses? Or is such a path
currently never selected by the optimizer?

I’m asking because if this scenario never occurs with the current
optimizer, it might make sense to apply the patch only to HEAD (i.e.,
for PG20). On the other hand, backpatching to PG17 might be justified,
given that DISTINCT does not guarantee sorted results in principle,
and the fix could benefit extension development on stable branches.

Regards,

---

From: Andrei Lepikhov <[email-masked]>
Date: Sun, 26 Apr 2026 11:06:47 +0200

> currently never selected by the optimizer?

The array_agg() function does not sort its output. In theory, this means the
join could return results in any order, but in practice, I have not seen this
happen.

> and the fix could benefit extension development on stable branches.

In stable versions, the planner's logic remains unchanged. So, it seems
reliable. However, backpatching could help extension developers a little bit.
Since this code fixes a real issue and does not break anything complex, I would
backpatch it. Still, I am fine with just committing it to master if you prefer.

P.S.
	
I looked into the issue further. The problem happens when the join sides are
shuffled. Here is what I found:

EXPLAIN of the successful execution (unnecessary details stripped):

Insert on pg_temp.hideblocks  (cost=1.21..1.66 rows=0 width=0)
   ->  Subquery Scan on unnamed_subquery  (cost=1.21..1.66 rows=5 width=8)
         Output: unnamed_subquery.do_set_block_offsets
         ->  GroupAggregate  (cost=1.21..1.61 rows=5 width=16)
               Output: do_set_block_offsets("*VALUES*".column1,
			(array_agg("*VALUES*_1".column1))::smallint[]), ...
               Group Key: "*VALUES*".column1
               ->  Sort  (cost=1.21..1.27 rows=25 width=12)
                     Output: "*VALUES*".column1, "*VALUES*_1".column1
                     Sort Key: "*VALUES*".column1
                     ->  Nested Loop  (cost=0.00..0.62 rows=25 width=12)
                           Output: "*VALUES*".column1, "*VALUES*_1".column1
                           ->  Values Scan on "*VALUES*"
				(cost=0.00..0.06 rows=5 width=8)
                                 Output: "*VALUES*".column1
                           ->  Values Scan on "*VALUES*_1"
				(cost=0.00..0.06 rows=5 width=4)
                                 Output: "*VALUES*_1".column1

EXPLAIN that causes assertion:

 Insert on pg_temp.hideblocks  (cost=1.03..1.48 rows=0 width=0)
   ->  Subquery Scan on unnamed_subquery  (cost=1.03..1.48 rows=5 width=8)
         Output: unnamed_subquery.do_set_block_offsets
         ->  GroupAggregate  (cost=1.03..1.43 rows=5 width=16)
               Output: do_set_block_offsets("*VALUES*".column1,
			(array_agg("*VALUES*_1".column1))::smallint[]),...
               Group Key: "*VALUES*".column1
               ->  Sort  (cost=1.03..1.09 rows=25 width=12)
                     Output: "*VALUES*".column1, "*VALUES*_1".column1
                     Sort Key: "*VALUES*".column1
                     ->  Nested Loop  (cost=0.00..0.45 rows=25 width=12)
                           Output: "*VALUES*".column1, "*VALUES*_1".column1
                           ->  Values Scan on "*VALUES*_1"
				(cost=0.00..0.06 rows=5 width=4)
                                 Output: "*VALUES*_1".column1
                           ->  Materialize  (cost=0.00..0.09 rows=5 width=8)
                                 Output: "*VALUES*".column1
                                 ->  Values Scan on "*VALUES*"
					(cost=0.00..0.06 rows=5 width=8)
                                       Output: "*VALUES*".column1

At the second case offsets have come to the aggregation without order that
highlighted the issue.

---

From: Masahiko Sawada <[email-masked]>
Date: Tue, 28 Apr 2026 10:09:17 -0700

> At the second case offsets have come to the aggregation without order that

Thank you for sharing the details.

While the assertion failure is not observed during regular regression
tests because the query is simple enough that the optimizer
consistently chooses plans producing the sorted results, given that
the DISTINCT without the ORDER BY doesn't guarantee to produce the
sorted results in theory, I think it makes sense to apply the proposed
patch. And, it would also make sense to backpatch to PG17, where
tid_store was introduced, for extension development on back branches.

I've attached the patches. I'm going to push them, barring any objections.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
From d3d63ff758f238f632342ca8ff0c957beb82ac75 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email-masked]>
Date: Wed, 15 Apr 2026 14:41:51 +0200
Subject: [PATCH] test_tidstore: Stabilize regression tests by sorting offsets.

TidStoreSetBlockOffsets() requires its offsets array to be strictly
ascending and asserts this precondition. In test_tidstore, we were
passing random offset numbers deduplicated by a DISTINCT clause in an
array_agg() call directly to the do_set_block_offsets() test
harness. However, DISTINCT without an ORDER BY clause does not
guarantee sorted results according to the SQL standard.

Fix this by sorting the offsets in-place inside do_set_block_offsets()
before calling TidStoreSetBlockOffsets().

While this assertion failure is not observed during regular regression
tests because they use queries simple enough that the optimizer
consistently chooses plans yielding sorted results, it makes sense to
stabilize the test. The failure could theoretically occur depending on
the optimizer's plan choice, and has been reported when experimenting
with certain third-party extensions.

Backpatch to v17, where tid_store was introduced, to ensure extension
development on stable branches does not hit this assertion.

Reported-by: Andrei Lepikhov <[email-masked]>
Author: Andrei Lepikhov <[email-masked]>
Discussion: https://postgr.es/m/[email-masked]
---
 src/test/modules/test_tidstore/test_tidstore.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c
index 3f6a11bf21c..e32ce4e5716 100644
--- a/src/test/modules/test_tidstore/test_tidstore.c
+++ b/src/test/modules/test_tidstore/test_tidstore.c
@@ -76,6 +76,19 @@ itemptr_cmp(const void *left, const void *right)
 	return 0;
 }
 
+static int
@@ -180,6 +193,9 @@ do_set_block_offsets(PG_FUNCTION_ARGS)
 
2.54.0

From d12d864634ec2d6445397362844609ab7528106e Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email-masked]>
Date: Wed, 15 Apr 2026 14:41:51 +0200
Subject: [PATCH] test_tidstore: Stabilize regression tests by sorting offsets.

TidStoreSetBlockOffsets() requires its offsets array to be strictly
ascending and asserts this precondition. In test_tidstore, we were
passing random offset numbers deduplicated by a DISTINCT clause in an
array_agg() call directly to the do_set_block_offsets() test
harness. However, DISTINCT without an ORDER BY clause does not
guarantee sorted results according to the SQL standard.

Fix this by sorting the offsets in-place inside do_set_block_offsets()
before calling TidStoreSetBlockOffsets().

While this assertion failure is not observed during regular regression
tests because they use queries simple enough that the optimizer
consistently chooses plans yielding sorted results, it makes sense to
stabilize the test. The failure could theoretically occur depending on
the optimizer's plan choice, and has been reported when experimenting
with certain third-party extensions.

Backpatch to v17, where tid_store was introduced, to ensure extension
development on stable branches does not hit this assertion.

Reported-by: Andrei Lepikhov <[email-masked]>
Author: Andrei Lepikhov <[email-masked]>
Discussion: https://postgr.es/m/[email-masked]
---
 src/test/modules/test_tidstore/test_tidstore.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

+static int
 
2.54.0

From dd3bf8933fc1368bfe22b039339a4633e2f510e1 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email-masked]>
Date: Wed, 15 Apr 2026 14:41:51 +0200
Subject: [PATCH] test_tidstore: Stabilize regression tests by sorting offsets.

TidStoreSetBlockOffsets() requires its offsets array to be strictly
ascending and asserts this precondition. In test_tidstore, we were
passing random offset numbers deduplicated by a DISTINCT clause in an
array_agg() call directly to the do_set_block_offsets() test
harness. However, DISTINCT without an ORDER BY clause does not
guarantee sorted results according to the SQL standard.

Fix this by sorting the offsets in-place inside do_set_block_offsets()
before calling TidStoreSetBlockOffsets().

While this assertion failure is not observed during regular regression
tests because they use queries simple enough that the optimizer
consistently chooses plans yielding sorted results, it makes sense to
stabilize the test. The failure could theoretically occur depending on
the optimizer's plan choice, and has been reported when experimenting
with certain third-party extensions.

Backpatch to v17, where tid_store was introduced, to ensure extension
development on stable branches does not hit this assertion.

Reported-by: Andrei Lepikhov <[email-masked]>
Author: Andrei Lepikhov <[email-masked]>
Discussion: https://postgr.es/m/[email-masked]
---
 src/test/modules/test_tidstore/test_tidstore.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

index eb16e0fbfa6..d12d54f3677 100644
 
+static int
@@ -179,6 +192,9 @@ do_set_block_offsets(PG_FUNCTION_ARGS)
 
2.54.0

---

From: Masahiko Sawada <[email-masked]>
Date: Wed, 29 Apr 2026 09:11:32 -0700

> I've attached the patches. I'm going to push them, barring any objections.

Pushed.

Regards,