Re: Fix detection of truncated zstd-compressed backups

> On Aug 11, 2026, at 00:28, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>
>> On 10 Aug 2026, at 10:11, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>> On Aug 10, 2026, at 14:45, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>>> See attached 0002 for the fix of gzip streamer. I will check the lz4 streamer next.
>>
>> Confirmed that lz4 also has the same problem. See the similar repro script:
>
> Thanks for this patchset, I think this is something we should fix. I took the
> liberty to squash the patchset into a single patch to start preparing it for
> the final shape, as well as adding a testcase to verify this.

Thank for taking care of this patch.

>
> + /* Reject empty input, which does not contain a complete zstd frame. */
> + streamer->decompression_ret = 1;
>
> I am not a huge fan of this, we claim that we save the return value but then we
> assign a value which hasn't yet been returned as a sentinel. Given that the
> return is a size_t we also can't really invent a sentinel. Since we don't
> actually use the returned value for anything but "done or not-done", so I
> propose something like the attached which interprets the value and stores a
> named state. What are your thoughts on this?

I agree with the direction. The new proposal can distinguish an empty stream from a truncated stream.

>
>
> <v4-0001-Fix-detection-of-truncated-compressed-backups.patch>

A few comments with v4:

1 - ztsd
```
+ if (mystreamer->state != STREAM_FINISHED)
+ pg_fatal("could not decompress data: compressed stream is incomplete");
+ else if (unlikely(mystreamer->state == STREAM_NEW))
+ pg_fatal("could not decompress data: compressed stream is empty");
```

This is a small logic error. As STREAM_NEW!=STREAM_FINISHED already, the “else if” is unreachable. We should check if (unlikely(mystreamer->state == STREAM_NEW) first.

2 - ztsd
```
+ /* The stream is only done when ZSTD_decompressStream returns 0 */
+ if (ret)
+ mystreamer->state = STREAM_HAS_DATA;
+ else
+ mystreamer->state = STREAM_FINISHED;
```

When ret == 0, that only means the current frame is complete, so the comment “the stream is only done” sounds too strong, I would change to “The frame is only done when …”.

3 - ztsd and lz4
```
+typedef enum
+{
+ STREAM_NEW,
+ STREAM_HAS_DATA,
+ STREAM_FINISHED,
+} pg_stream_state;
```

Similar to comment 2, return == 0 means the current is complete and >0 means the current frame is incomplete, thus I would rename STREAM_HAS_DATA to FRAME_HAS_DATA, and STREAM_FINISHED to FRAME_FINISHED.

I addressed all the 3 comments in v5.

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