Re: Fix detection of truncated zstd-compressed backups - Mailing list pgsql-hackers

From Chao Li
Subject Re: Fix detection of truncated zstd-compressed backups
Date
Msg-id AAF3BDDB-B9D1-4348-88AC-E0F5F864AF4F@gmail.com
Whole thread
In response to Re: Fix detection of truncated zstd-compressed backups  (Daniel Gustafsson <daniel@yesql.se>)
Responses Re: Fix detection of truncated zstd-compressed backups
List pgsql-hackers

> On Oct 9, 2026, at 06:36, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> On 7 Sep 2026, at 05:13, Chao Li <li.evan.chao@gmail.com> wrote:
>
>> PFA v9.
>
> Sorry for returning to this late, things have been busy.

No worries at all.

> I think v9 is pretty
> much ready to go in, but I have a few small comments/questions.
>
> - pg_fatal("could not uncompress data: %s", zp->msg);
> + pg_fatal("could not uncompress data: %s",
> + zp->msg ? zp->msg : "unknown error");
>
> Is this change a precaution or fixing a known issue which can happen?

This change was to address one of your previous comments:
```
>> pg_restore: error: could not uncompress data: (null)
>
> While not strictly related we should take the opportunity to fix this while in
> here.  Perhaps use "unkown error" in case zp->msg is null when erroring out?

Fixed by replacing (null) with “unknown error” in 0002.
```

> /* Lazy init */
> if (!LZ4Stream_init(state, false /* decompressing */ ))
> - {
> - pg_log_error("unable to initialize LZ4 library: %s",
> - LZ4F_getErrorName(state->errcode));
> return -1;
> - }
>
> I understand the rationale for this change, but I don't think it's improving
> things.  As it stands now we are losing context in the messaging when all
> errors say "can't read from input file", and the fread error cannot be captured
> with LZ4Stream_get_error since fread doesn't set errno.  The double log isn't
> pretty, but getting worse errormessages isn't better so we need to stick with
> it for now.  A refactoring for 20 could be to pass down an error buffer and
> psprintf the message there for the caller to log.  This close to v20 RC I don't
> want to redesign any part though.

Sounds fair.

>
> The above point, along with minor cleanup and polish here and there is in the
> attached v10.
>
> --
> Daniel Gustafsson
>
>
<v10-0001-Fix-detection-of-truncated-compressed-backups.patch><v10-0002-Fix-detection-of-truncated-zstd-and-LZ4-dump-dat.patch>

I compared v10 with v9:

* v10-0001 has only a formatting change and an updated commit message.
* v10-0002 restores the original error messages, as discussed in your second comment. It also has some formatting
changeson tests. 

So, v10 looks good to me.

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







pgsql-hackers by date:

Previous
From: surya poondla
Date:
Subject: Re: pg_walinspect: add functions to locate and list WAL by time and LSN
Next
From: Manu
Date:
Subject: Re: [PATCH] Extensible ReadyForQuery wire protocol message and C hook, for connection pools and WAIT FOR LSN