On Sun, 08 Dec 2024 14:25:53 -0500 Tom Lane <t...@sss.pgh.pa.us> wrote:
> I wrote: > > After looking at this further, I think this whole "run_once" > > business is badly designed, worse documented, and redundant > > (as well as buggy). It can be replaced with three self-contained > > lines in ExecutePlan, as per the attached. > > > (Obviously, the API changes in this wouldn't do for the back > > branches. But we could leave those arguments in place as > > unused dummies.) > > Here's a cut-down version that I think would be OK to use in the > back branches. I confirmed both versions of the pach fixed the issue using pgproto. Also, I feel your fix makes the codes easier to understand at least for me. I have a few minor comment: * moving in the specified direction. * * Runs to completion if numberTuples is 0 - * - * Note: the ctid attribute is a 'junk' attribute that is removed before the - * user can see it * ---------------------------------------------------------------- */ This comment remove seems not related to the fix of ExecutePlan. Should this actually have been done by 8a5849b7ff24c? static void -ExecutePlan(EState *estate, - PlanState *planstate, +ExecutePlan(QueryDesc *queryDesc, bool use_parallel_mode, CmdType operation, bool sendTuples, uint64 numberTuples, ScanDirection direction, - DestReceiver *dest, - bool execute_once) + DestReceiver *dest) { + EState *estate = queryDesc->estate; + PlanState *planstate = queryDesc->planstate; TupleTableSlot *slot; uint64 current_tuple_count I guess planstate is removed due to the redundancy that this is included in queryDesc. If so, I think we can also remove use_parallel_mode for the same reason. Regards, Yugo Nagata -- Yugo NAGATA <nag...@sraoss.co.jp>