| From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
|---|---|
| To: | Hüseyin Demir <huseyin(dot)d3r(at)gmail(dot)com> |
| Cc: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: PSQL - improve tab completion for pub/sub options |
| Date: | 2026-08-10 05:30:16 |
| Message-ID: | CAHut+PvT6WJ4HTwuRMHo=hGv++Zfoe4ciDQLFo-T8PHr=jeDMA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Aug 9, 2026 at 6:55 PM Hüseyin Demir <huseyin(dot)d3r(at)gmail(dot)com> wrote:
> Hi,
>
> Created a v3 and issued the following topics I found.
>
> - Fixed typo in CREATE SUBSCRIPTION block: max_rentention_duration →
> max_retention_duration (was causing the TailMatches check to never
> fire for that option)
> - Applied pgindent to clean up indentation in the ALTER PUBLICATION
> and ALTER SUBSCRIPTION SET blocks (reproduces the same fix pattern
> identified in v1 review)
Thanks for your review, and catching that typo.
>
> While reviewing, I noticed you applied the "(*)" idiom to ALTER
> PUBLICATION/SUBSCRIPTION SET blocks (great fix), but the CREATE
> PUBLICATION/SUBSCRIPTION blocks still use the seen_with scan. For
> consistency, the same pattern could be applied there too. What is your
> opinion about it?
>
>
Yeah, IIRC, I already tried to do that but came to grief.
e.g. Compared to ALTER, the CREATE PUBLICATION/SUBSCRIPION has a lot
more flexibility before the "WITH (", like multiple publication
clauses or multiple subscribed publications, so I'd need MatchManyN
for those...
However, MatchAnyN is only supported inside Matches() -- not HeadMatches().
But if we try to use Matches like:
else if (Matches("CREATE", "PUBLICATION", MatchAny, MatchAnyN, "WITH", "(*") &&
!Matches("CREATE", "PUBLICATION", MatchAny, MatchAnyN, "WITH", "(*)"))
... then it just doesn't work for the comma "," separators. IIUC it's
because the comma is another token.
The following response from AI explains it better:
------
"(*" means: one word that starts with (. It matches "(", "(a",
"(publish", but it only ever matches a single token from
previous_words[].
That's actually what makes the HeadMatches approach work —
get_previous_words() groups a completed parenthesized expression into
one token (e.g. "(publish = insert)"), which matches "(*)" (starts
with (, ends with )). An uncompleted one like "(" matches "(*" but not
"(*)". Words typed inside the open parens appear at lower indices and
don't interfere with the position of the ( token when checked from the
head.
So the multi-word wildcard you'd need is MatchAnyN (""), which is only
supported by Matches/MatchesCS — and that's exactly why Matches(...,
MatchAnyN, "WITH", "(*") breaks after a comma: by the time options are
typed, previous_words[0] is something like "insert,", not "(".
------
So, it's catch-22:
syntax flexiblity of CREATE means I want MatchAnyN
--> but MatchAnyN is not compatible with HeadMatches
--> need to use Matches
--> but Matches cannot work, because logic needs needs HeadMatches
;-(
~~
I've experimented with other things like:
else if (Matches("CREATE", "PUBLICATION", MatchAny, MatchAnyN, "WITH", "(*") ||
Matches("CREATE", "PUBLICATION", MatchAny, MatchAnyN, "WITH", "(", MatchAnyN))
... but, apparently the Matches() logic only recognizes the first MatchAnyN.
To cut a long story short, the only approach that I found that works
was the `seen_with` scan.
~~~
PSA v4. It has unchanged content from v3, but restores the original
commit message of my patch.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
| Attachment | Content-Type | Size |
|---|---|---|
| v4-0001-psql-tab-completion-of-pub-sub-options.patch | application/octet-stream | 8.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-08-10 05:31:50 | Re: [PATCH] Fix vacuum_delay_point happening inside lock |
| Previous Message | Amit Kapila | 2026-08-10 05:01:58 | Re: Proposal: Conflict log history table for Logical Replication |