Re: Pluggable Storage - Andres's take

On 18/05/2019 01:19, Ashwin Agrawal wrote:
> On Tue, Apr 9, 2019 at 6:17 AM Heikki Linnakangas <hlinnaka@iki.fi 
> <mailto:hlinnaka@iki.fi>> wrote:
> 
>     On 08/04/2019 20:37, Andres Freund wrote:
>      > On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote:
>      >> There's a little bug in index-only scan executor node, where it
>     mixes up the
>      >> slots to hold a tuple from the index, and from the table. That
>     doesn't cause
>      >> any ill effects if the AM uses TTSOpsHeapTuple, but with my toy
>     AM, which
>      >> uses a virtual slot, it caused warnings like this from
>     index-only scans:
>      >
>      > Hm. That's another one that I think I had fixed previously :(,
>     and then
>      > concluded that it's not actually necessary for some reason. Your fix
>      > looks correct to me.  Do you want to commit it? Otherwise I'll
>     look at
>      > it after rebasing zheap, and checking it with that.
> 
>     I found another slot type confusion bug, while playing with
>     zedstore. In
>     an Index Scan, if you have an ORDER BY key that needs to be rechecked,
>     so that it uses the reorder queue, then it will sometimes use the
>     reorder queue slot, and sometimes the table AM's slot, for the scan
>     slot. If they're not of the same type, you get an assertion:
> 
>     TRAP: FailedAssertion("!(op->d.fetch.kind == slot->tts_ops)", File:
>     "execExprInterp.c", Line: 1905)
> 
>     Attached is a test for this, again using the toy table AM, extended to
>     be able to test this. And a fix.
> 
> 
> It seems the two patches from email [1] fixing slot confusion in Index 
> Scans are pending to be committed.
> 
> 1] 
> https://www.postgresql.org/message-id/e71c4da4-3e82-cc4f-32cc-ede387fac8b0%40iki.fi

Pushed the first patch now. Andres already fixed the second issue in 
commit b8b94ea129.

- Heikki