On Sun, Aug 9, 2026 at 6:55 PM Hüseyin Demir <[email protected]> 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: v4-0001-psql-tab-completion-of-pub-sub-options.patch
Description: Binary data

Reply via email to