v50-0002-Refactor-visit_nav_exec.partial

application/octet-stream

Filename: v50-0002-Refactor-visit_nav_exec.partial
Type: application/octet-stream
Part: 4
Message: Re: Row pattern recognition
From f63ae05529f960d34c912d09b24ab4847a47d637 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 2 Jul 2026 17:38:24 +0800
Subject: [PATCH v50 2/4] Refactor visit_nav_exec

Call eval_nav_offset_helper twice is enough.  execExprInterp.c offset,
compound_offset value error checking is not necessary.

Based on https://github.com/assam258-5892/postgres/commits/RPR
---
 src/backend/executor/execExprInterp.c | 26 +-------
 src/backend/executor/nodeWindowAgg.c  | 96 ++++++++++++---------------
 src/include/nodes/primnodes.h         | 16 ++---
 3 files changed, 50 insertions(+), 88 deletions(-)

diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 5b6281e9e3..af50a2e16d 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -6020,23 +6020,9 @@ ExecAggPlainTransByRef(AggState *aggstate, AggStatePerTrans pertrans,
 static int64
 rpr_nav_get_compound_offset(ExprEvalStep *op)
 {
-	int64		val;
-
 	Assert(op->d.rpr_nav.offset_value != NULL);
 
-	if (op->d.rpr_nav.offset_isnull[1])
-		ereport(ERROR,
-				errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-				errmsg("row pattern navigation offset must not be null"));
-
-	val = DatumGetInt64(op->d.rpr_nav.offset_value[1]);
-
-	if (val < 0)
-		ereport(ERROR,
-				errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				errmsg("row pattern navigation offset must not be negative"));
-
-	return val;
+	return DatumGetInt64(op->d.rpr_nav.offset_value[1]);
 }
 
 /*
@@ -6069,17 +6055,7 @@ ExecEvalRPRNavSet(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
 	 */
 	if (op->d.rpr_nav.offset_value != NULL)
 	{
-		if (*op->d.rpr_nav.offset_isnull)
-			ereport(ERROR,
-					errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
-					errmsg("row pattern navigation offset must not be null"));
-
 		offset = DatumGetInt64(*op->d.rpr_nav.offset_value);
-
-		if (offset < 0)
-			ereport(ERROR,
-					errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					errmsg("row pattern navigation offset must not be negative"));
 	}
 	else
 	{
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c
index 8995329949..fec21c2835 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -3990,8 +3990,7 @@ put_notnull_info(WindowObject winobj, int64 pos, int argno, bool isnull)
  * is returned as a harmless placeholder.
  */
 static int64
-eval_nav_offset_helper(WindowAggState *winstate, Expr *offset_expr,
-					   int64 defaultOffset)
+eval_nav_offset_helper(WindowAggState *winstate, Expr *offset_expr)
 {
 	ExprContext *econtext = winstate->ss.ps.ps_ExprContext;
 	ExprState  *estate;
@@ -3999,18 +3998,20 @@ eval_nav_offset_helper(WindowAggState *winstate, Expr *offset_expr,
 	bool		isnull;
 	int64		offset;
 
-	if (offset_expr == NULL)
-		return defaultOffset;
-
 	estate = ExecInitExpr(offset_expr, (PlanState *) winstate);
 	val = ExecEvalExprSwitchContext(estate, econtext, &isnull);
 
 	if (isnull)
-		return 0;
+		ereport(ERROR,
+				errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+				errmsg("row pattern navigation offset must not be null"));
 
 	offset = DatumGetInt64(val);
+
 	if (offset < 0)
-		return 0;
+		ereport(ERROR,
+				errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				errmsg("row pattern navigation offset must not be negative"));
 
 	return offset;
 }
@@ -4048,6 +4049,9 @@ typedef struct
 static void
 visit_nav_exec(NavTraversal *t, RPRNavExpr *nav)
 {
+	int64		inner = 0;
+	int64		outer = 1;
+
 	EvalDefineOffsetsContext *context = (EvalDefineOffsetsContext *) t->data;
 
 	/*
@@ -4060,72 +4064,54 @@ visit_nav_exec(NavTraversal *t, RPRNavExpr *nav)
 	Assert(nav->compound_offset_arg == NULL ||
 		   !IsA(nav->compound_offset_arg, RPRNavExpr));
 
+	if (nav->offset_arg != NULL)
+		inner = eval_nav_offset_helper(context->winstate,
+									   nav->offset_arg);
+	else if (nav->kind == RPR_NAV_PREV || nav->kind == RPR_NAV_NEXT)
+		inner = 1;
+
+	if (nav->compound_offset_arg != NULL)
+		outer = eval_nav_offset_helper(context->winstate,
+									   nav->compound_offset_arg);
+
 	/* Backward reach: PREV, LAST-with-offset */
 	if (!context->maxOverflow)
 	{
-		int64		reach = 0;
-		bool		gotReach = false;
-
-		if (nav->kind == RPR_NAV_PREV)
-		{
-			reach = eval_nav_offset_helper(context->winstate,
-										   nav->offset_arg, 1);
-			gotReach = true;
-		}
-		else if (nav->kind == RPR_NAV_LAST && nav->offset_arg != NULL)
-		{
-			reach = eval_nav_offset_helper(context->winstate,
-										   nav->offset_arg, 0);
-			gotReach = true;
-		}
-		else if (nav->kind == RPR_NAV_PREV_LAST ||
-				 nav->kind == RPR_NAV_NEXT_LAST)
+		if (nav->kind == RPR_NAV_PREV ||
+			nav->kind == RPR_NAV_LAST ||
+			nav->kind == RPR_NAV_PREV_LAST ||
+			nav->kind == RPR_NAV_NEXT_LAST)
 		{
-			int64		inner = eval_nav_offset_helper(context->winstate,
-													   nav->offset_arg, 0);
-			int64		outer = eval_nav_offset_helper(context->winstate,
-													   nav->compound_offset_arg, 1);
+			int64		reach = 0;
 
-			if (nav->kind == RPR_NAV_PREV_LAST)
+			if (nav->kind == RPR_NAV_PREV || nav->kind == RPR_NAV_LAST)
+				reach = inner;
+			else if (nav->kind == RPR_NAV_PREV_LAST)
 			{
 				if (pg_add_s64_overflow(inner, outer, &reach))
 					context->maxOverflow = true;
-				else
-					gotReach = true;
 			}
 			else
-			{
 				reach = Max(inner - outer, 0);
-				gotReach = true;
-			}
-		}
 
-		if (gotReach)
-			context->maxOffset = Max(context->maxOffset, reach);
+			if (!context->maxOverflow)
+				context->maxOffset = Max(context->maxOffset, reach);
+		}
 	}
 
 	/* Forward reach from match_start: FIRST, compound PREV_FIRST/NEXT_FIRST */
-	if (nav->kind == RPR_NAV_FIRST)
+	if (nav->kind == RPR_NAV_FIRST ||
+		nav->kind == RPR_NAV_PREV_FIRST ||
+		nav->kind == RPR_NAV_NEXT_FIRST)
 	{
-		int64		reach;
-
-		reach = eval_nav_offset_helper(context->winstate,
-									   nav->offset_arg, 0);
+		int64		reach = 0;
 
 		context->hasFirst = true;
 
-		context->minFirstOffset = Min(context->minFirstOffset, reach);
-	}
-	else if (nav->kind == RPR_NAV_PREV_FIRST ||
-			 nav->kind == RPR_NAV_NEXT_FIRST)
-	{
-		int64		inner = eval_nav_offset_helper(context->winstate,
-												   nav->offset_arg, 0);
-		int64		outer = eval_nav_offset_helper(context->winstate,
-												   nav->compound_offset_arg, 1);
-		int64		reach;
-
-		if (nav->kind == RPR_NAV_PREV_FIRST)
+		if (nav->kind == RPR_NAV_FIRST)
+			context->minFirstOffset = Min(context->minFirstOffset,
+										  inner);
+		else if (nav->kind == RPR_NAV_PREV_FIRST)
 		{
 			/*
 			 * reach = inner - outer.  Both are non-negative, so the result >=
@@ -4143,7 +4129,7 @@ visit_nav_exec(NavTraversal *t, RPRNavExpr *nav)
 			if (pg_add_s64_overflow(inner, outer, &reach))
 				reach = PG_INT64_MAX;
 		}
-		context->hasFirst = true;
+
 		context->minFirstOffset = Min(context->minFirstOffset, reach);
 	}
 }
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 8576a3c9c5..8ba530333b 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -668,15 +668,15 @@ typedef struct WindowFuncRunCondition
  */
 typedef enum RPRNavKind
 {
-	RPR_NAV_PREV,
-	RPR_NAV_NEXT,
-	RPR_NAV_FIRST,
-	RPR_NAV_LAST,
+	RPR_NAV_PREV,				/* offset default: 1 */
+	RPR_NAV_NEXT,				/* offset default: 1 */
+	RPR_NAV_FIRST,				/* offset default: 0 */
+	RPR_NAV_LAST,				/* offset default: 0 */
 	/* compound: outer(inner(arg)) */
-	RPR_NAV_PREV_FIRST,
-	RPR_NAV_PREV_LAST,
-	RPR_NAV_NEXT_FIRST,
-	RPR_NAV_NEXT_LAST,
+	RPR_NAV_PREV_FIRST,			/* (offset, compound_offset) default: (0, 1) */
+	RPR_NAV_PREV_LAST,			/* (offset, compound_offset) default: (0, 1) */
+	RPR_NAV_NEXT_FIRST,			/* (offset, compound_offset) default: (0, 1) */
+	RPR_NAV_NEXT_LAST,			/* (offset, compound_offset) default: (0, 1) */
 } RPRNavKind;
 
 typedef struct RPRNavExpr
-- 
2.34.1