v48-0001-Refactor-ParseFuncOrColumn-and-ParseRPRNavCall.nocfbot

application/octet-stream

Filename: v48-0001-Refactor-ParseFuncOrColumn-and-ParseRPRNavCall.nocfbot
Type: application/octet-stream
Part: 0
Message: Re: Row pattern recognition
From d58451ec7cc04783e77ea6f4ceb9c96c2a33db10 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Wed, 17 Jun 2026 13:08:28 +0800
Subject: [PATCH v48 1/1] Refactor ParseFuncOrColumn and ParseRPRNavCall

Simplify ParseFuncOrColumn:
It now routes to ParseRPRNavCall exclusively when ParseExprKind is
EXPR_KIND_RPR_DEFINE and not column projection and list_length(funcname) == 1.
Original behavior is preserved otherwise.

Centralize error handling:
Treat RPR navigation as FUNCDETAIL_NORMAL to reuse the common error handling in
ParseFuncOrColumn, effectively stripping redundant error checks from
ParseRPRNavCall.

Other miscellaneous code cleanups and minor refactoring.
---
 src/backend/parser/parse_func.c        | 225 +++++++++++--------------
 src/test/regress/expected/rpr_base.out |  16 +-
 src/test/regress/sql/rpr_base.sql      |  13 ++
 3 files changed, 125 insertions(+), 129 deletions(-)

diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 0b9d676d64..50d7be58ae 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -124,6 +124,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 	int			fgc_flags;
 	char		aggkind = 0;
 	ParseCallbackState pcbstate;
+	bool		could_be_rpr_nav = false;
 
 	/*
 	 * If there's an aggregate filter, transform it using transformWhereClause
@@ -220,22 +221,19 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 		Assert(first_arg != NULL);
 	}
 
-	/*
-	 * Inside an RPR DEFINE clause, an unqualified call to one of the row
-	 * pattern navigation names PREV/NEXT/FIRST/LAST always denotes the
-	 * navigation operation, regardless of what functions exist -- the names
-	 * are recognized here, before any catalog lookup, with no fallback to
-	 * function resolution.  A schema-qualified call is the explicit way to
-	 * reach an ordinary function of one of these names.
-	 */
-	if (!is_column && !proc_call &&
-		pstate->p_expr_kind == EXPR_KIND_RPR_DEFINE &&
-		list_length(funcname) == 1)
+	if (pstate->p_expr_kind == EXPR_KIND_RPR_DEFINE)
 	{
-		retval = ParseRPRNavCall(pstate, funcname, fargs, argnames, fn,
-								 location);
-		if (retval)
-			return retval;
+		if (!is_column && !proc_call &&
+			list_length(funcname) == 1)
+		{
+			const char *name = strVal(linitial(funcname));
+
+			if (strcmp(name, "prev") == 0 ||
+				(strcmp(name, "next") == 0) ||
+				(strcmp(name, "first") == 0) ||
+				(strcmp(name, "last") == 0))
+				could_be_rpr_nav = true;
+		}
 	}
 
 	/*
@@ -284,27 +282,6 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 	 * ordinary function (as happens outside a DEFINE clause) is still open
 	 * for discussion.
 	 */
-	if (is_column && pstate->p_expr_kind == EXPR_KIND_RPR_DEFINE)
-	{
-		const char *navname = NULL;
-
-		if (strcmp(strVal(llast(funcname)), "prev") == 0)
-			navname = "PREV";
-		else if (strcmp(strVal(llast(funcname)), "next") == 0)
-			navname = "NEXT";
-		else if (strcmp(strVal(llast(funcname)), "first") == 0)
-			navname = "FIRST";
-		else if (strcmp(strVal(llast(funcname)), "last") == 0)
-			navname = "LAST";
-
-		if (navname != NULL)
-			ereport(ERROR,
-					(errcode(ERRCODE_SYNTAX_ERROR),
-					 errmsg("cannot use row pattern navigation function %s in attribute notation",
-							navname),
-					 errhint("To call an ordinary function of this name, schema-qualify it."),
-					 parser_errposition(pstate, location)));
-	}
 
 	/*
 	 * func_get_detail looks up the function in the catalogs, does
@@ -321,17 +298,26 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 	 * with default arguments.
 	 */
 
