Feedback-ID: ie3de48e3:Fastmail
Date: Mon, 15 Sep 2025 22:24:05 +0200
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Xuneng Zhou <xunengzhou@gmail.com>, Tomas Vondra <tomas@vondra.me>,
	Yura Sokolov <y.sokolov@postgrespro.ru>, pgsql-hackers@lists.postgresql.org
Subject: Re: Implement waiting for wal lsn replay: reloaded
Message-ID: <202509152003.m3wlpwa7i657@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CAPpHfdsz0tfBJuY1QBsGtO2AXo-JDvTV=rc7sjWru1+2CjfAQA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202509152003.m3wlpwa7i657%40alvherre.pgsql>
Precedence: bulk

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/