Re: BUG #19049: Assert failure when using skip arrays on an index key with DESC order

The patch looks good and fixes our failing tests.

Thank you,
Natalya Aksman.

On Thu, Sep 11, 2025 at 12:48 PM Peter Geoghegan <pg(at)bowt(dot)ie> wrote:

> On Thu, Sep 11, 2025 at 10:35 AM PG Bug reporting form
> <noreply(at)postgresql(dot)org> wrote:
> > Assert is:
> > if (ScanDirectionIsForward(dir) && array->low_compare)
> > Assert(DatumGetBool(FunctionCall2Coll(&array->low_compare->sk_func,
> > array->low_compare->sk_collation,
> > tupdatum,
> > array->low_compare->sk_argument)));
> >
> > The problem is "array->low_compare" changing "t1<10" to "t1>10" because
> t1
> > order is DESC.
>
> This explanation is correct: I simply neglected to account for the way
> that DESC column scalar keys (used as a skip array's
> low_compare/high_compare) will have already had their operator
> strategy inverted, at the point where preprocessing converts a
> low_compare > into an equivalent >= (and/or a high_compare < into an
> equivalent <=) as an optimization. As Tom pointed out, this is a bug
> in the mechanism added by commit b3f1a13f (not the main skip scan
> commit)
>
> A draft fix is attached. This fixes the crash that your test case
> exhibits, and passes all of my existing tests. I'll commit something
> along these lines within the next few days, barring any objections.
>
> I'm going to revisit my test coverage for DESC columns in light of
> this issue. I didn't add support for DESC columns in my fuzz testing
> scripts, which now seems like an oversight -- I'd definitely have
> caught this problem myself, had I done that from the start.
>
> --
> Peter Geoghegan
>