| From: | Thom Brown <thom(at)linux(dot)com> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Baji Shaik <baji(dot)pgdev(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: [PATCH] Fix psql tab completion for REPACK boolean options |
| Date: | 2026-05-18 12:29:23 |
| Message-ID: | CAA-aLv57ymRoDJ+2UnHCE7WDB_FH5pW9+bFX7z5wEegudPrdRQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, 18 May 2026 at 12:49, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> On 2026-May-18, Thom Brown wrote:
>
> > I saw this just got committed. Is there a reason the option handling
> > for this, VACUUM, and EXPLAIN aren't done in one place?
>
> I don't think they take exactly the same options; but if you see room
> for improvement, by all means send a patch.
I wasn't thinking of a shared handler for the options themselves,
because, as you say, those differ per command. I meant the handling of
the DefElem list: last-duplicate-wins, rejecting unknown options,
coercion via defGetBoolean, etc. That part is the same everywhere, but
each command writes its own set of checks, and the bug fixed here is
what you get from that: a loop that enabled the option if any
occurrence was ON, instead of just overwriting.
So each command would hand its DefElem list to a common routine along
with a table of valid option names and types. The routine iterates the
list, rejects unknown names, coerces values, and applies last-wins for
repeated options. Command-specific semantic checks would stay in the
command.
I could try writing a patch, but at this point in the dev cycle, it's
probably something for later.
Thom
| From | Date | Subject | |
|---|---|---|---|
| Next Message | vignesh C | 2026-05-18 12:35:40 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Bertrand Drouvot | 2026-05-18 12:14:34 | Re: Avoid orphaned objects dependencies, take 3 |