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

Peter Geoghegan <pg@bowt.ie>

From: Peter Geoghegan <pg@bowt.ie>
To: natalya@timescale.com, pgsql-bugs@lists.postgresql.org
Date: 2025-09-11T16:48:06Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Always commute strategy when preprocessing DESC keys.

  2. Avoid extra index searches through preprocessing.

Attachments

On Thu, Sep 11, 2025 at 10:35 AM PG Bug reporting form
<noreply@postgresql.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