Re: Optimizing nbtree ScalarArrayOp execution, allowing multi-column ordered scans, skip scan

Peter Geoghegan <pg@bowt.ie>

From: Peter Geoghegan <pg@bowt.ie>

To: Alexander Lakhin <exclusion@gmail.com>

Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Jeff Davis <pgsql@j-davis.com>, benoit <benoit@hopsandfork.com>, Alexander Korotkov <aekorotkov@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>

Date: 2024-08-26T14:58:50Z

Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

Move nbtree preprocessing into new .c file.
- 597b1ffbf123 18.0 landed
Fix nbtree lookahead overflow bug.
- 09a8407dbfd8 18.0 landed
- 6749d4aabe74 17.0 landed
Remove unneeded nbtree array preprocessing assert.
- 480bc6e3ed3a 17.0 landed
Don't try to fix eliminated nbtree array scan keys.
- f22e17f76cf5 17.0 landed
Remove redundant nbtree preprocessing assertions.
- 3b08133cd13c 17.0 landed
Avoid extra lookups with nbtree array inequalities.
- 473411fc5115 17.0 landed
Enhance nbtree ScalarArrayOp execution.
- 5bf748b86bc6 17.0 landed
Improvements and fixes for e0b1ee17dc
- 7e6fb5da41d8 17.0 cited
Skip checking of scan keys required for directional scan in B-tree
- e0b1ee17dc3a 17.0 cited
Fix btmarkpos/btrestrpos array key wraparound bug.
- 714780dcddf0 17.0 cited
Add nbtree high key "continuescan" optimization.
- 29b64d1de7c7 12.0 cited
Consider secondary factors during nbtree splits.
- fab250243387 12.0 cited
Make heap TID a tiebreaker nbtree index column.
- dd299df8189b 12.0 cited
Fix planning of btree index scans using ScalarArrayOpExpr quals.
- 807a40c551dd 9.3.0 cited
Fix btree stop-at-nulls logic properly.
- 882368e854b6 9.2.0 cited
Teach btree to handle ScalarArrayOpExpr quals natively.
- 9e8da0f75731 9.2.0 cited

On Mon, Aug 26, 2024 at 10:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> I'm sorry to bother you again, but I've come across another assertion
> failure.

You've found a real bug. I should be the one apologizing - not you.

> Please try the following query (I use a clean "postgres" database,
> just after initdb):
> EXPLAIN SELECT conname
>    FROM pg_constraint WHERE conname IN ('pkey', 'ID')
>    ORDER BY conname DESC;
>
> SELECT conname
>    FROM pg_constraint WHERE conname IN ('pkey', 'ID')
>    ORDER BY conname DESC;

The problem is that _bt_checkkeys_look_ahead didn't quite get
everything right with sanitizing the page offset number it uses to
check if a later tuple is before the recently advanced array scan
keys. The page offset itself was checked, but in a way that was faulty
in cases where the int16 we use could overflow.

I can fix the bug by making sure that pstate->targetdistance (an int16
variable) can only be doubled when it actually makes sense. That way
there can never be an int16 overflow, and so the final offnum cannot
underflow to a value that's much higher than the page's max offset
number.

This approach works:

     /*
      * The look ahead distance starts small, and ramps up as each call here
      * allows _bt_readpage to skip over more tuples
      */
     if (!pstate->targetdistance)
         pstate->targetdistance = LOOK_AHEAD_DEFAULT_DISTANCE;
-    else
+    else if (pstate->targetdistance < MaxIndexTuplesPerPage / 2)
         pstate->targetdistance *= 2;

I'll push a fix along these lines shortly.

Thanks for the report!
-- 
Peter Geoghegan