On 2025-Sep-15, Alexander Korotkov wrote:
> > It's LGTM. The same pattern is observed in VACUUM, EXPLAIN, and CREATE
> > PUBLICATION - all use minimal grammar rules that produce generic
> > option lists, with the actual interpretation done in their respective
> > implementation files. The moderate complexity in wait.c seems
> > acceptable.
Actually I find the code in ExecWaitStmt pretty unusual. We tend to use
lists of DefElem (a name optionally followed by a value) instead of
individual scattered elements that must later be matched up. Why not
use utility_option_list instead and then loop on the list of DefElems?
It'd be a lot simpler.
Also, we've found that failing to surround the options by parens leads
to pain down the road, so maybe add that. Given that the LSN seems to
be mandatory, maybe make it something like
WAIT FOR LSN 'xy/zzy' [ WITH ( utility_option_list ) ]
This requires that you make LSN a keyword, albeit unreserved. Or you
could make it
WAIT FOR Ident [the rest]
and then ensure in C that the identifier matches the word LSN, such as
we do for "permissive" and "restrictive" in
RowSecurityDefaultPermissive.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
