pgsql: Remove obsolete executor cleanup code

Started by Amit Langoteover 2 years ago3 messagescomitters
Jump to latest
#1Amit Langote
Langote_Amit_f8@lab.ntt.co.jp

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: /messages/by-id/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(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Amit Langote (#1)
Re: pgsql: Remove obsolete executor cleanup code

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

#3Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Tom Lane (#2)
Re: pgsql: Remove obsolete executor cleanup code

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