Re: Row pattern recognition

Chao Li <li.evan.chao@gmail.com>

From: Chao Li <li.evan.chao@gmail.com>
To: Vik Fearing <vik@postgresfriends.org>, Tatsuo Ishii <ishii@postgresql.org>
Cc: david.g.johnston@gmail.com, jacob.champion@enterprisedb.com, er@xs4all.nl, peter@eisentraut.org, pgsql-hackers@postgresql.org
Date: 2025-11-19T04:14:03Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Add temporal FOREIGN KEY contraints

  2. Remove obsolete executor cleanup code


> On Nov 18, 2025, at 19:19, Vik Fearing <vik@postgresfriends.org> wrote:
> 
> 
> Because of position. Without making DEFINE a reserved keyword, how do you know that it isn't another variable in the PATTERN clause?
> 

Ah, thanks for the clarification. Now I got it.

I’m continue to review 0002.

5 - 0002 - parse_clause.c
```
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("FRAME must start at current row when row patttern recognition is used")));
```

Nit typo: patttern -> pattern

6 - 0002 - parse_clause.c
```
+	/* DEFINE variable name initials */
+	static char *defineVariableInitials = "abcdefghijklmnopqrstuvwxyz”;
```

This string can also be const, so “static const char *”

7 - 0002 - parse_clause.c
```
+	/*
+	 * Create list of row pattern DEFINE variable name's initial. We assign
+	 * [a-z] to them (up to 26 variable names are allowed).
+	 */
+	restargets = NIL;
+	i = 0;
+	initialLen = strlen(defineVariableInitials);
+
+	foreach(lc, windef->rpCommonSyntax->rpDefs)
+	{
+		char		initial[2];
+
+		restarget = (ResTarget *) lfirst(lc);
+		name = restarget->name;
```

Looks like “name” is not used inside “foreach”.

8 - 0002 - parse_clause.c
```
+	/*
+	 * Create list of row pattern DEFINE variable name's initial. We assign
+	 * [a-z] to them (up to 26 variable names are allowed).
+	 */
+	restargets = NIL;
+	i = 0;
+	initialLen = strlen(defineVariableInitials);
+
+	foreach(lc, windef->rpCommonSyntax->rpDefs)
+	{
+		char		initial[2];
```

I guess this “foreach” for build initial list can be combined into the previous foreach loop of checking duplication. If an def has no dup, then assign an initial to it.

9 - 0002 - parse_clause.c
```
+static void
+transformPatternClause(ParseState *pstate, WindowClause *wc,
+					   WindowDef *windef)
+{
+	ListCell   *lc;
+
+	/*
+	 * Row Pattern Common Syntax clause exists?
+	 */
+	if (windef->rpCommonSyntax == NULL)
+		return;
```

Checking  (windef->rpCommonSyntax == NULL) seems a duplicate, because transformPatternClause() is only called by transformRPR(), and transformRPR() has done the check.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/