| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | Kiran Kaki <itskkpg(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz> |
| Subject: | Re: pg_walinspect: fix LSN validation messages and empty range handling |
| Date: | 2026-09-21 22:50:42 |
| Message-ID: | 4179E230-F999-437E-B964-94659F0AD3D7@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Sep 21, 2026, at 16:55, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Sun, Sep 20, 2026 at 11:56 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>> PFA v2:
>
> Thanks for reporting and sending the patch.
>
> Yes, it's an oversight in 5c1b6628075a. +1 for "must be less than or
> equal to", since that is the wording used elsewhere in the code.
>
> That said, an error is raised only when no valid record is found at or
> after the start LSN (or the input LSN), either because that WAL is
> already removed or because nothing valid follows it, which is what the
> documentation already mentions. Once a record is found,
> pg_get_wal_record_info() emits it, whereas the range functions emit
> only the records ending at or before the end LSN, so equal start and
> end LSNs emit nothing. A start LSN equal to the current LSN ends up
> the same way, since the end LSN is capped at the current LSN, making
> the two equal, and it errors because nothing follows the current LSN.
>
> The v2 patch looks good to me. I adjusted the commit message and
> re-attached the patch, which I think is ready for commit. I'm fine not
> back-patching this for a couple of reasons. The error is still
> reported in the back-branches, just with slightly incorrect wording
> matching the condition the code uses, and it went unnoticed for many
> years. CC-ing Michael for any thoughts.
Thanks for reviewing and updating v2.
>
> While here, do we also need to fix AlterSubscription()'s skip WAL
> location and ParseVariableDouble()'s min and max bound messages? Maybe
> separately.
>
Yeah, we can do that. As those two functions are in core, and 0001 changes the extension, I put the new changes to 0002.
PFA v3.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v3-0001-Fix-input-LSN-validation-error-messages-in-pg_wal.patch | application/octet-stream | 6.4 KB |
| v3-0002-Fix-inaccurate-inclusive-bound-error-messages.patch | application/octet-stream | 5.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-09-21 22:53:17 | Re: Support for 8-byte TOAST values, round two |
| Previous Message | Bharath Rupireddy | 2026-09-21 22:45:32 | Re: [PATCH] Explain what the default output_plugin_libraries do |