| From: | Henson Choi <assam258(at)gmail(dot)com> |
|---|---|
| To: | Tatsuo Ishii <ishii(at)postgresql(dot)org>, shinsj4653(at)gmail(dot)com |
| Cc: | jian(dot)universality(at)gmail(dot)com, vik(at)postgresfriends(dot)org, pgsql-hackers(at)postgresql(dot)org, zsolt(dot)parragi(at)percona(dot)com, sjjang112233(at)gmail(dot)com, er(at)xs4all(dot)nl, jacob(dot)champion(at)enterprisedb(dot)com, david(dot)g(dot)johnston(at)gmail(dot)com, peter(at)eisentraut(dot)org, li(dot)evan(dot)chao(at)gmail(dot)com |
| Subject: | Re: Row pattern recognition |
| Date: | 2026-07-19 12:47:55 |
| Message-ID: | CAAAe_zB6=ab-OyKzG20Eyxwe6=4GHoiig32bWPmuXdkqnC55UQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Tatsuo, Seongjun,
> BTW, I noticed a comment in gram.y:
>
> * We need to do this for PARTITION, RANGE, ROWS, GROUPS, AFTER, INITIAL,
> * SEEK, PATTERN_P to support opt_existing_window_name (see comment
there).
>
> Even if we change INITIAL to INITIAL_P, we should not change the
> comment. Moreover, PATTERN_P (which was added by RPR patch too) in
> the comment above should have been PATTERN.
Agreed on both. For INITIAL the rename patch already behaves that
way: it changes only the token -- the %nonassoc precedence line, the
%token list and the productions -- and leaves the comment's INITIAL
untouched, so nothing there turns into INITIAL_P.
And you're right that PATTERN_P should be PATTERN, and I think it is
worth fixing. It is not from the rename -- it has been in the
raw-parser patch itself -- and it appears in two comments, both
describing SQL keywords rather than tokens:
gram.y, above the %nonassoc block:
* We need to do this for PARTITION, RANGE, ROWS, GROUPS, AFTER, INITIAL,
* SEEK, PATTERN_P to support opt_existing_window_name (see comment
there).
gram.y, above opt_existing_window_name (the "comment there"):
* If we see PARTITION, RANGE, ROWS, GROUPS, AFTER, INITIAL, SEEK or
PATTERN_P
* as the first token after the '(' of a window_specification, ...
In both, PATTERN_P should read PATTERN, exactly as INITIAL (not
INITIAL_P) already does, since these lines name SQL keywords. INITIAL
stays as it is.
If that's agreeable, I'll add the one-word fix (PATTERN_P -> PATTERN,
in both comments) to 0002, alongside Seongjun's INITIAL_P rename, and
send it with the next incremental posting.
Best regards,
Henson
PS -- to avoid mixing them up: two different people on this thread
have the same given name in Korean, though it romanizes differently.
Seongjun Shin (shinsj4653(at)gmail(dot)com) wrote the INITIAL_P patch; SungJun
Jang (sjjang112233(at)gmail(dot)com) did the Oracle and Trino cross-validation.
Different surnames -- two different people.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alexandre Felipe | 2026-07-19 13:26:04 | Re: SLOPE - Planner optimizations on monotonic expressions. |
| Previous Message | Tatsuo Ishii | 2026-07-19 12:21:08 | Re: Row pattern recognition |