From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
cc: Evgeniy Gorbanev <gorbanyoves@basealt.ru>,
        pgsql-hackers@lists.postgresql.org
Subject: Re: No error checking when reading from file using zstd in pg_dump
In-reply-to: <3815B6CD-924B-40FA-823A-55E2F8F24602@yesql.se>
References: <6b9817a8-88ec-4efd-b441-9e2a0439c6b8@basealt.ru>
 <C22FF2A1-783D-4334-998B-2D4432EFD7C5@yesql.se>
 <95255a86-4bc7-4ff4-aa26-4441aafdf0fb@basealt.ru>
 <522DB522-3962-4F91-A691-2DE790F6CCD6@yesql.se>
 <b653a2f4-1dcb-45ba-b899-fc6aa80794f2@basealt.ru>
 <D38B293E-E886-4F0D-BD5A-C958DCB2344C@yesql.se>
 <e2b7edd1-c5b8-484d-9c79-7a7c4466fad2@basealt.ru>
 <517794.1750082166@sss.pgh.pa.us>
 <3815B6CD-924B-40FA-823A-55E2F8F24602@yesql.se>
Comments: In-reply-to Daniel Gustafsson <daniel@yesql.se>
	message dated "Mon, 16 Jun 2025 16:11:58 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <520141.1750083607.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Mon, 16 Jun 2025 10:20:07 -0400
Message-ID: <520142.1750083607@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/520142.1750083607%40sss.pgh.pa.us>
Precedence: bulk

Daniel Gustafsson <daniel@yesql.se> writes:
>> On 16 Jun 2025, at 15:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I've not checked to see what the other users of this API do, but
>> if they're all like this then we need to fix that comment.

> AFAICT all other callers of this API are throwing an error with pg_fatal=
, and
> so does the function in question for ZStd decompression errors.

I think I agree that we need to handle the ferror() case inside the
read_func for consistency.  But there is another problem, which is
that neither of its callers are paying attention to the API: as
defined, the read_func can never return anything but "true",
which is how Zstd_read behaves.  But both the callers in
compress_zstd.c seem to think they should test its result to
detect EOF.  Apparently, our tests do not cover the case of an
unexpected EOF.

This API is actually quite bizarrely defined, if you ask me.
Returning the byte count is optional, but if you don't pay
attention to the byte count you cannot know if you got any
data.  At best, that's bug-encouraging.

			regards, tom lane