Re: Avoid full GIN index scan when possible

Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> writes:
> Updated patch is attached. It contains more comments as well as commit message.

I reviewed this a little bit. I agree this seems way more straightforward
than the patches we've been considering so far. I wasn't too happy with
the state of the comments, so I rewrote them a bit in the attached v13.

One thing I'm still not happy about is the comment in
collectMatchesForHeapRow. v12 failed to touch that at all, so I tried to
fill it in, but I'm not sure if my explanation is good. Also, if we know
that excludeOnly keys are going to be ignored, can we save any work in
the main loop of that function?

The test cases needed some work too. Notably, some of the places where
you tried to use EXPLAIN ANALYZE are unportable because they expose "Heap
Blocks" counts that are not stable. (I checked the patch on a 32-bit
machine and indeed some of these failed.) While it'd be possible to work
around that by filtering the EXPLAIN output, that would not be any simpler
or faster than our traditional style of just doing a plain EXPLAIN and a
separate execution.

It troubles me a bit as well that the test cases don't really expose
any difference between patched and unpatched code --- I checked, and
they "passed" without applying any of the code changes. Maybe there's
not much to be done about that, since after all this is an optimization
that's not supposed to change any query results.

I didn't repeat any of the performance testing --- it seems fairly
clear that this can't make any cases worse.

Other than the collectMatchesForHeapRow issue, I think this is
committable.

regards, tom lane