On Mon, Feb 21, 2022 at 04:41:17PM +0900, Michael Paquier wrote:
> So, I got my hands on this area, and found myself applying 07daca5 as
> a first piece of the puzzle. Anyway, after more review today, I have
> bumped into more pieces that could be consolidated, and finished with
> the following patch set:
> - 0001 changes a couple of SRFs that I found worth simplifying. These
> refer mostly to the second and fourth group mentioned upthread by
> Melanie, with two exceptions: pg_tablespace_databases() where it is
> not worth changing to keep it backward-compatible and pg_ls_dir() as
> per its one-arg version. That's a nice first cut in itself.
> - 0002 changes a couple of places to unify some existing SRF checks,
> that I bumped into on the way. The value here is in using the same
> error messages everywhere, reducing the translation effort and
> generating the same errors for all cases based on the same conditions.
> This eases the review of the next patch.
These two have been now applied, with some comment improvements and
the cleanup of pg_options_to_table() done in 0001, and a slight change
in 0002 for pageinspect where a check was not necessary for a BRIN
code path.
> - 0003 is the actual refactoring meat, where I have been able to move
> the check on expectedDesc into MakeFuncResultTuplestore(), shaving
> more code than previously. If you discard the cases of patch 0001, it
> should actually be possible to set setResult, setDesc and returnMode
> within the new function, which would feel natural as that's the place
> where we create the function's tuplestore and we want to materialize
> its contents. The cases with the JSON code are also a bit hairy and
> need more thoughts, but we could also cut this part of the code from
> the initial refactoring effort.
This is the remaining piece, as attached, that I have not been able to
poke much at yet.
--
Michael
