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

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>

To: Peter Geoghegan <pg@bowt.ie>

Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Jeff Davis <pgsql@j-davis.com>, benoit <benoit@hopsandfork.com>, Alexander Korotkov <aekorotkov@gmail.com>

Date: 2024-04-08T01:25:38Z

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

Peter Geoghegan <pg@bowt.ie> writes:
> On Sun, Apr 7, 2024 at 8:48 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Coverity pointed out something that looks like a potentially live
>> problem in 5bf748b86:
>> ... which certainly makes it look like array->scan_key could be
>> a null-pointer dereference.

> But the "Assert(xform[j].ikey == array->scan_key)" assertion is
> located in a block where it's been established that the scan key (the
> one stored in xform[j] at this point in execution) must have an array.

> This is probably very hard for tools like Coverity to understand.

It's not obvious to human readers either ...

> Would Coverity stop complaining if I just removed the assertion? I
> could just do that, I suppose, but that seems backwards to me.

Perhaps this'd help:

-                        Assert(xform[j].ikey == array->scan_key);
+                        Assert(array && xform[j].ikey == array->scan_key);

If that doesn't silence it, I'd be prepared to just dismiss the
warning.

Some work in the comment to explain why we must have an array here
wouldn't be out of place either, perhaps.

			regards, tom lane