public inbox for [email protected]
help / color / mirror / Atom feedTRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2+ messages / 2 participants
[nested] [flat]
* TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
@ 2026-04-15 12:48 Andrei Lepikhov <[email protected]>
2026-04-15 20:50 ` Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Masahiko Sawada <[email protected]>
0 siblings, 1 reply; 2+ messages in thread
From: Andrei Lepikhov @ 2026-04-15 12:48 UTC (permalink / raw)
To: PostgreSQL mailing lists <[email protected]>; Masahiko Sawada <[email protected]>
Hi,
While experimenting with query plans, I periodically see test_tidstore
fail on the assertion in TidStoreSetBlockOffsets().
The cause is that the harness function do_set_block_offsets() forwards
the SQL array straight to TidStoreSetBlockOffsets(), which has an
explicit contract:
"The offset numbers 'offsets' must be sorted in ascending order."
array_agg() without ORDER BY gives no such guarantee, and plan shapes
that reshuffle the input can deliver the offsets out of order and trip
the Assert.
The issue is minor and doesn't expose any underlying bug, but it is
still worth fixing: it removes a source of flaky test runs and makes
life easier for extension developers who reuse the same pattern.
Patch attached.
--
regards, Andrei Lepikhov,
pgEdge
From 99c9abb3b751ec3603b6ad8248fb94ec529d2932 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Wed, 15 Apr 2026 14:41:51 +0200
Subject: [PATCH v0] 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.
---
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.53.0
Attachments:
[text/plain] v0-0001-Sort-offsets-in-test_tidstore-s-do_set_block_offs.patch (2.2K, 2-v0-0001-Sort-offsets-in-test_tidstore-s-do_set_block_offs.patch)
download | inline diff:
From 99c9abb3b751ec3603b6ad8248fb94ec529d2932 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Wed, 15 Apr 2026 14:41:51 +0200
Subject: [PATCH v0] 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.
---
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.53.0
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c"
2026-04-15 12:48 TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
@ 2026-04-15 20:50 ` Masahiko Sawada <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Masahiko Sawada @ 2026-04-15 20:50 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: PostgreSQL mailing lists <[email protected]>
On Wed, Apr 15, 2026 at 5:48 AM Andrei Lepikhov <[email protected]> wrote:
>
> Hi,
>
> While experimenting with query plans, I periodically see test_tidstore
> fail on the assertion in TidStoreSetBlockOffsets().
>
> The cause is that the harness function do_set_block_offsets() forwards
> the SQL array straight to TidStoreSetBlockOffsets(), which has an
> explicit contract:
>
> "The offset numbers 'offsets' must be sorted in ascending order."
>
> array_agg() without ORDER BY gives no such guarantee, and plan shapes
> that reshuffle the input can deliver the offsets out of order and trip
> the Assert.
>
> The issue is minor and doesn't expose any underlying bug, but it is
> still worth fixing: it removes a source of flaky test runs and makes
> life easier for extension developers who reuse the same pattern.
>
> Patch attached.
Thank you for the report.
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.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-04-15 20:50 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-15 12:48 TRAP: failed Assert("offsets[i] > offsets[i - 1]"), File: "tidstore.c" Andrei Lepikhov <[email protected]>
2026-04-15 20:50 ` 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