Hi Kuroda-San

Thanks for your review comments!

On Tue, Jul 28, 2026 at 4:46 PM Hayato Kuroda (Fujitsu)
<[email protected]> wrote:
...
> 01.
> ```
> --- a/src/bin/psql/tab-complete.in.c
> +++ b/src/bin/psql/tab-complete.in.c
> @@ -2341,9 +2341,20 @@ match_previous_words(int pattern_id,
>                 COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_schemas
>                                                                  " AND 
> nspname NOT LIKE E'pg\\\\_%%'",
>                                                                  
> "CURRENT_SCHEMA");
> -       /* ALTER PUBLICATION <name> SET ( */
> -       else if (Matches("ALTER", "PUBLICATION", MatchAny, MatchAnyN, "SET", 
> "("))
> -               COMPLETE_WITH("publish", "publish_generated_columns", 
> "publish_via_partition_root");
> +       /* ALTER PUBLICATION <name> SET ( <opt> */
> +       else if (HeadMatches("ALTER", "PUBLICATION", MatchAny, "SET", "("))
> +       {
> +               if (ends_with(prev_wd, '(') || ends_with(prev_wd, ','))
> ```
>
> According to others, the else-if statement should be slightly changed like:
>
> ```
> else if (HeadMatches("ALTER", "PUBLICATION", MatchAny, "SET", "(*") &&
>                  !HeadMatches("ALTER", "PUBLICATION", MatchAny, "SET", "(*)"))
> ```

Done as suggested.

>
> 02.
> ```
> +                       COMPLETE_WITH("publish", "publish_generated_columns 
> =",
> +                                                 
> "publish_via_partition_root");
> ```
>
> Let me confirm the policy here: the parameter "publish" won't be accepted 
> alone,
> but " =" is not completed for it. Is there a reason?
>
> 03.
> ```
>                        COMPLETE_WITH("binary", "conflict_log_destination =",
> +                                         "disable_on_error", "failover", 
> "max_retention_duration",
> +                                         "origin =", "password_required", 
> "retain_dead_tuples",
> +                                         "run_as_owner", "slot_name", 
> "streaming =",
> +                                         "synchronous_commit =", "two_phase",
> ```
>
> Same as 02. max_retention_duration and slot_name cannot be alone.
>

I initially included " =" only for the enum options. But you are
correct; it could/should have been added for some of the other options
too. Fixed now.

> 04.
>
> While testing, I recognized that the word "PUBLICATION" cannot be suggested
> if users input till below. Can you also fix?
>
> ```
> postgres=# CREATE SUBSCRIPTION sub CONNECTION 'dbname=tmp'
> ```

Hmm, I don't think you have to try very hard to find lots of examples
where tab-completion could be improved.

The scope of this particular thread/patch is *only* the pub/sub
options. I am happy to investigate what you reported, but that belongs
in another thread.

~~~

PSA patch v2.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachment: v2-0001-psql-tab-completion-of-pub-sub-options.patch
Description: Binary data

Reply via email to