Re: Improve tab completion for various SET/RESET forms

Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> writes:

> On Wed, Aug 13, 2025 at 5:52 PM Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> wrote:
>> > > I also noticed that ALTER SYSTEM RESET tab completes with all variables,
>> > > not just ones that have been set with ALTER SYSTEM. Getting this right
>> > > is a bit more complicated, since the only way to distinguish them is
>> > > pg_settings.sourcefile = '$PGDATA/postgresql.auto.conf', but Windows
>> > > uses \ for the directory separator, so we'd have to use a regex. But
>> > > there's no function for escaping a string to use as a literal match in a
>> > > regex (like Perl's quotemeta()), so we have to use LIKE instead,
>> > > manually escaping %, _ and \, and accepting any character as the
>> > > directory separator. If people think this over-complicated, we could
>> > > just check sourcefile ~ '[\\/]postgresql\.auto\.conf$', and accept false
>> > > positives if somone has used an include directive with a file with the
>> > > same name in a different directory.
>> > >
>> > > Another complication is that both current_setting('data_directory') and
>> > > pg_settings.sourcefile are only available to superusers, so I added
>> > > another version for non-superusers that completes variables they've been
>> > > granted ALTER SYSTEM on, by querying pg_parameter_acl.
[...]
> While reviewing these patches, I noticed that tab completion for
> parameter names is missing for SET LOCAL and SET SESSION. Would it be
> possible to fix this at the same time?

SESSION is the default for SET <parameter>, and SET SESSION already
completes with AUTHORIZATION and CHARACTERISTICS AS TRANSACTION. I
don't think it would be useful to drown those out with configuration
parmeters as well.

Adding tab completion for variables and keywords after SET LOCAL would
be easy, but adding support for value completion (e.g. for enums and
timezones) would be harder, since the gen_tabcomplete.pl script doesn't
support condtions on the form ((Matches(...) || Matches(...)) && ...),
which would be necessary in order to add a TailMatches("SET",
"SESSION|LOCAL", MatchAny, "TO|=") alternative to this:

else if (TailMatches("SET", MatchAny, "TO|=") &&
!TailMatches("UPDATE", MatchAny, "SET", MatchAny, "TO|="))

So I agree with Kirill that that's should be a separate patch. This
series is already much longer than I intended to make it when I started.

> Regarding the pg_parameter_acl catalog, which was introduced in
> PostgreSQL 15, are there any backward-compatibility concerns? I don't
> think that tab completion for ALTER SYSTEM is a high priority, as the
> implementation would likely be overly complex.

I've made the query version-specific now, so it only gets run on 15 and
newer. I'm not sure whaty you mean by "would likely be overly complex".
You can see the pg_parameter_acl query for non-superusers in patch 5,
and it isn't particularly complex.

However, the sourcefile condition for superusers in patch 4 is quite
hairy. That could be made much simpler if we accept false positives for
prameters that are set in files named 'postgresql.auto.conf' included
from directories besides the data directory. That wuold let us change

" WHERE sourcefile LIKE pg_catalog.regexp_replace( " \
" pg_catalog.current_setting('data_directory'), " \
" '[_%%\\\\]', '\\\\\\&') || '_postgresql.auto.conf' " \

to

" WHERE sourcefile ~ '[\\\\/]postgresql\\.auto\\.conf$' " \

On further thought condition is so much nicer that I think it's worth
the extremely unlikey false positives, so I've changed it to that in v6.

Please find attached an updated patch series.

- ilmari