Re: Improve tab completion for various SET/RESET forms

On Wed, Aug 13, 2025 at 3:56 PM Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> wrote:
>
> On Sat, Aug 9, 2025 at 7:55 AM Dagfinn Ilmari Mannsåker
> <ilmari(at)ilmari(dot)org> wrote:
> >
> > Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> writes:
> >
> > > On Fri, Aug 1, 2025 at 2:16 AM Dagfinn Ilmari Mannsåker
> > > <ilmari(at)ilmari(dot)org> wrote:
> > >>
> > >> Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)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(at)ilmari(dot)org> wrote:
> >
> > Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org> writes:
> >
> > > Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> writes:
> > >
> > >> On Fri, Aug 1, 2025 at 2:16 AM Dagfinn Ilmari Mannsåker
> > >> <ilmari(at)ilmari(dot)org> wrote:
> > >>>
> > >>> Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)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
> ----

Oh, sorry. I can apply them with git am.

--
Best regards,
Shinya Kato
NTT OSS Center