> On Nov 18, 2025, at 19:19, Vik Fearing <[email protected]> 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/






Reply via email to