Re: make tuplestore helper function

On Sun, Mar 6, 2022 at 9:29 PM Michael Paquier <michael@paquier.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