On 7/4/24 6:26 PM, Daniel Gustafsson wrote:
2) We could generate functions which return void rather than NULL and therefore 
not have to do a return at all but I am not sure that small optimization and 
extra clarity would be worth the hassle. The current approach with adding 
Assert() is ok with me. Daniel, what do you think?

I'm not sure that would move the needle enough to warrant the extra complexity.
It could be worth pursuing, but it can be done separately from this.

Agreed.

I looked some more at the patch and have a suggestion for code style. Attaching the diff. Do with them what you wish, for me they make to code easier to read.

Andreas
From 9ea9a07ce6e4faf728ccc4a7b161a70a214601c8 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <andr...@proxel.se>
Date: Thu, 18 Jul 2024 21:54:29 +0200
Subject: [PATCH] Suggested style changes

---
 src/backend/executor/execExprInterp.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 90f37db62e..845d077b6f 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -546,7 +546,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 		EEO_CASE(EEOP_DONE_NO_RETURN)
 		{
 			Assert(isnull == NULL);
-			return 0;
+			return (Datum) 0;
 		}
 
 		EEO_CASE(EEOP_INNER_FETCHSOME)
@@ -1966,13 +1966,10 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
 		{
 			/* unreachable */
 			Assert(false);
-			goto out_error;
+			pg_unreachable();
+			return (Datum) 0;
 		}
 	}
-
-out_error:
-	pg_unreachable();
-	return (Datum) 0;
 }
 
 /*
-- 
2.43.0

Reply via email to