-	setup_parser_errposition_callback(&pcbstate, pstate, location);
+	if (!could_be_rpr_nav)
+	{
+		setup_parser_errposition_callback(&pcbstate, pstate, location);
 
-	fdresult = func_get_detail(funcname, fargs, argnames, nargs,
-							   actual_arg_types,
-							   !func_variadic, true, proc_call,
-							   &fgc_flags,
-							   &funcid, &rettype, &retset,
-							   &nvargs, &vatype,
-							   &declared_arg_types, &argdefaults);
+		fdresult = func_get_detail(funcname, fargs, argnames, nargs,
+								   actual_arg_types,
+								   !func_variadic, true, proc_call,
+								   &fgc_flags,
+								   &funcid, &rettype, &retset,
+								   &nvargs, &vatype,
+								   &declared_arg_types, &argdefaults);
 
-	cancel_parser_errposition_callback(&pcbstate);
+		cancel_parser_errposition_callback(&pcbstate);
+	}
+	else
+	{
+		Assert(!proc_call);
+
+		fdresult = FUNCDETAIL_NORMAL;
+	}
 
 	/*
 	 * Check for various wrong-kind-of-routine cases.
@@ -706,6 +692,22 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
 					 parser_errposition(pstate, location)));
 	}
 
+	if (could_be_rpr_nav)
+	{
+		/*
+		 * Inside an RPR DEFINE clause, an unqualified call to one of the row
+		 * pattern navigation names PREV/NEXT/FIRST/LAST always denotes the
+		 * navigation operation, regardless of what functions exist -- the
+		 * names are recognized here, before any catalog lookup, with no
+		 * fallback to function resolution.  A schema-qualified call is the
+		 * explicit way to reach an ordinary function of one of these names.
+		 */
+		return ParseRPRNavCall(pstate, funcname, fargs,
+							   argnames,
+							   fn,
+							   location);
+	}
+
 	/*
 	 * If there are default arguments, we have to include their types in
 	 * actual_arg_types for the purpose of checking generic type consistency.
@@ -2098,8 +2100,6 @@ FuncNameAsType(List *funcname)
  * to unqualified names) is the documented way to reach such a function
  * instead.
  *
- * Returns the RPRNavExpr, or NULL if funcname is not a navigation name, in
- * which case the caller resolves it as an ordinary function.
  */
 static Node *
 ParseRPRNavCall(ParseState *pstate, List *funcname, List *fargs,
@@ -2134,7 +2134,10 @@ ParseRPRNavCall(ParseState *pstate, List *funcname, List *fargs,
 		navname = "LAST";
 	}
 	else
+	{
+		pg_unreachable();
 		return NULL;
+	}
 
 	/*
 	 * Once the name matches we never fall back to function resolution, so any
@@ -2143,77 +2146,43 @@ ParseRPRNavCall(ParseState *pstate, List *funcname, List *fargs,
 	 * existing messages so the user-visible text and translations are
 	 * unchanged.
 	 */
-	if (fn->agg_star)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("%s(*) specified, but %s is not an aggregate function",
-						NameListToString(funcname),
-						NameListToString(funcname)),
-				 parser_errposition(pstate, location)));
-	if (fn->agg_distinct)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("DISTINCT specified, but %s is not an aggregate function",
-						NameListToString(funcname)),
-				 parser_errposition(pstate, location)));
-	if (fn->agg_within_group)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("WITHIN GROUP specified, but %s is not an aggregate function",
-						NameListToString(funcname)),
-				 parser_errposition(pstate, location)));
-	if (fn->agg_order != NIL)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("ORDER BY specified, but %s is not an aggregate function",
-						NameListToString(funcname)),
-				 parser_errposition(pstate, location)));
-	if (fn->agg_filter)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("FILTER specified, but %s is not an aggregate function",
-						NameListToString(funcname)),
-				 parser_errposition(pstate, location)));
-	if (fn->over)
-		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("OVER specified, but %s is not a window function nor an aggregate function",
-						NameListToString(funcname)),
-				 parser_errposition(pstate, location)));
 	if (fn->func_variadic)
 		ereport(ERROR,
-				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("cannot use VARIADIC with row pattern navigation function %s",
-						navname),
-				 parser_errposition(pstate, location)));
+				errcode(ERRCODE_SYNTAX_ERROR),
+				errmsg("cannot use VARIADIC with row pattern navigation function %s",
+					   navname),
+				parser_errposition(pstate, location));
+
 	if (argnames != NIL)
 		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("row pattern navigation operations cannot use named arguments"),
