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: Wed, 25 Jun 2025 11:58:54 -0400
Message-ID: <1296195.1750867134@sss.pgh.pa.us> (raw)
In-Reply-To: <EBEE0042-E30B-43CB-8BAA-F51908248928@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>

Daniel Gustafsson <daniel@yesql.se> writes:
> I spent a little bit of time reading over all the implementations and cross
> referencing the API for conformity, and came up with the attached.  The 0001
> patch is the one from upstream, and each subsequent commit is fixing one
> function for all the implementations.  Before pushing it should all be squashed
> into a single commit IMHO.

Thanks for tackling this!

I looked over this patchset briefly, and found a couple of nits:

v5-0002, in compress_io.h:

+	 * Returns true on success and throws error for all error conditions.

It doesn't return true anymore.  Should be more like

+	 * Returns nothing.  Exits via pg_fatal for all error conditions.

In LZ4Stream_write: you dropped the bit about

-			errno = (errno) ? errno : ENOSPC;

but I think that's still necessary: we must assume ENOSPC if fwrite
doesn't set errno.  Other fwrite callers (write_none, Zstd_write) need
this too.  v5-0004 has an instance too, in Zstd_close.  I did not check
to see if other fwrite calls are OK, but it'd be good to verify
that they all follow the pattern of presetting errno to 0 and then
replacing that with ENOSPC.

			regards, tom lane





view thread (29+ messages)  latest in thread

Message-ID: <1296195.1750867134@sss.pgh.pa.us>
Permalink:  ../1296195.1750867134@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/1296195.1750867134@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: <1296195.1750867134@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