From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Andres Freund <andres@anarazel.de>
Subject: Re: make tuplestore helper function
Date: Tue, 2 Nov 2021 15:17:05 -0300
Message-ID: <202111021817.ngdvsnnsmkq5@alvherre.pgsql> (raw)
In-Reply-To: <CAAKRu_azyd1Z3W_r7Ou4sorTjRCs+PxeHw1CWJeXKofkE6TuZg@mail.gmail.com>
On 2021-Nov-02, Melanie Plageman wrote:
> Attached is a patch to create a helper function which creates a
> tuplestore to be used by FUNCAPI-callable functions.
Looks good, and given the amount of code being removed, it seems a
no-brainer that some helper(s) is/are needed.
I think the name is a bit too generic. How about
MakeFuncResultTuplestore()?
I think the function should not modify rsinfo -- having that as
(undocumented) side effect is weird. I would suggest to just return the
tuplestore, and have the caller stash it in rsinfo->setResult and modify
the other struct members alongside that. It's a couple more lines per
caller, but the code is clearer that way.
> There are a few places with very similar code to the new helper
> (MakeTuplestore()) but which, for example, don't expect the return type
> to be a row (pg_ls_dir(), pg_tablespace_databases(), text_to_table()). I
> wasn't sure if it was worth modifying it for their benefit.
Is this just because of the tupdesc? If that's the case, then maybe the
tupdesc can be optionally passed in by caller, and when it's not, the
helper does get_call_result_type() and the tupdesc is used as output
argument.
> I wasn't sure if the elog() check for call result type should be changed
> to an ereport().
I don't see a reason for that: it's still an internal (not user-facing) error.
> All callers of MakeTuplestore() pass work_mem as the third parameter, so
> I could just omit it and hard-code it in, but I wasn't sure that felt
> right in a utility/helper function.
No opinion. I would have used the global in the function instead of
passing it in.
> I inlined deflist_to_tuplestore() in foreign.c since it was billed as a
> helper function but wasn't used elsewhere and it made it harder to use
> MakeTuplestore() in this location.
This is a bit strange. Does it work to pass rsinfo->expectedDesc?
--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
"No renuncies a nada. No te aferres a nada."
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, melanieplageman@gmail.com, andres@anarazel.de
Subject: Re: make tuplestore helper function
In-Reply-To: <202111021817.ngdvsnnsmkq5@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox