Date: Mon, 8 Nov 2021 14:13:38 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: pgsql-hackers@postgresql.org, Andres Freund <andres@anarazel.de>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>
Subject: Re: make tuplestore helper function
Message-ID: <20211108201338.GU17618@telsasoft.com>
References: 
 <CAAKRu_azyd1Z3W_r7Ou4sorTjRCs+PxeHw1CWJeXKofkE6TuZg@mail.gmail.com>
 <20211102202353.GG17618@telsasoft.com>
 <CAAKRu_a-ZZTez9yn4EMDAQQgt1UFpFcTVr=MDxf-b75tZ9q8UQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAAKRu_a-ZZTez9yn4EMDAQQgt1UFpFcTVr=MDxf-b75tZ9q8UQ@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20211108201338.GU17618%40telsasoft.com>
Precedence: bulk

On Mon, Nov 08, 2021 at 02:52:28PM -0500, Melanie Plageman wrote:
> On Tue, Nov 2, 2021 at 4:23 PM Justin Pryzby <pryzby@telsasoft.com> 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_ARGS)
> > > -     tupstore =
> > > -             tuplestore_begin_heap(rsinfo->allowedModes & SFRM_Materialize_Random,
> > > -                                                       false, work_mem);
> >
> > > @@ -2701,42 +2701,13 @@ pg_hba_file_rules(PG_FUNCTION_ARGS)
> > > -     tuple_store =
> > > -             tuplestore_begin_heap(rsi->allowedModes & SFRM_Materialize_Random,
> > > -                                                       false, work_mem);
> >
> > > @@ -4799,31 +4797,8 @@ pg_timezone_names(PG_FUNCTION_ARGS)
> > > -     randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
> > > -     tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
> >
> > > @@ -575,38 +575,12 @@ pg_ls_dir_1arg(PG_FUNCTION_ARGS)
> > > -     randomAccess = (rsinfo->allowedModes & SFRM_Materialize_Random) != 0;
> > > -     tupstore = tuplestore_begin_heap(randomAccess, false, work_mem);
> >
> > > @@ -1170,17 +1154,12 @@ pg_cursor(PG_FUNCTION_ARGS)
> > > -     tupstore =
> > > -             tuplestore_begin_heap(rsinfo->allowedModes & SFRM_Materialize_Random,
> > > -                                                       false, work_mem);
> >
> > > +++ b/src/backend/utils/fmgr/funcapi.c
> > > +     tupstore = tuplestore_begin_heap(true, false, maxKBytes);
> 
> I believe the patch has preserved the same behavior. All of the callers
> for which I replaced tuplestore_begin_heap() which passed a variable for
> the randomAccess parameter had set that variable to something which was
> effectively the same as passing true -- SFRM_Materialize_Random.

I don't think so ?

They callers aren't passing SFRM_Materialize_Random, but rather
(allowedModes & SFRM_Materialize_Random) != 0

Where allowedModes is determined EXEC_FLAG_BACKWARD.

src/include/executor/executor.h:extern Tuplestorestate *ExecMakeTableFunctionResult(SetExprState *setexpr,
src/include/executor/executor.h-                                                    ExprContext *econtext,
src/include/executor/executor.h-                                                    MemoryContext argContext,
src/include/executor/executor.h-                                                    TupleDesc expectedDesc,
src/include/executor/executor.h-                                                    bool randomAccess);

src/backend/executor/nodeFunctionscan.c=FunctionNext(FunctionScanState *node)
src/backend/executor/nodeFunctionscan.c:             ExecMakeTableFunctionResult(node->funcstates[0].setexpr,
src/backend/executor/nodeFunctionscan.c-                                         node->ss.ps.ps_ExprContext,
src/backend/executor/nodeFunctionscan.c-                                         node->argcontext,
src/backend/executor/nodeFunctionscan.c-                                         node->funcstates[0].tupdesc,
src/backend/executor/nodeFunctionscan.c-                                         node->eflags & EXEC_FLAG_BACKWARD);

-- 
Justin