From: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
---|---|
To: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
Cc: | Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, jian he <jian(dot)universality(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Implement waiting for wal lsn replay: reloaded |
Date: | 2025-10-15 08:51:42 |
Message-ID: | 202510150831.g75i32rzddor@alvherre.pgsql |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I didn't review the patch other than look at the grammar, but I disagree
with using opt_with in it. I think WITH should be a mandatory word, or
just not be there at all. The current formulation lets you do one of:
1. WAIT FOR LSN '123/456' WITH (opt = val);
2. WAIT FOR LSN '123/456' (opt = val);
3. WAIT FOR LSN '123/456';
and I don't see why you need two ways to specify an option list.
So one option is to remove opt_wait_with_clause and just use
opt_utility_option_list, which would remove the WITH keyword from there
(ie. only keep 2 and 3 from the above list). But I think that's worse:
just look at the REPACK grammar[1], where we have to have additional
productions for the optional parenthesized option list.
So why not do just
+opt_wait_with_clause:
+ WITH '(' utility_option_list ')' { $$ = $3; }
+ | /*EMPTY*/ { $$ = NIL; }
+ ;
which keeps options 1 and 3 of the list above.
Note: you don't need to worry about WITH_LA, because that's only going
to show up when the user writes WITH TIME or WITH ORDINALITY (see
parser.c), and that's a syntax error anyway.
[1] https://postgr.es/m/202510101352.vvp4p3p2dblu@alvherre.pgsql
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La virtud es el justo medio entre dos defectos" (Aristóteles)
From | Date | Subject | |
---|---|---|---|
Next Message | Алена Васильева | 2025-10-15 08:54:30 | [PATCH] Handle out-of-range timestamps in timestamptz_to_str() |
Previous Message | Алена Васильева | 2025-10-15 08:50:21 | [PATCH] Handle out-of-range timestamps in timestamptz_to_str() |