| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Vik Fearing <vik(at)postgresfriends(dot)org>, Tatsuo Ishii <ishii(at)postgresql(dot)org> |
| Cc: | david(dot)g(dot)johnston(at)gmail(dot)com, jacob(dot)champion(at)enterprisedb(dot)com, er(at)xs4all(dot)nl, peter(at)eisentraut(dot)org, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Row pattern recognition |
| Date: | 2025-11-19 04:14:03 |
| Message-ID: | 66415FA2-B539-40C1-A58D-B28E373BDB26@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Nov 18, 2025, at 19:19, Vik Fearing <vik(at)postgresfriends(dot)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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2025-11-19 04:36:19 | Re: nbtree VACUUM's REDO routine doesn't clear page's VACUUM cycle ID |
| Previous Message | Xuneng Zhou | 2025-11-19 03:44:05 | Re: Improve read_local_xlog_page_guts by replacing polling with latch-based waiting |