-				 parser_errposition(pstate, location)));
+				errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				errmsg("row pattern navigation operations cannot use named arguments"),
+				parser_errposition(pstate, location));
+
 	if (fn->ignore_nulls != NO_NULLTREATMENT)
 		ereport(ERROR,
-				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("row pattern navigation operations do not accept RESPECT/IGNORE NULLS"),
-				 parser_errposition(pstate, location)));
+				errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				errmsg("row pattern navigation operations do not accept RESPECT/IGNORE NULLS"),
+				parser_errposition(pstate, location));
 
 	/* arity: a value expression and an optional offset */
 	if (nargs == 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("too few arguments for row pattern navigation function %s",
-						navname),
-				 errdetail("%s takes a value expression and an optional offset argument.",
-						   navname),
-				 parser_errposition(pstate, location)));
+				errcode(ERRCODE_SYNTAX_ERROR),
+				errmsg("too few arguments for row pattern navigation function %s",
+					   navname),
+				errdetail("%s takes a value expression and an optional offset argument.",
+						  navname),
+				parser_errposition(pstate, location));
+
 	if (nargs > 2)
 		ereport(ERROR,
-				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("too many arguments for row pattern navigation function %s",
-						navname),
-				 errdetail("%s takes a value expression and an optional offset argument.",
-						   navname),
-				 parser_errposition(pstate, location)));
+				errcode(ERRCODE_SYNTAX_ERROR),
+				errmsg("too many arguments for row pattern navigation function %s",
+					   navname),
+				errdetail("%s takes a value expression and an optional offset argument.",
+						  navname),
+				parser_errposition(pstate, location));
 
 	/*
 	 * Resolve a still-unknown first argument to text, the same way the
@@ -2224,39 +2193,41 @@ ParseRPRNavCall(ParseState *pstate, List *funcname, List *fargs,
 	 */
 	arg = linitial(fargs);
 	if (exprType(arg) == UNKNOWNOID)
-		arg = coerce_to_common_type(pstate, arg, TEXTOID, navname);
+		arg = coerce_to_target_type(pstate, arg, UNKNOWNOID,
+									TEXTOID, -1,
+									COERCION_IMPLICIT,
+									COERCE_IMPLICIT_CAST, -1);
 
 	navexpr = makeNode(RPRNavExpr);
 	navexpr->kind = kind;
 	navexpr->arg = (Expr *) arg;
+	navexpr->offset_arg = NULL;
 
-	/* an explicit offset is coerced to int8, which the executor reads */
+	/* compound_offset_arg is populated in define_walker */
+	navexpr->compound_offset_arg = NULL;
+
+	/* The offset argument must be coercible to int8 */
 	if (nargs == 2)
 	{
 		Node	   *offset = lsecond(fargs);
-		Oid			offtype = exprType(offset);
+		Node	   *newoffset;
 
-		if (offtype != INT8OID)
-		{
-			Node	   *newoffset;
+		newoffset = coerce_to_target_type(pstate, offset, exprType(offset),
+										  INT8OID, -1,
+										  COERCION_IMPLICIT,
+										  COERCE_IMPLICIT_CAST, -1);
 
-			newoffset = coerce_to_target_type(pstate, offset, offtype,
-											  INT8OID, -1, COERCION_IMPLICIT,
-											  COERCE_IMPLICIT_CAST, -1);
-			if (newoffset == NULL)
-				ereport(ERROR,
-						(errcode(ERRCODE_DATATYPE_MISMATCH),
-						 errmsg("offset argument of %s must be type %s, not type %s",
-								navname, "bigint", format_type_be(offtype)),
-						 parser_errposition(pstate, exprLocation(offset))));
-			offset = newoffset;
-		}
-		navexpr->offset_arg = (Expr *) offset;
+		if (newoffset == NULL)
+			ereport(ERROR,
+					errcode(ERRCODE_DATATYPE_MISMATCH),
+					errmsg("%s offset argument of type %s cannot be coerced to the expected %s",
+						   navname, format_type_be(exprType(offset)), format_type_be(INT8OID)),
+					errhint("You will need to rewrite or cast the expression."),
+					parser_errposition(pstate, exprLocation(offset)));
+
+		navexpr->offset_arg = (Expr *) newoffset;
 	}
