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 ([email protected]) wrote the INITIAL_P patch; SungJun
Jang ([email protected]) did the Oracle and Trino cross-validation.
Different surnames -- two different people.
