Обсуждение: pgsql: Remove obsolete executor cleanup code
Remove obsolete executor cleanup code This commit removes unnecessary ExecExprFreeContext() calls in ExecEnd* routines because the actual cleanup is managed by FreeExecutorState(). With no callers remaining for ExecExprFreeContext(), this commit also removes the function. This commit also drops redundant ExecClearTuple() calls, because ExecResetTupleTable() in ExecEndPlan() already takes care of resetting and dropping all TupleTableSlots initialized with ExecInitScanTupleSlot() and ExecInitExtraTupleSlot(). After these modifications, the ExecEnd*() routines for ValuesScan, NamedTuplestoreScan, and WorkTableScan became redundant. So, this commit removes them. Reviewed-by: Robert Haas Discussion: https://postgr.es/m/CA+HiwqFGkMSge6TgC9KQzde0ohpAycLQuV7ooitEEpbKB0O_mg@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/d060e921ea5aa47b6265174c32e1128cebdbc3df Modified Files -------------- src/backend/executor/execProcnode.c | 18 ++++++------------ src/backend/executor/execUtils.c | 26 -------------------------- src/backend/executor/nodeAgg.c | 10 ---------- src/backend/executor/nodeBitmapHeapscan.c | 12 ------------ src/backend/executor/nodeBitmapIndexscan.c | 8 -------- src/backend/executor/nodeCtescan.c | 12 ------------ src/backend/executor/nodeCustom.c | 7 ------- src/backend/executor/nodeForeignscan.c | 8 -------- src/backend/executor/nodeFunctionscan.c | 15 --------------- src/backend/executor/nodeGather.c | 3 --- src/backend/executor/nodeGatherMerge.c | 3 --- src/backend/executor/nodeGroup.c | 5 ----- src/backend/executor/nodeHash.c | 5 ----- src/backend/executor/nodeHashjoin.c | 12 ------------ src/backend/executor/nodeIncrementalSort.c | 5 ----- src/backend/executor/nodeIndexonlyscan.c | 16 ---------------- src/backend/executor/nodeIndexscan.c | 16 ---------------- src/backend/executor/nodeLimit.c | 1 - src/backend/executor/nodeMaterial.c | 5 ----- src/backend/executor/nodeMemoize.c | 9 --------- src/backend/executor/nodeMergejoin.c | 11 ----------- src/backend/executor/nodeModifyTable.c | 11 ----------- src/backend/executor/nodeNamedtuplestorescan.c | 22 ---------------------- src/backend/executor/nodeNestloop.c | 10 ---------- src/backend/executor/nodeProjectSet.c | 10 ---------- src/backend/executor/nodeResult.c | 10 ---------- src/backend/executor/nodeSamplescan.c | 12 ------------ src/backend/executor/nodeSeqscan.c | 12 ------------ src/backend/executor/nodeSetOp.c | 4 ---- src/backend/executor/nodeSort.c | 7 ------- src/backend/executor/nodeSubqueryscan.c | 12 ------------ src/backend/executor/nodeTableFuncscan.c | 12 ------------ src/backend/executor/nodeTidrangescan.c | 12 ------------ src/backend/executor/nodeTidscan.c | 12 ------------ src/backend/executor/nodeUnique.c | 5 ----- src/backend/executor/nodeValuesscan.c | 24 ------------------------ src/backend/executor/nodeWindowAgg.c | 17 ----------------- src/backend/executor/nodeWorktablescan.c | 22 ---------------------- src/include/executor/executor.h | 1 - src/include/executor/nodeNamedtuplestorescan.h | 1 - src/include/executor/nodeValuesscan.h | 1 - src/include/executor/nodeWorktablescan.h | 1 - 42 files changed, 6 insertions(+), 419 deletions(-)
Amit Langote <amitlan@postgresql.org> writes:
> After these modifications, the ExecEnd*() routines for ValuesScan,
> NamedTuplestoreScan, and WorkTableScan became redundant. So, this
> commit removes them.
This seems like quite a bad idea. From a documentation standpoint
alone, it would be far better for these functions to exist but be
empty.
regards, tom lane
On Thu, Sep 28, 2023 at 10:08 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Amit Langote <amitlan@postgresql.org> writes: > > After these modifications, the ExecEnd*() routines for ValuesScan, > > NamedTuplestoreScan, and WorkTableScan became redundant. So, this > > commit removes them. > > This seems like quite a bad idea. From a documentation standpoint > alone, it would be far better for these functions to exist but be > empty. Actually, the documentation aspect of this change does bother me a bit, but I also agree with Robert's preference to remove such empty functions altogether to shave some albeit few CPU cycles off [1]. I am fine with reverting this part if you insist though. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com