On Wed, Oct 29, 2025 at 8:10 PM Chao Li <[email protected]> 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 only need 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

Attachment: v3-0002-fix-wal_level-equality-code.patch
Description: Binary data

Attachment: v3-0001-fix-wal_level-equality-comments.patch
Description: Binary data

Reply via email to