Re: [PROPOSAL] comments in repl_scanner

Hi Michael,

Thanks for your notes.

čt 16. 10. 2025 v 6:08 odesílatel Michael Paquier <michael(at)paquier(dot)xyz>
napsal:

> On Tue, Oct 14, 2025 at 01:13:38PM +0200, Matěj Klonfar wrote:
> > I can imagine this limitation is likely a holdover from the system's
> > evolution from physical replication where comments make no sense.
> However,
> > in logical replication walsender mode both SQL and replication statements
> > can be issued [1], so the current state brings the necessity to
> distinguish
> > when to inject the comment and when not to. What do you feel, are there
> any
> > unexpected impacts of extending the replication grammar with comments?
> >
> > I attached a simple patch extending the `replication/repl_scanner.l` with
> > following test:
> >
> > What do you feel, is that a good idea and/or are there any downsides I am
> > missing? Thank you all for the feedback.
>
> A downside here is the extra maintenance that this creates. I cannot
> get much excited about the support of comments in the context of
> replication commands, TBH.

Makes sense, on the other hand, the most of the work was done
in e4a8fb8fefb9 where `yyextra` was introduced to handle the context
information (necessary for nested `/* */` comments). This proposal is then,
in my opinion, just a small enhancement built on top of that with no
significant impact to the maintenance costs.

> That's just one opinion, of course, others
> may have a different view. Note that even if one uses
> "replication=database", the code falls back to the main query parser,
> where comments work.
>
I do not agree. Tried now on the current master HEAD (commit 41c674d2):
```

psql "dbname=postgres replication=database"

Border style is 2.

Line style is unicode.

psql (14.19 (Homebrew), server 19devel)

WARNING: psql major version 14, server major version 19.

Some psql features might not work.

Type "help" for help.

postgres=# /* foo */ IDENTIFY_SYSTEM;

2025-10-16 09:49:59.152 CEST [49754] ERROR: syntax error at or near
"IDENTIFY_SYSTEM" at character 11

2025-10-16 09:49:59.152 CEST [49754] STATEMENT: /* foo */ IDENTIFY_SYSTEM;

ERROR: syntax error at or near "IDENTIFY_SYSTEM"

LINE 1: /* foo */ IDENTIFY_SYSTEM;

```

Matej

> FWIW, this is a bit like a past proposal with making the replication
> commands case-insensitive, back in 2017:
>
> https://www.postgresql.org/message-id/CAB7nPqSg2ZECE+ctGXieiCpVFibLyWgwAGaoEP3-zwYqJwaP-g@mail.gmail.com
>
> The argument for rejection back then is that these commands are not
> for manual consumption, so we should keep the replication grammar file
> simpler. Perhaps there could be an argument with allowing commands
> when it comes to the logging of replication commands in the server
> logs, where comments can be used as a reference. That's a very narrow
> case, so I'd still argue that this is not enough to balance in favor
> of this proposal. Just my 2c.
> --
> Michael
>