Re: BUG #4553: HOLD cursors not materializing results fully

"Andrew Gierth" <andrew(at)tao11(dot)riddles(dot)org(dot)uk> writes:
> The materialization logic for holdable cursors isn't detoasting data prior
> to storage in the portal's tuplestore, which leads to problems like this:

Hmm ... I think we'd seen this reported once before in the context of
dropping the cursor's source table, which didn't seem tremendously
pressing to fix. However, an example involving just VACUUM obviously
raises the priority a bit.

I concur that the only reasonable fix is to detoast any out-of-line
datums (fortunately we don't have to decompress them too) while pushing
the data into the hold store. It doesn't seem too difficult to make
this happen in tstoreReceiver.c, but there's an API issue: we use
tstoreReceiver.c also for a number of cases with regular non-holdable
Portals, and we surely don't want to incur the detoasting overhead in
those cases. So it seems we need one more parameter for creation of
a tuplestore DestReceiver, to tell it whether to detoast or not.

ISTM this puts the final nail in the coffin of the present API of
CreateDestReceiver: the majority of its callers are going to be working
around its inadequate ability to pass receiver-type-specific arguments.

There are a number of possible solutions: abandon CreateDestReceiver
altogether, allow it to pass through a "void *" parameter, etc. What
I'm kind of inclined to do is just to remove its Portal argument and
require callers that need to pass information to call a
receiver-type-specific "set parameters" function after the basic
CreateDestReceiver step.

Or we could leave CreateDestReceiver alone and make tuplestore
DestReceiver creation examine the state of the portal to decide
what to do; but that seems pretty ugly/fragile. It might be the
way to do it in the back branches, though, to avoid an API change
there. (I have no idea if there's any third-party code calling
CreateDestReceiver...)

Comments, better ideas?

regards, tom lane