On Wed, Oct 29, 2025 at 8:10 PM Chao Li <li.evan.chao@gmail.com> wrote:
...
> I think 0001 basically good. A tiny comment is that, in inval.c, "wal_level>=logical” doesn’t have white-spaces
around“=“, while in the other two files, they have. So maybe all add white-spaces around “=“ here.
>
> For 0002, I have a fixed feeling.
>
> This change is okay to me:
> ```
> - if (wal_level != WAL_LEVEL_LOGICAL)
> + if (wal_level < WAL_LEVEL_LOGICAL)
> ```
>
> But I really don’t like the error message changes:
> ```
> if (nslots_on_old > 0 && strcmp(wal_level, "logical") != 0)
> - pg_fatal("\"wal_level\" must be \"logical\" but is set to \"%s\"",
> + pg_fatal("\"wal_level\" must be \"logical\" or higher but is set to \"%s\"",
> ```
> And
> ```
> -HINT: Set "wal_level" to "logical" before creating subscriptions.
> +HINT: Set "wal_level" >= "logical" before creating subscriptions.
> ```
>
> Which will really make end users confused. I believe end users don’t care about so-called future extensions, they
onlyneed accurate information.
>
Hi Chao.
Thanks for your review comments. Here are the v3* patches.
* Patch 0001 - Fixed spaces per suggestion.
* Patch 0002 - Unchanged. For now, this patch 0002 is mostly only a
placeholder until Sawada-San's patch [1] is pushed, and then I will
revisit it. There is lots of overlap, so perhaps much of it will be
made redundant.
======
[1] https://www.postgresql.org/message-id/flat/CAD21AoAtqpZW%3DzC57qxEFbBCVhJ2kF2HXmuUT3w_tHGZCYmpnw%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia