Re: Improve tab completion for various SET/RESET forms - Mailing list pgsql-hackers

On Sat, Aug 9, 2025 at 7:55 AM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:
>
> Shinya Kato <shinya11.kato@gmail.com> writes:
>
> > On Fri, Aug 1, 2025 at 2:16 AM Dagfinn Ilmari Mannsåker
> > <ilmari@ilmari.org> wrote:
> >>
> >> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:
> >>
> >> > I just noticed that in addition to ALTER ROLE ... RESET being buggy, the
> >> > ALTER DATABASE ... RESET query doesn't schema-qualify the unnest() call.
> >> > Here's an updated patch series that fixes that too.  The first two are
> >> > bug fixes to features new in 18 and should IMO be committed before
> >> > that's released.  The rest can wait for 19.
> >>
> >> Now that Tomas has committed the two bugfixes, here's the rest of the
> >> patches rebased over that.
> >
> > Thank you for the patches.
> >
> > +   else if (Matches("ALTER", "FOREIGN", "TABLE", MatchAny, "SET"))
> > +       COMPLETE_WITH("SCHEMA");
> >
> > In addition to SET SCHEMA, I think you should add tab completion for
> > SET WITHOUT OIDS.
>
> As Kirill pointed out, support for WITH OIDS was removed in v12.  The
> SET WITHOUT OIDS syntax only remains as a no-op for backwards
> compatibility with existing SQL scripts, so there's no point in offering
> tab completion for it.

I got it, thanks!

> > +#define Query_for_list_of_session_vars \
> > +"SELECT pg_catalog.lower(name) FROM pg_catalog.pg_settings "\
> > +" WHERE context IN ('user', 'superuser') "\
> > +"   AND source = 'session' "\
> > +"   AND pg_catalog.lower(name) LIKE pg_catalog.lower('%s')"
> >
> > I think the context IN ('user', 'superuser') condition is redundant
> > here. If a parameter's source is 'session', its context must be either
> > 'user' or 'superuser'. Therefore, the source = 'session' check alone
> > should be sufficient.
>
> Good point, updated in the attached v4 patches.

Thank you.


On Mon, Aug 11, 2025 at 8:40 PM Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:
>
> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:
>
> > Shinya Kato <shinya11.kato@gmail.com> writes:
> >
> >> On Fri, Aug 1, 2025 at 2:16 AM Dagfinn Ilmari Mannsåker
> >> <ilmari@ilmari.org> wrote:
> >>>
> >>> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:
> >>>
> >>> > I just noticed that in addition to ALTER ROLE ... RESET being buggy, the
> >>> > ALTER DATABASE ... RESET query doesn't schema-qualify the unnest() call.
> >>> > Here's an updated patch series that fixes that too.  The first two are
> >>> > bug fixes to features new in 18 and should IMO be committed before
> >>> > that's released.  The rest can wait for 19.
> >>>
> >>> Now that Tomas has committed the two bugfixes, here's the rest of the
> >>> patches rebased over that.
>
> 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.

I failed to apply these patches.
----
$ git apply v5-000* -v
Checking patch src/bin/psql/tab-complete.in.c...
error: while searching for:
                                          "STATISTICS", "STORAGE",?
                /* a subset of ALTER SEQUENCE options */?
                                          "INCREMENT", "MINVALUE",
"MAXVALUE", "START", "NO", "CACHE", "CYCLE");?
        /* ALTER TABLE ALTER [COLUMN] <foo> SET ( */?
        else if (Matches("ALTER", "TABLE", MatchAny, "ALTER",
"COLUMN", MatchAny, "SET", "(") ||?
                         Matches("ALTER", "TABLE", MatchAny, "ALTER",
MatchAny, "SET", "("))?
                COMPLETE_WITH("n_distinct", "n_distinct_inherited");?
        /* ALTER TABLE ALTER [COLUMN] <foo> SET COMPRESSION */?
        else if (Matches("ALTER", "TABLE", MatchAny, "ALTER",
"COLUMN", MatchAny, "SET", "COMPRESSION") ||?

error: patch failed: src/bin/psql/tab-complete.in.c:2913
error: src/bin/psql/tab-complete.in.c: patch does not apply
----

And you should move the CF entry to the next CF. If so, CFBot reports
the above error.

--
Best regards,
Shinya Kato
NTT OSS Center