Re: [PATCH] Fix psql tab completion for REPACK boolean options

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

In response to

Browse pgsql-hackers by date

  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