On Mon, Nov 8, 2021 at 3:13 PM Justin Pryzby <pry...@telsasoft.com> 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 <pry...@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); >
You are right. I misread it. So, I've attached a patch where randomAccess is now an additional parameter (and registered for the next fest). I was thinking about how to add a test that would have broken when I passed true for randomAccess to tuplestore_begin_heap() when false was required. But, I don't fully understand the problem. If backward accesses to a tuplestore are not allowed but randomAccess is mistakenly passed as true, would the potential result be potentially wrong results from accessing the tuplestore results backwards? - Melanie
v3-0001-Add-helper-to-make-tuplestore.patch
Description: Binary data
