Executor code - found an instance of a WHILE that should just be an IF
Hi,
During debugging I noticed some code in ExecResult() where a WHILE
loop is being used with an unconditional RETURN at the end of the
block (which is intentional, looking at the history of changes), but
now there's no actual use of the loop in any way. The code should
probably be changed to just use IF for clarity.
I've attached a patch.
Regards,
Greg Nancarrow
Fujitsu Australia
Attachments:
v1-0001-Change-an-instance-of-WHILE-to-IF-in-executor-code.patchapplication/octet-stream; name=v1-0001-Change-an-instance-of-WHILE-to-IF-in-executor-code.patchDownload
From 1edbbba56238e736700720744bb4e98e8e771be6 Mon Sep 17 00:00:00 2001
From: Greg Nancarrow <gregn4422@gmail.com>
Date: Mon, 10 May 2021 19:00:17 +1000
Subject: [PATCH] Change one instance of WHILE in executor code, that should
really just be an IF condition.
The existing WHILE loop has an unconditional RETURN at the end of the code block
and functions the same as an IF condition, so should be changed to IF for
clarity.
---
src/backend/executor/nodeResult.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/executor/nodeResult.c b/src/backend/executor/nodeResult.c
index 1762b87c99..0946af0a54 100644
--- a/src/backend/executor/nodeResult.c
+++ b/src/backend/executor/nodeResult.c
@@ -103,7 +103,7 @@ ExecResult(PlanState *pstate)
* called, OR that we failed the constant qual check. Either way, now we
* are through.
*/
- while (!node->rs_done)
+ if (!node->rs_done)
{
outerPlan = outerPlanState(node);
--
2.27.0
On Mon, 10 May 2021 at 21:16, Greg Nancarrow <gregn4422@gmail.com> wrote:
During debugging I noticed some code in ExecResult() where a WHILE
loop is being used with an unconditional RETURN at the end of the
block (which is intentional, looking at the history of changes), but
now there's no actual use of the loop in any way. The code should
probably be changed to just use IF for clarity.
I've attached a patch.
Looks like leftovers from ea15e1867.
I don't think this will affect any code generation but you are right,
it should be an "if".
David
On Mon, 10 May 2021 at 23:49, David Rowley <dgrowleyml@gmail.com> wrote:
On Mon, 10 May 2021 at 21:16, Greg Nancarrow <gregn4422@gmail.com> wrote:
During debugging I noticed some code in ExecResult() where a WHILE
loop is being used with an unconditional RETURN at the end of the
block (which is intentional, looking at the history of changes), but
now there's no actual use of the loop in any way. The code should
probably be changed to just use IF for clarity.
I've attached a patch.Looks like leftovers from ea15e1867.
I don't think this will affect any code generation but you are right,
it should be an "if".
Since there's no bug fix here, I thought that there's not much point
in backpatching this.
Does anyone object to making this small change in master?
David
On Thu, May 13, 2021 at 08:20:36PM +1200, David Rowley wrote:
Since there's no bug fix here, I thought that there's not much point
in backpatching this.
Indeed. I would not bother with a back-patch either.
Does anyone object to making this small change in master?
No objections from here.
--
Michael
On Thu, May 13, 2021 at 08:06:18PM +0900, Michael Paquier wrote:
On Thu, May 13, 2021 at 08:20:36PM +1200, David Rowley wrote:
Since there's no bug fix here, I thought that there's not much point
in backpatching this.Indeed. I would not bother with a back-patch either.
Does anyone object to making this small change in master?
No objections from here.
+1 to both.
On Fri, 14 May 2021 at 00:27, Julien Rouhaud <rjuju123@gmail.com> wrote:
On Thu, May 13, 2021 at 08:06:18PM +0900, Michael Paquier wrote:
On Thu, May 13, 2021 at 08:20:36PM +1200, David Rowley wrote:
Since there's no bug fix here, I thought that there's not much point
in backpatching this.Indeed. I would not bother with a back-patch either.
Does anyone object to making this small change in master?
No objections from here.
+1 to both.
Thanks for the votes. Pushed.
David