-	else
-		navexpr->offset_arg = NULL;
 
-	/* compound_offset_arg stays NULL; define_walker flattening fills it in */
 	navexpr->resulttype = exprType(arg);
 	/* resultcollid will be set by parse_collate.c */
 	navexpr->location = location;
diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out
index ee74853a9f..5fbec93bab 100644
--- a/src/test/regress/expected/rpr_base.out
+++ b/src/test/regress/expected/rpr_base.out
@@ -1898,6 +1898,19 @@ SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
   5 | 150 |   0 |        
 (5 rows)
 
+-- (val).prev is interpreted as a call to the (not schema-qualified) function
+-- prev with val as its argument
+SELECT count(*) OVER w
+FROM nt
+WINDOW w AS (
+    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+    PATTERN (START UP+)
+    DEFINE START AS TRUE, UP AS (val).prev IS NOT NULL);
+ERROR:  volatile functions are not allowed in DEFINE clause
+LINE 6:     DEFINE START AS TRUE, UP AS (val).prev IS NOT NULL);
+                                         ^
+-- rpr_navns.prev(val) is interpreted as a call to the schema-qualified function
+-- rpr_navns.prev(), passing the val ((column of table nt) as its argument.
 SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
   FROM nt
   WINDOW w AS (PARTITION BY g ORDER BY id
@@ -2129,10 +2142,9 @@ SELECT count(*) OVER w FROM ct
   WINDOW w AS (ORDER BY id
     ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING INITIAL
     PATTERN (A+) DEFINE A AS (p).prev > 0);
-ERROR:  cannot use row pattern navigation function PREV in attribute notation
+ERROR:  column "prev" not found in data type rpr_navns_pair
 LINE 4:     PATTERN (A+) DEFINE A AS (p).prev > 0);
                                       ^
-HINT:  To call an ordinary function of this name, schema-qualify it.
 -- Navigation offset must not contain a navigation operation
 SELECT id, val
   FROM nt
diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql
index 1d0cccf719..3be6de38ca 100644
--- a/src/test/regress/sql/rpr_base.sql
+++ b/src/test/regress/sql/rpr_base.sql
@@ -1352,6 +1352,18 @@ SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
     PATTERN (START UP+)
     DEFINE START AS TRUE, UP AS val > PREV(val))
   ORDER BY id;
+
+-- (val).prev is interpreted as a call to the (not schema-qualified) function
+-- prev with val as its argument
+SELECT count(*) OVER w
+FROM nt
+WINDOW w AS (
+    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+    PATTERN (START UP+)
+    DEFINE START AS TRUE, UP AS (val).prev IS NOT NULL);
+
+-- rpr_navns.prev(val) is interpreted as a call to the schema-qualified function
+-- rpr_navns.prev(), passing the val ((column of table nt) as its argument.
 SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
   FROM nt
   WINDOW w AS (PARTITION BY g ORDER BY id
@@ -1360,6 +1372,7 @@ SELECT id, val, count(*) OVER w AS cnt, last_value(id) OVER w AS last_id
     DEFINE A AS rpr_navns.prev(val) = -999)
   ORDER BY id;
 DROP FUNCTION prev(integer);
+
 -- IMMUTABLE: unqualified is nav; qualified is the escape hatch and succeeds
 CREATE FUNCTION prev(integer) RETURNS integer AS 'SELECT -999'
   LANGUAGE sql IMMUTABLE;
-- 
2.34.1