Re: [PATCH] Fix quotation logic for unreserved keywords in window specifications

Thanks for the kind review, Mr. Henson Choi and Mr. Tatsuo Ishii.

I also checked commit 4c310eca2ea. Yes, (a) breaks that.

I also think that breaking changes are not desirable for a small patch like
this one.

So, as Mr. Tatsuo pointed out, this patch (v2) would limit the quoted range.

I also added some commit text that I missed in the first patch file.

That was my mistake, so it includes the "Reviewed-by" tag and a link to the
"Discussion" thread.

I hope this commit will be helpful for the work you are currently doing.

2026년 8월 14일 (금) 오전 11:50, Tatsuo Ishii <ishii(at)postgresql(dot)org>님이 작성:

> Hi Henson,
>
> > Hi Kwangwon, Tatsuo,
> >
> > Thanks for the patch -- this is a real dump/restore hazard, and the fix
> > is in the right place (get_rule_windowspec, refname only).
> >
> >> The attached patch quotes the window name when it is a keyword
> >
> > One thing worth settling: as written this quotes the refname for any
> > keyword, but only four names actually break on reparse -- PARTITION,
> > RANGE, ROWS, GROUPS, the ones the grammar won't accept unquoted as an
> > existing window name (exactly the comment you found). Every other
> > keyword still round-trips fine unquoted, so quoting just those four is
> > enough; quoting all keywords works too, it is just broader than needed.
> >
> > For context, this is an old interaction between two commits that never
> > knew about each other: 4c310eca2ea (2007) made quote_identifier() stop
> > quoting unreserved keywords -- the point being that unreserved keywords
> > are usable bare by definition, so quoting them was just unnecessary
> > noise -- and 95b07bc7f50 (2008, window functions) later had the grammar
> > reject four of those keywords as a bare existing-window-name. So when
> > such a keyword lands in that slot as a refname, deparse prints it
> > unquoted (quote_identifier skips it as unreserved) while the grammar
> > will not take it unquoted -- and reparsing breaks in exactly that gap.
> >
> > So the choice is:
> >
> > (a) quote any keyword (what the patch does now) -- simple, but
> > over-quotes.
> > (b) quote only those four -- minimal, but it needs an explicit list in
> > ruleutils kept in sync with the grammar, since no keyword category
> > isolates them.
> >
> > Tatsuo, I would like your read as committer, because this has a
> > cross-patch consequence. The Row Pattern Recognition patch (CF 4460)
> > adds four more names -- PATTERN, AFTER, INITIAL, SEEK -- to the same
> > grammar exclusion, so it hits the same bug. Under (b), RPR would have to
> > extend the list itself (plus a round-trip test per name) and depend on
> > this patch landing first; under (a), RPR needs nothing here.
> >
> > That history is also why I lean (b): quoting every keyword on the
> > refname (option (a)) re-introduces exactly the broad quoting
> > 4c310eca2ea set out to remove, just confined to one slot. (b) quotes
> > only the four names the grammar actually rejects, which keeps faith with
> > that original intent. If we take (b), then Kwangwon, since you would be
> > building that list here anyway, would you be up for helping with the
> > matching change on the RPR side (adding those four names to the same
> > list in CF 4460)? It is the same mechanism, so doing both keeps the list
> > in one place and consistent.
> >
> > Either way, I am happy to help review it through.
>
> Thank you for the explanation. I prefer (b) too since (a) breaks
> 4c310eca2ea as you said.
>
> Regards,
> --
> Tatsuo Ishii
> SRA OSS K.K.
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
>