[PATCH] Fix psql tab completion for REPACK boolean options

On Tue, May 12, 2026 at 4:45 AM Baji Shaik <baji.pgdev@gmail.com> wrote:

Hi,

I noticed that psql's tab completion for REPACK's parenthesized
boolean options (ANALYZE, CONCURRENTLY, VERBOSE) does not offer
ON/OFF after typing one of these keywords.
For example:
REPACK (VERBOSE <Tab> -- nothing offered
VACUUM (VERBOSE <Tab> -- correctly offers ON/OFF

The issue is in tab-complete.in.c. The code uses:

TailMatches("ANALYZE", "CONCURRENTLY", "VERBOSE")

With comma-separated arguments, TailMatches matches a sequence of
consecutive words. So this only matches when all three words appear
in order, which is not a valid REPACK syntax.

The fix is to use pipe-separated alternatives (matching any one of
the keywords), consistent with how VACUUM handles the same pattern:

TailMatches("ANALYZE|CONCURRENTLY|VERBOSE")

Patch attached.

Thanks for the patch! It looks good to me.
Unless there are objections, I will commit it.

BTW, I found that the boolean options for CLUSTER and REINDEX have
the same issue, i.e., tab completion does not suggest boolean values
such as ON and OFF. However, this has existed for a while, and adding
those completions seems more like an improvement than a bug fix,
so it should probably be handled separately during v20 development.

Regards,

--
Fujii Masao

On 2026-May-13, Fujii Masao wrote:

On Tue, May 12, 2026 at 4:45 AM Baji Shaik <baji.pgdev@gmail.com> wrote:

The fix is to use pipe-separated alternatives (matching any one of
the keywords), consistent with how VACUUM handles the same pattern:

TailMatches("ANALYZE|CONCURRENTLY|VERBOSE")

Patch attached.

Thanks for the patch! It looks good to me.
Unless there are objections, I will commit it.

Yeah, looks good to me, thanks.

BTW, I found that the boolean options for CLUSTER and REINDEX have
the same issue, i.e., tab completion does not suggest boolean values
such as ON and OFF. However, this has existed for a while, and adding
those completions seems more like an improvement than a bug fix,
so it should probably be handled separately during v20 development.

Fine with me. I wouldn't have a problem saying it's a backpatchable
bugfix, but it's certainly not very high priority or criticality.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

On Wed, May 13, 2026 at 9:46 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

Thanks for the patch! It looks good to me.
Unless there are objections, I will commit it.

Yeah, looks good to me, thanks.

I've pushed the patch. Thanks!

Fine with me. I wouldn't have a problem saying it's a backpatchable
bugfix, but it's certainly not very high priority or criticality.

Agreed.

BTW, while testing REPACK boolean options, I found two other issues
in their parsing.

First, REPACK (CONCURRENTLY OFF) failed with:

ERROR: unrecognized REPACK option "concurrently"

even though REPACK (CONCURRENTLY ON) works correctly. That is,
CONCURRENTLY was treated as an unrecognized option when disabled.

Second, when the same option is specified multiple times, the last value
is not always honored. In particular, if any occurrence sets an option to ON,
the option remains enabled even when the final setting is OFF.

I think these are bugs, and the attached patch fixes them. Thoughts?

Regards,

--
Fujii Masao

On 2026-May-15, Fujii Masao wrote:

BTW, while testing REPACK boolean options, I found two other issues
in their parsing.

First, REPACK (CONCURRENTLY OFF) failed with:

ERROR: unrecognized REPACK option "concurrently"

even though REPACK (CONCURRENTLY ON) works correctly. That is,
CONCURRENTLY was treated as an unrecognized option when disabled.

Oh :-(

Second, when the same option is specified multiple times, the last value
is not always honored. In particular, if any occurrence sets an option to ON,
the option remains enabled even when the final setting is OFF.

I think these are bugs, and the attached patch fixes them. Thoughts?

I agree that these are bugs. The patch looks good to me on a cursory
look, though I didn't actually test it.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

On Fri, May 15, 2026 at 11:32 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

Second, when the same option is specified multiple times, the last value
is not always honored. In particular, if any occurrence sets an option to ON,
the option remains enabled even when the final setting is OFF.

I think these are bugs, and the attached patch fixes them. Thoughts?

I agree that these are bugs. The patch looks good to me on a cursory
look, though I didn't actually test it.

Thanks for the review! I've tested it again and then pushed it.

Regards,

--
Fujii Masao

On Fri, 15 May 2026 at 10:36, Fujii Masao <masao.fujii@gmail.com> wrote:

On Wed, May 13, 2026 at 9:46 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

Thanks for the patch! It looks good to me.
Unless there are objections, I will commit it.

Yeah, looks good to me, thanks.

I've pushed the patch. Thanks!

Fine with me. I wouldn't have a problem saying it's a backpatchable
bugfix, but it's certainly not very high priority or criticality.

Agreed.

BTW, while testing REPACK boolean options, I found two other issues
in their parsing.

First, REPACK (CONCURRENTLY OFF) failed with:

ERROR: unrecognized REPACK option "concurrently"

even though REPACK (CONCURRENTLY ON) works correctly. That is,
CONCURRENTLY was treated as an unrecognized option when disabled.

Second, when the same option is specified multiple times, the last value
is not always honored. In particular, if any occurrence sets an option to ON,
the option remains enabled even when the final setting is OFF.

I think these are bugs, and the attached patch fixes them. Thoughts?

I saw this just got committed. Is there a reason the option handling
for this, VACUUM, and EXPLAIN aren't done in one place?

Thom

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.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"This is a foot just waiting to be shot" (Andrew Dunstan)

On Mon, 18 May 2026 at 12:49, Álvaro Herrera <alvherre@kurilemu.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