public inbox for [email protected]
help / color / mirror / Atom feedRe: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
11+ messages / 2 participants
[nested] [flat]
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
@ 2026-04-16 07:13 Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Andrei Lepikhov @ 2026-04-16 07:13 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On 15/04/2026 22:50, Masahiko Sawada wrote:
> On Wed, Apr 15, 2026 at 5:48 AM Andrei Lepikhov <[email protected]> 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 protected]>
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
Attachments:
[text/plain] v1-0001-Sort-offsets-in-test_tidstore-s-do_set_block_offs.patch (4.1K, 2-v1-0001-Sort-offsets-in-test_tidstore-s-do_set_block_offs.patch)
download | inline diff:
From 2b04d6476c1d4e75e0caab57dbe61ed817a1fac2 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
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
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
@ 2026-04-16 08:11 ` Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Masahiko Sawada @ 2026-04-16 08:11 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On Thu, Apr 16, 2026 at 12:13 AM Andrei Lepikhov <[email protected]> wrote:
>
> On 15/04/2026 22:50, Masahiko Sawada wrote:
> > On Wed, Apr 15, 2026 at 5:48 AM Andrei Lepikhov <[email protected]> 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.
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
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
@ 2026-04-16 08:26 ` Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Andrei Lepikhov @ 2026-04-16 08:26 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
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? I made this change mainly to meet the
TidStoreSetBlockOffsets contract. Since this is just a simple test, performance
isn't really a concern.
--
regards, Andrei Lepikhov,
pgEdge
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
@ 2026-04-16 17:58 ` Masahiko Sawada <[email protected]>
2026-04-17 21:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Masahiko Sawada @ 2026-04-16 17:58 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: PostgreSQL mailing lists <[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);
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
@ 2026-04-17 21:26 ` Andrei Lepikhov <[email protected]>
2026-04-22 16:51 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Andrei Lepikhov @ 2026-04-17 21:26 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On 16/04/2026 19:58, Masahiko Sawada wrote:
> 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.
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.
--
regards, Andrei Lepikhov,
pgEdge
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-17 21:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
@ 2026-04-22 16:51 ` Masahiko Sawada <[email protected]>
2026-04-22 17:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Masahiko Sawada @ 2026-04-22 16:51 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On Fri, Apr 17, 2026 at 2:26 PM Andrei Lepikhov <[email protected]> wrote:
>
> On 16/04/2026 19:58, Masahiko Sawada wrote:
> > 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.
>
> 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.
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,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-17 21:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-22 16:51 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
@ 2026-04-22 17:23 ` Andrei Lepikhov <[email protected]>
2026-04-24 23:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Andrei Lepikhov @ 2026-04-22 17:23 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On 22/04/2026 18:51, Masahiko Sawada wrote:
> On Fri, Apr 17, 2026 at 2:26 PM Andrei Lepikhov <[email protected]> wrote:
>>
>> On 16/04/2026 19:58, Masahiko Sawada wrote:
> 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?
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
--
regards, Andrei Lepikhov,
pgEdge
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-17 21:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-22 16:51 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-22 17:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
@ 2026-04-24 23:23 ` Masahiko Sawada <[email protected]>
2026-04-26 09:06 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Masahiko Sawada @ 2026-04-24 23:23 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On Wed, Apr 22, 2026 at 10:23 AM Andrei Lepikhov <[email protected]> wrote:
>
> On 22/04/2026 18:51, Masahiko Sawada wrote:
> > On Fri, Apr 17, 2026 at 2:26 PM Andrei Lepikhov <[email protected]> wrote:
> >>
> >> On 16/04/2026 19:58, Masahiko Sawada wrote:
> > 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?
> 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.
Thank you for the clarification!
> 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.
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,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-17 21:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-22 16:51 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-22 17:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-24 23:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
@ 2026-04-26 09:06 ` Andrei Lepikhov <[email protected]>
2026-04-28 17:09 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Andrei Lepikhov @ 2026-04-26 09:06 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On 25/04/2026 01:23, Masahiko Sawada wrote:
> On Wed, Apr 22, 2026 at 10:23 AM Andrei Lepikhov <[email protected]> wrote:
>> 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.
>
> 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?
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.
>
> 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.
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.
--
regards, Andrei Lepikhov,
pgEdge
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-17 21:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-22 16:51 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-22 17:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-24 23:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-26 09:06 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
@ 2026-04-28 17:09 ` Masahiko Sawada <[email protected]>
2026-04-29 16:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Masahiko Sawada @ 2026-04-28 17:09 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On Sun, Apr 26, 2026 at 2:06 AM Andrei Lepikhov <[email protected]> wrote:
>
> On 25/04/2026 01:23, Masahiko Sawada wrote:
> > On Wed, Apr 22, 2026 at 10:23 AM Andrei Lepikhov <[email protected]> wrote:
> >> 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.
> >
> > 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?
>
> 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.
>
> >
> > 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.
>
> 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.
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
Attachments:
[text/x-patch] REL_17_0001-test_tidstore-Stabilize-regression-tests-by-sorting-.patch (2.6K, 2-REL_17_0001-test_tidstore-Stabilize-regression-tests-by-sorting-.patch)
download | inline diff:
From d3d63ff758f238f632342ca8ff0c957beb82ac75 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
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 protected]>
Author: Andrei Lepikhov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
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
+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
@@ -180,6 +193,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.54.0
[text/x-patch] master_0001-test_tidstore-Stabilize-regression-tests-by-sorting-.patch (2.6K, 3-master_0001-test_tidstore-Stabilize-regression-tests-by-sorting-.patch)
download | inline diff:
From d12d864634ec2d6445397362844609ab7528106e Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
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 protected]>
Author: Andrei Lepikhov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
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 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.54.0
[text/x-patch] REL_18_0001-test_tidstore-Stabilize-regression-tests-by-sorting-.patch (2.6K, 4-REL_18_0001-test_tidstore-Stabilize-regression-tests-by-sorting-.patch)
download | inline diff:
From dd3bf8933fc1368bfe22b039339a4633e2f510e1 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
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 protected]>
Author: Andrei Lepikhov <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
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 eb16e0fbfa6..d12d54f3677 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
@@ -179,6 +192,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.54.0
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-17 21:26 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-22 16:51 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-22 17:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-24 23:23 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
2026-04-26 09:06 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-28 17:09 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
@ 2026-04-29 16:11 ` Masahiko Sawada <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Masahiko Sawada @ 2026-04-29 16:11 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On Tue, Apr 28, 2026 at 10:09 AM Masahiko Sawada <[email protected]> wrote:
>
> On Sun, Apr 26, 2026 at 2:06 AM Andrei Lepikhov <[email protected]> wrote:
> >
> > On 25/04/2026 01:23, Masahiko Sawada wrote:
> > > On Wed, Apr 22, 2026 at 10:23 AM Andrei Lepikhov <[email protected]> wrote:
> > >> 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.
> > >
> > > 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?
> >
> > 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.
> >
> > >
> > > 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.
> >
> > 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.
>
> 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.
>
Pushed.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2026-04-29 16:11 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-16 07:13 Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-16 08:11 ` Masahiko Sawada <[email protected]>
2026-04-16 08:26 ` Andrei Lepikhov <[email protected]>
2026-04-16 17:58 ` Masahiko Sawada <[email protected]>
2026-04-17 21:26 ` Andrei Lepikhov <[email protected]>
2026-04-22 16:51 ` Masahiko Sawada <[email protected]>
2026-04-22 17:23 ` Andrei Lepikhov <[email protected]>
2026-04-24 23:23 ` Masahiko Sawada <[email protected]>
2026-04-26 09:06 ` Andrei Lepikhov <[email protected]>
2026-04-28 17:09 ` Masahiko Sawada <[email protected]>
2026-04-29 16:11 ` Masahiko Sawada <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox