Re: Row pattern recognition - Mailing list pgsql-hackers

From Chao Li
Subject Re: Row pattern recognition
Date
Msg-id 66415FA2-B539-40C1-A58D-B28E373BDB26@gmail.com
Whole thread Raw
In response to Re: Row pattern recognition  (Vik Fearing <vik@postgresfriends.org>)
List pgsql-hackers

> 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
PATTERNclause? 
>

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.
Ifan 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/







pgsql-hackers by date:

Previous
From: Xuneng Zhou
Date:
Subject: Re: Improve read_local_xlog_page_guts by replacing polling with latch-based waiting
Next
From: Andrey Borodin
Date:
Subject: Re: nbtree VACUUM's REDO routine doesn't clear page's VACUUM cycle ID