On 2019-11-08 17:59, Robert Haas wrote:
On Wed, Nov 6, 2019 at 12:00 PM Andres Freund <and...@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


Reply via email to