Thread
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix tab completion for ALTER ROLE|USER ... RESET
- 88914332eaed 18.0 landed
- ca09ef3a6aa6 19 (unreleased) landed
-
Schema-qualify unnest() in ALTER DATABASE ... RESET
- 72c437f6e464 18.0 landed
- dbf5a83d4650 19 (unreleased) landed
-
Improve tab completion for various SET/RESET forms
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-06-09T20:27:07Z
Hi hackers, I noticed that psql tab-completes every possible session-settable variable after RESET, not just the ones that have actually been set in the current session. However, as I was fixing that I noticed several other deficiencies around other forms of SET/RESET. So, here's the resulting yak stack. Feel free to squash them as desired when committing, I just find it easier to explain each thing as a separate commit/patch. 1. Fix tab completion for ALTER ROLE/USER ... RESET It was trying to get the variables set for the current user, but the query only worked on the intial tab after RESET, not when you started typing a prefix to pick the one you wanted. Fix in the same way as ALTER DATABASE ... RESET does it. 2. Add tab completion for ALTER TABLE ... ALTER COLUMN ... RESET Complete with "(" and then the same as after ALTER COLUMN ... SET (. There are only two possible attribute options, so no need to filter down to the ones that have actually been set. 3. Add tab completion for ALTER FOREIGN TABLE ... SET Setting the schema is the only supported thing. 4. Remove guard against generic SET/RESET completion after ALTER TABLE ... RESET This is already handled specifically earlier in the code. Only UPDATE is later and needs guarding against. 5. The actual patch I set out to write: only complete variables that have actually been set in the current session after RESET. - ilmari -- part-time yak stylist -
Re: Improve tab completion for various SET/RESET forms
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-06-09T20:33:46Z
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes: > Hi hackers, > > I noticed that psql tab-completes every possible session-settable > variable after RESET, not just the ones that have actually been set in > the current session. However, as I was fixing that I noticed several > other deficiencies around other forms of SET/RESET. So, here's the > resulting yak stack. Added to the next commitfest: https://commitfest.postgresql.org/patch/5810/ - ilmari
-
Re: Improve tab completion for various SET/RESET forms
Peter Eisentraut <peter@eisentraut.org> — 2025-06-11T08:34:44Z
On 09.06.25 22:27, Dagfinn Ilmari Mannsåker wrote: > I noticed that psql tab-completes every possible session-settable > variable after RESET, not just the ones that have actually been set in > the current session. However, as I was fixing that I noticed several > other deficiencies around other forms of SET/RESET. So, here's the > resulting yak stack. These technical changes seem fine, but I haven't looked in detail. But I want to say that I don't like this proposed behavior change at all. I don't think tab completion should filter out by things that are probably not interesting or useful depending on session state. That could be very confusing, especially since there is no surface to explain this anywhere. The obvious extreme case is that RESET in a fresh session wouldn't offer any completions at all.
-
Re: Improve tab completion for various SET/RESET forms
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-06-11T10:06:42Z
Peter Eisentraut <peter@eisentraut.org> writes: > On 09.06.25 22:27, Dagfinn Ilmari Mannsåker wrote: >> I noticed that psql tab-completes every possible session-settable >> variable after RESET, not just the ones that have actually been set in >> the current session. However, as I was fixing that I noticed several >> other deficiencies around other forms of SET/RESET. So, here's the >> resulting yak stack. > > These technical changes seem fine, but I haven't looked in detail. But > I want to say that I don't like this proposed behavior change at all. This is not the first case of behaviour like this: there's precedence for it for ALTER DATABASE ... RESET and ALTER ROLE ... RESET (but that was buggy and is fixed by the first patch in the series): they only complete variables that are actually set on the relevant entity. See the thread at https://postgr.es/m/CAEP4nAzqiT6VbVC5r3nq5byLTnPzjniVGzEMpYcnAHQyNzEuaw%40mail.gmail.com Why should the session be any different? > don't think tab completion should filter out by things that are probably > not interesting or useful depending on session state. That could be > very confusing, especially since there is no surface to explain this > anywhere. The obvious extreme case is that RESET in a fresh session > wouldn't offer any completions at all. That's not the case, as mentioned in the commit message for the final patch: it also completes with the keywords ALL, ROLE and SESSION (which will subsequently be completed with AUTHORIZATION). Also, it's a handy way of seeing which variables have been set in the current session, withouth having to query pg_settings manually. - ilmari
-
Re: Improve tab completion for various SET/RESET forms
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-06-12T13:12:52Z
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes: > Peter Eisentraut <peter@eisentraut.org> writes: > >> On 09.06.25 22:27, Dagfinn Ilmari Mannsåker wrote: >>> I noticed that psql tab-completes every possible session-settable >>> variable after RESET, not just the ones that have actually been set in >>> the current session. However, as I was fixing that I noticed several >>> other deficiencies around other forms of SET/RESET. So, here's the >>> resulting yak stack. >> >> These technical changes seem fine, but I haven't looked in detail. But >> I want to say that I don't like this proposed behavior change at all. > > This is not the first case of behaviour like this: there's precedence > for it for ALTER DATABASE ... RESET and ALTER ROLE ... RESET (but that > was buggy and is fixed by the first patch in the series): they only > complete variables that are actually set on the relevant entity. See the > thread at > https://postgr.es/m/CAEP4nAzqiT6VbVC5r3nq5byLTnPzjniVGzEMpYcnAHQyNzEuaw%40mail.gmail.com 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. - ilmari
-
Re: Improve tab completion for various SET/RESET forms
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-07-31T17:16:17Z
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. - ilmari
-
Re: Improve tab completion for various SET/RESET forms
Shinya Kato <shinya11.kato@gmail.com> — 2025-08-08T03:03:30Z
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. +#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. -- Best regards, Shinya Kato NTT OSS Center -
Re: Improve tab completion for various SET/RESET forms
Kirill Reshke <reshkekirill@gmail.com> — 2025-08-08T10:44:57Z
Hi! On Fri, 8 Aug 2025 at 08:04, Shinya Kato <shinya11.kato@gmail.com> wrote: > In addition to SET SCHEMA, I think you should add tab completion for > SET WITHOUT OIDS. This is unsupported since v12 [1] [1] https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-DESC-SET-WITHOUT-OIDS -- Best regards, Kirill Reshke
-
Re: Improve tab completion for various SET/RESET forms
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-08-08T22:55:33Z
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. > +#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. - ilmari -
Re: Improve tab completion for various SET/RESET forms
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-08-11T11:40:34Z
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. - ilmari -
Re: Improve tab completion for various SET/RESET forms
Shinya Kato <shinya11.kato@gmail.com> — 2025-08-13T06:56:45Z
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 -
Re: Improve tab completion for various SET/RESET forms
Shinya Kato <shinya11.kato@gmail.com> — 2025-08-13T08:52:45Z
On Wed, Aug 13, 2025 at 3:56 PM Shinya Kato <shinya11.kato@gmail.com> wrote: > > 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 > ---- Oh, sorry. I can apply them with git am. -- Best regards, Shinya Kato NTT OSS Center -
Re: Improve tab completion for various SET/RESET forms
Shinya Kato <shinya11.kato@gmail.com> — 2025-08-13T09:53:59Z
On Wed, Aug 13, 2025 at 5:52 PM Shinya Kato <shinya11.kato@gmail.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. > > > > 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. 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? 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. -- Best regards, Shinya Kato NTT OSS Center -
Re: Improve tab completion for various SET/RESET forms
Kirill Reshke <reshkekirill@gmail.com> — 2025-08-13T14:04:50Z
On Wed, 13 Aug 2025 at 14:54, Shinya Kato <shinya11.kato@gmail.com> wrote: > > 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? Indeed, SET SESSION <tab> does not suggest any parameter. That should be a separate patch though. Also, SET <tab> suggests SESSION but not LOCAL, which also can be improved -- Best regards, Kirill Reshke
-
Re: Improve tab completion for various SET/RESET forms
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-11-11T20:27:20Z
Shinya Kato <shinya11.kato@gmail.com> writes: > On Wed, Aug 13, 2025 at 5:52 PM Shinya Kato <shinya11.kato@gmail.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