Re: BUG #19049: Assert failure when using skip arrays on an index key with DESC order
Natalya Aksman <natalya@tigerdata.com>
From: Natalya Aksman <natalya@tigerdata.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: natalya@timescale.com, pgsql-bugs@lists.postgresql.org
Date: 2025-09-11T17:08:55Z
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 →
-
Always commute strategy when preprocessing DESC keys.
- 796962922e69 19 (unreleased) landed
- e457312c0d19 18.0 landed
-
Avoid extra index searches through preprocessing.
- b3f1a13f22f9 18.0 cited
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@bowt.ie> wrote: > 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 >