On 2019-11-08 17:59, Robert Haas wrote:
> On Wed, Nov 6, 2019 at 12:00 PM Andres Freund <andres@anarazel.de> wrote:
>> I'd like an AM to have the *option* of implementing something better, or
>> at least go in the direction of making that possible.
>
> OK. Could you see what you think of the attached patches? 0001 does
> some refactoring of toast_fetch_datum() and toast_fetch_datum_slice()
> to make them look more like each other and clean up a bunch of stuff
> that I thought was annoying, and 0002 then pulls out the common logic
> into a heap-specific function. If you like this direction, we could
> then push the heap-specific function below tableam, but I haven't done
> that yet.
Compared to the previous patch (v7) where the API just had a "use this
AM for TOAST" field and the other extreme of pushing TOAST entirely
inside the heap AM, this seems like the worst of both worlds, with the
maximum additional complexity.
I don't think we need to nail down this API for eternity, so I'd be
happy to err on the side of practicality here. However, it seems it's
not quite clear what for example the requirements and wishes from zheap
would be. What's the simplest way to move this forward?
The refactorings you proposed seem reasonable on their own, and I have
some additional comments on that if we decide to go forward in this
direction. One thing that's confusing is that the TOAST tables have
fields chunk_id and chunk_seq, but when an error message talks about
"chunk %d" or "chunk number %d", they usually mean the "seq" and not the
"id".
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services