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: Wed, 22 Apr 2026 09:51:18 -0700
Message-ID: <CAD21AoCmEqiQVgJc34yGK7DSQj-p-zBVrm4-PfrwYHdNkJGt5g@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]>
	<CAD21AoAit-Qp9OriVbb9c_Qebi=Cgxz0AQJu+zxQpp=_1Lt2dQ@mail.gmail.com>
	<[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

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: <CAD21AoCmEqiQVgJc34yGK7DSQj-p-zBVrm4-PfrwYHdNkJGt5g@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