From f63ae05529f960d34c912d09b24ab4847a47d637 Mon Sep 17 00:00:00 2001 From: jian he 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