Re: make tuplestore helper function

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Justin Pryzby <pryzby(at)telsasoft(dot)com>
Cc: Melanie Plageman <melanieplageman(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Subject: Re: make tuplestore helper function
Date: 2022-02-21 07:41:17
Message-ID: YhNB+OvCi9/AMqfo@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Feb 17, 2022 at 04:10:01PM +0900, Michael Paquier wrote:
> Asserting that we are in the correct memory context in when calling
> MakeFuncResultTuplestore() sounds rather sensible from here as per the
> magics done in the various json functions. Still, it really feels
> like we could do a more centralized consolidation of the whole.

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.
- 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.

So, for now, 0001 and 0002 look like rather committable pieces. 0003
needs a bit more thoughts about all the corner cases we need to
consider, mostly what I am mentioning above. Perhaps the pieces of
0003 related to pg_options_to_table() should be moved to 0001 as a
matter of clarity.
--
Michael

Attachment Content-Type Size
v7-0001-Clean-up-and-simplify-some-code-related-to-SRFs.patch text/x-diff 9.7 KB
v7-0002-Simplify-set-of-SRF-related-checks.patch text/x-diff 8.5 KB
v7-0003-Introduce-MakeFuncResultTuplestore.patch text/x-diff 46.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message kato-sho@fujitsu.com 2022-02-21 07:41:58 RE: Synchronizing slots from primary to standby
Previous Message Andy Fan 2022-02-21 07:31:22 Re: Condition pushdown: why (=) is pushed down into join, but BETWEEN or >= is not?