Re: make tuplestore helper function

On Sun, Mar 6, 2022 at 9:29 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Mar 02, 2022 at 03:43:17PM +0900, Michael Paquier wrote:
> > This is actually setting up a function in the context of a single call
> > where we fill the tuplestore with all its values, so instead I have
> > settled down to name that SetSingleFuncCall(), to make a parallel with
> > the existing MultiFuncCall*(). funcapi.c is the right place for
> > that, and I have added more documentation in the fmgr's README as well
> > as funcapi.h.
>
> I have tortured all those code paths for the last couple of days, and
> the new function name, as well as its options, still seemed fine to
> me, so I have applied the patch. The buildfarm is cool with it. It
> is worth noting that there are more code paths in contrib/ that could
> be simplified with this new routine.

Wow! Thanks so much for taking the time to review, refine, and commit
this work.

I've attached a patch using the helper in most locations in contrib
modules that seemed useful.

The following I don't think we can use the helper or it is not worth it:

- pg_logdir_ls() in contrib/adminpack has return type TYPEFUNC_RECORD
and expectedDesc is not already created, so the helper can't be used.

But basically, since it doesn't specify OUT argument names, it has to
do TupleDescInitEntry() itself anyway, I think.

- contrib/tablefunc.c was also not simple to refactor. the various parts
of SetSingleFuncCall are spread throughout different functions in the
file.

- contrib/dblink has one function which returns a tuplestore that was
simple to change (dblink_get_notify()) and I've done so.

However, most of the other creation of tuplestore and tupledescriptors
is in helper functions (like materializeResult()) which separate the
tuplestore creation from the tuple descriptor initialization in a way
that made it hard to do a drop-in replacement with the helper function.

- Melanie