Attached is a patch to create a helper function which creates a
tuplestore to be used by FUNCAPI-callable functions.
It was suggested in [1] that I start a separate thread for this patch.
A few notes:
There are a few places with very similar code to the new helper
(MakeTuplestore()) but which, f
The attached patch contains a helper now named
MakeFuncResultTuplestore() which aims to eliminate a few lines of
redundant code that all FUNCAPI functions making tuplestores repeated.
While investigating all of these call sites and making the changes
suggested by Álvaro, I noticed that the functio
On Tue, Nov 2, 2021 at 4:23 PM Justin Pryzby wrote:
>
> Several places have a conditional value for the first argument (randomAccess),
> but your patch changes the behavior to a constant "true". I didn't review the
> patch beyond that.
>
> > @@ -740,18 +724,14 @@ pg_prepared_statement(PG_FUNCTION
On Mon, Nov 08, 2021 at 02:52:28PM -0500, Melanie Plageman wrote:
> On Tue, Nov 2, 2021 at 4:23 PM Justin Pryzby wrote:
> >
> > Several places have a conditional value for the first argument
> > (randomAccess),
> > but your patch changes the behavior to a constant "true". I didn't review
> > th
On Mon, Nov 8, 2021 at 3:13 PM Justin Pryzby wrote:
>
> On Mon, Nov 08, 2021 at 02:52:28PM -0500, Melanie Plageman wrote:
> > On Tue, Nov 2, 2021 at 4:23 PM Justin Pryzby wrote:
> > >
> > > Several places have a conditional value for the first argument
> > > (randomAccess),
> > > but your patch
On Thu, Nov 18, 2021 at 12:59:03PM -0500, Melanie Plageman wrote:
> On Mon, Nov 8, 2021 at 3:13 PM Justin Pryzby wrote:
> > On Mon, Nov 08, 2021 at 02:52:28PM -0500, Melanie Plageman wrote:
> > > On Tue, Nov 2, 2021 at 4:23 PM Justin Pryzby wrote:
> > > >
> > > > Several places have a conditional
On Thu, Nov 18, 2021 at 1:24 PM Justin Pryzby wrote:
>
> On Thu, Nov 18, 2021 at 12:59:03PM -0500, Melanie Plageman wrote:
> > On Mon, Nov 8, 2021 at 3:13 PM Justin Pryzby wrote:
> > > On Mon, Nov 08, 2021 at 02:52:28PM -0500, Melanie Plageman wrote:
> > > > On Tue, Nov 2, 2021 at 4:23 PM Justin
On Mon, Dec 13, 2021 at 05:27:52PM -0500, Melanie Plageman wrote:
> Done in attached v4
Thanks.
I think these comments can be removed from the callers:
/* it's a simple type, so don't use get_call_result_type */
You removed one call to tuplestore_donestoring(), but not the others.
I guess you co
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
Several places have a conditional value for the first argument (randomAccess),
but your patch changes the behavior to a constant "true". I didn't review the
patch beyond that.
> @@ -740,18 +724,14 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
> - tupstore =
> - tuplestore_begin_heap(
Thanks for the detailed review!
On Fri, Dec 17, 2021 at 3:04 PM Justin Pryzby wrote:
>
> On Mon, Dec 13, 2021 at 05:27:52PM -0500, Melanie Plageman wrote:
> > Done in attached v4
>
> Thanks.
>
> I think these comments can be removed from the callers:
> /* it's a simple type, so don't use get_call
On Wed, Jan 05, 2022 at 12:09:16PM -0500, Melanie Plageman wrote:
> On Fri, Dec 17, 2021 at 3:04 PM Justin Pryzby wrote:
> > The rest of these are questions more than comments, and I'm not necessarily
> > suggesting to change the patch:
> >
> > This isn't new in your patch, but for me, it's more c
On Wed, Jan 5, 2022 at 7:57 PM Justin Pryzby wrote:
>
> On Wed, Jan 05, 2022 at 12:09:16PM -0500, Melanie Plageman wrote:
> > On Fri, Dec 17, 2021 at 3:04 PM Justin Pryzby wrote:
> > > There's a couples places that you're checking expectedDesc where it wasn't
> > > being checked before. Is that
On Tue, Jan 11, 2022 at 10:19:33AM -0500, Melanie Plageman wrote:
> On Wed, Jan 5, 2022 at 7:57 PM Justin Pryzby wrote:
>> I'd leave it for input from a committer about those:
>>
>> - remove tuplestore_donestoring() calls ?
This part has been raised on a different thread, as of:
https://www.post
On Tue, Jan 11, 2022 at 10:19:33AM -0500, Melanie Plageman wrote:
> > If the expectedDesc NULL check were an 0001 patch, then 0002 (the main
> > patch)
> > would be even easier to review. Only foreign.c is different.
>
> I'll wait to do that if preferred by committer.
> Are you imagining that pa
On Tue, Feb 15, 2022 at 11:57:56PM -0600, Justin Pryzby wrote:
> On Tue, Jan 11, 2022 at 10:19:33AM -0500, Melanie Plageman wrote:
>> I'll wait to do that if preferred by committer.
>> Are you imagining that patch 0001 would only add the check for
>> expectedDesc that is missing from pg_config() an
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 m
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 pa
On Thu, Feb 24, 2022 at 08:25:06PM +0900, Michael Paquier wrote:
> This is the remaining piece, as attached, that I have not been able to
> poke much at yet.
So, I have finally poked at this last part of the patch set, and I
found that we can be more aggressive with the refactoring, by moving
into
On Mon, Feb 28, 2022 at 04:49:41PM +0900, Michael Paquier wrote:
> In order to keep things pluggable at will, MakeFuncResultTuplestore()
> has been changed to access a set of bits32 flags, able to control the
> two options above. With this facility in place, I have been able to
> cut much more cod
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 Mul
On Sun, Mar 6, 2022 at 9:29 PM Michael Paquier 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 tha
On Mon, Mar 07, 2022 at 05:09:19PM -0500, Melanie Plageman wrote:
> I've attached a patch using the helper in most locations in contrib
> modules that seemed useful.
Thanks for the patch. I was also looking at that yesterday, and this
pretty much maps to what I have finished with, except for dbli
23 matches
Mail list logo
