Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: [BUG] [PATCH] pg_basebackup produces wrong incremental files
 after relation truncation in segmented tables
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CA+TgmoZ7LQ40tLGpHw0U51CReH_8jv0sO0dALGRJp=FT+ZtGFQ@mail.gmail.com>
Date: Tue, 16 Dec 2025 16:05:56 +0800
Cc: Amul Sul <sulamul@gmail.com>,
 Oleg Tkachenko <oatkachenko@gmail.com>,
 pgsql-hackers@lists.postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <DA5AF966-A9D5-4C02-83B5-773AF47F5332@gmail.com>
References: <00FEFC88-EA1D-4271-B38F-EB741733A84A@gmail.com>
 <F687B661-E975-4383-8A94-F9130FE7ACC1@gmail.com>
 <CAAJ_b94y98BWWgf0KMw-E_pqM_y8+RogqHtu5nK0kA1uBE3WWw@mail.gmail.com>
 <CA+TgmoZ7LQ40tLGpHw0U51CReH_8jv0sO0dALGRJp=FT+ZtGFQ@mail.gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/DA5AF966-A9D5-4C02-83B5-773AF47F5332%40gmail.com>
Precedence: bulk


> On Dec 16, 2025, at 00:35, Robert Haas <robertmhaas@gmail.com> wrote:
>=20
> [ sorry for not noticing this thread sooner; thanks to Andres for
> pointing me to it ]
>=20
> On Mon, Dec 15, 2025 at 9:01=E2=80=AFAM Amul Sul <sulamul@gmail.com> =
wrote:
>> Thanks for the reproducer; I can see the reported issue, but I am not
>> quite sure the proposed fix is correct and might break other cases (I
>> haven't tried constructed that case yet) but there is a comment
>> detailing that case just before the point where you are planning to =
do
>> the changes:
>>=20
>>    /*
>>     * The truncation block length is the minimum length of the =
reconstructed
>>     * file. Any block numbers below this threshold that are not =
present in
>>     * the backup need to be fetched from the prior backup. At or =
above this
>>     * threshold, blocks should only be included in the result if they =
are
>>     * present in the backup. (This may require inserting zero blocks =
if the
>>     * blocks included in the backup are non-consecutive.)
>>     */
>>=20
>> IIUC, we might need the original assignment logic as it is. But we
>> need to ensure that truncation_block_length is not set to a value =
that
>> exceeds RELSEG_SIZE.
>=20
> I think you're right. By way of example, let's say that the current
> length of the file is 200 blocks, but the limit block is 100 blocks
> into the current segment. That means that the only blocks that we can
> get from any previous backup are blocks 0-99. Blocks 100-199 of the
> current segment are either mentioned in the WAL summaries we're using
> for this backup, or they're all zeroes. We can't set the
> truncation_block_length to a value greater than 100, or we'll go
> looking for the contents of any zero-filled blocks in previous
> backups, will will either fail or produce the wrong answer. But Oleg
> is correct that we also shouldn't set it to a value greater than
> RELSEG_SIZE. So my guess is that the correct fix might be something
> like the attached (untested, for discussion).
>=20
> --=20
> Robert Haas
> EDB: http://www.enterprisedb.com
> <v1-0001-Don-t-set-the-truncation_block_length-rather-than.patch>


The change looks good to me. Only nitpick is:

```
Subject: [PATCH v1] Don't set the truncation_block_length rather than
 RELSEG_SIZE.
```

I guess you meant to say =E2=80=9Clarger (or greater) than=E2=80=9D =
instead of =E2=80=9Crather than=E2=80=9D.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/