Re: Fix detection of truncated zstd-compressed backups

From: Japin Li <japinli(at)hotmail(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Osama Abdul Qader <osamaabdulqader(dot)cs(at)gmail(dot)com>
Subject: Re: Fix detection of truncated zstd-compressed backups
Date: 2026-08-13 03:39:06
Message-ID: SY7PR01MB10921B4071D855E80DAD7F695B6DB2@SY7PR01MB10921.ausprd01.prod.outlook.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

References: <5962B878-C43D-4EBC-9E95-1F945CE5E586(at)gmail(dot)com>
<CAN4CZFNuV2NKXbPTGkN8chga01sVGpUcNinM-HpdyFtH0eJR0w(at)mail(dot)gmail(dot)com>
<06FBF640-1A1D-48B0-BBEA-F4EBB4017309(at)gmail(dot)com>
<91EEEF17-A331-4CEA-ABEA-E63C4FBDB173(at)gmail(dot)com>
<2E9971CA-062B-4EC0-8858-9979C73A3888(at)gmail(dot)com>
<CF427D27-3AF6-4C0E-ABAE-9FF7DED20FCD(at)yesql(dot)se>
<1381EA72-8932-4734-9F7B-8315208EDB96(at)gmail(dot)com>
<CAC+8b5imyLMnif=JBYnDK9H6xLhAgSP_Yw2WtU-vTOb34iqW5w(at)mail(dot)gmail(dot)com>
<6D85CC75-CB35-45E3-ADBB-8287DF6BB429(at)gmail(dot)com>
<CAC+8b5itHRfCcgSt4WKCxkcOZ5+DvmQR2mavj1gU6YhBBpvZrQ(at)mail(dot)gmail(dot)com>
<6A9A9FC1-B59E-4454-8B95-570054038EFE(at)gmail(dot)com>
<CAN4CZFM_RaAxiRcf0JmhmNqUac+9-3KYGVu+Uvc43VxH7bibaw(at)mail(dot)gmail(dot)com>
<3664E105-4512-4BD2-99A7-0F5C25EBB173(at)gmail(dot)com>
User-Agent: mu4e 1.14.1; emacs 30.2
Hi Chao,

Thanks for updating the patch.

Date: Thu, 13 Aug 2026 11:32:59 +0800

On Thu, 13 Aug 2026 at 09:47, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>> On Aug 13, 2026, at 05:56, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>>
>> I have two minor comment for v5, otherwise it looks good to me:
>
> Hi Zsolt,
>
> Thanks a lot for reviewing.
>
>>
>> +typedef enum
>> +{
>> + STREAM_NEW,
>> + STREAM_HAS_DATA,
>> + STREAM_FINISHED,
>> +} pg_stream_state;
>> +
>>
>> This is duplicated in two files, wouldn't be astreamer.h a better place for it?
>
> I think this makes sense. Plus your second comment, this enum can be shared by all 3 astreamers. I moved it to astreamer.h and renamed it to astreamer_decompression_state.
>

The astreamer_decompression_state should be added to pgindent's typedefs.list.

>>
>> + if (!mystreamer->stream_finished)
>> + pg_fatal("could not decompress data: compressed stream is incomplete");
>> +
>>
>> The other two checks distinguish empty from incomplete, is this
>> difference intended?
>
> I do not think that is intentional. My initial implementation did not
> check for an empty stream, and gzip uses a different pattern from zstd
> and LZ4, which is probably why Daniel initially added the empty-stream
> check only to zstd and lz4.
>
> I have now added the same check to gzip so that all three astreamers are consistent.
>
> PFA v6: addressed Zsolt’s comments.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2026-08-13 04:16:19 Re: Allow a condition string in an injection point
Previous Message shveta malik 2026-08-13 03:31:45 Re: [PATCH] Release replication slot on error in SQL-callable slot functions