On 26/10/2026 01:20, Chao Li wrote: > I think your suggestion expresses “only these two option names are allowed” > in one place, which is good. However, I am also concerned that if a future > option needs a similar check, following this pattern would require another > loop. > > When I wrote the code, I followed the “_given” pattern used in many places, > such as publicationcmds.c and foreigncmds.c, which records whether an option > is specified during parsing. > > So, I’d leave the v1 implementation as it is and wait for more opinions.
Although I think that a future option would need the same care in both approaches, I agree that following a known pattern is preferable. +1 >> All error cases are handled in the same path, so theoretically only a >> few options (representative ones) would suffice. But I realise that if >> the code ever changes, we might end up with untested code. I'd be >> inclined to leave them as-is, but I thought I should bring it up. If >> it's supposed to cover all possible options, you could consider adding >> VACUUM (ONLY_DATABASE_STATS, VERBOSE false). > Added the test case in v3. Thanks! Best, Jim
