pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Tomas Vondra <tomas@vondra.me>
Cc: Evgeniy Gorbanev <gorbanyoves@basealt.ru>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: No error checking when reading from file using zstd in pg_dump
Date: Thu, 26 Jun 2025 09:33:56 -0400
Message-ID: <1448707.1750944836@sss.pgh.pa.us> (raw)
In-Reply-To: <A9EB5A06-6C77-418B-B582-AFE5F027B4B3@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>
	<520142.1750083607@sss.pgh.pa.us>
	<D011594F-7FA3-4BE4-BF89-B1AB18CD3456@yesql.se>
	<644360.1750096605@sss.pgh.pa.us>
	<f2dec453-b6af-4495-a75d-a39628411510@vondra.me>
	<666035.1750106940@sss.pgh.pa.us>
	<EBEE0042-E30B-43CB-8BAA-F51908248928@yesql.se>
	<1296195.1750867134@sss.pgh.pa.us>
	<A9EB5A06-6C77-418B-B582-AFE5F027B4B3@yesql.se>

Daniel Gustafsson <daniel@yesql.se> writes:
> On 25 Jun 2025, at 17:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It doesn't return true anymore.  Should be more like
>> + * Returns nothing.  Exits via pg_fatal for all error conditions.

> Instead of this I changed the write_func signature to return the number of
> bytes written as size_t.  The API is loosely modelled around the libc stream API so
> going to void seemed less justifiable than size_t, even if the actual
> returnvalue is fairly useless due to erroring out via pg_fatal.

Hm.  My one concern about that is that using "void" ensures that the
compiler will catch any write_funcs or callsites that we missed
updating, whereas replacing bool with size_t probably prevents that
detection.

Of course this is now moot for the in-core code since we presumably
caught everything already.  But I wonder about patches (say to
support an additional compression method) that might be in the
pipeline, or in use in some local fork somewhere.  There's no
certainty that they'd get the word, especially since any tests
that didn't exercise failure conditions would still work.

So on the whole I prefer the "void" approach.  I'm not dead
set on that though, it's just a niggling worry.

			regards, tom lane





view thread (29+ messages)  latest in thread

Message-ID: <1448707.1750944836@sss.pgh.pa.us>
Permalink:  ../1448707.1750944836@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/1448707.1750944836@sss.pgh.pa.us

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, daniel@yesql.se, tomas@vondra.me, 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: <1448707.1750944836@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox