> On Oct 27, 2025, at 10:59, Peter Smith <[email protected]> wrote:
>
> On Fri, Oct 24, 2025 at 9:09 PM Ashutosh Bapat
> <[email protected]> wrote:
>>
>> On Wed, Oct 22, 2025 at 4:49 AM Peter Smith <[email protected]> wrote:
>>>
>>> On Tue, Oct 21, 2025 at 2:11 AM Robert Haas <[email protected]> wrote:
>>>>
>>>> On Mon, Oct 20, 2025 at 3:20 AM Peter Smith <[email protected]> wrote:
>>>>> Do you have thoughts about the patch?
>>>>
>>>> I agree with the rationale that Ashutosh states but I don't see a
>>>> strong need to patch the code to make this a 100% invariable rule. (Of
>>>> course, someone else may disagree, which is fine.)
>>>>
>>>
>>> In case it makes any difference...
>>>
>>> The codebase already follows this rule in 95% of cases. The patch
>>> simply corrects a couple of inconsistencies that appeared to be
>>> accidental oversights.
>>
>> I think we should accept comment-only changes in the patch. With those
>> changes comments are consistent with the code; otherwise code-readers
>> will get confused. I don't have a strong opinion about the comment +
>> code changes though. They may wait till changes in [1] get committed.
>> As Robert said, we may not want that to be an invariable rule.
>>
>
> OK, here is the same patch split into comment-only changes, and code-changes.
>
> ======
> Kind Regards,
> Peter Smith.
> Fujitsu Australia
> <v2-0002-fix-wal_level-equality-code.patch><v2-0001-fix-wal_level-equality-comments.patch>
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.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
