Re: Add LZ4 compression in pg_dump

From: gkokolatos(at)pm(dot)me
To: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
Cc: Alexander Lakhin <exclusion(at)gmail(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Rachel Heaton <rachelmheaton(at)gmail(dot)com>
Subject: Re: Add LZ4 compression in pg_dump
Date: 2023-03-16 17:04:16
Message-ID: JduQzRIUMhF5NDvC_klfyHTDhzb5Py60xH9ICRUtluHQ4dD02nw-BH7WfX7DQG6gaFWl1Q919RvX3Xvm_5mau_0YKRQeUs6ezUSFDlFVPsE=@pm.me
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

------- Original Message -------
On Tuesday, March 14th, 2023 at 4:32 PM, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:

>
>
>
>
> On 3/14/23 16:18, gkokolatos(at)pm(dot)me wrote:
>
> > ...> Would you mind me trying to come with a patch to address your points?
>
>
> That'd be great, thanks. Please keep it split into smaller patches - two
> might work, with one patch for "cosmetic" changes and the other tweaking
> the API error-handling stuff.

Please find attached a set for it. I will admit that the splitting in the
series might not be ideal and what you requested. It is split on what
seemed as a logical units. Please advice how a better split can look like.

0001 is unifying types and return values on the API
0002 is addressing the constant definitions
0003 is your previous 0004 adding comments

As far as the error handling is concerned, you had said upthread:

> I think the right approach is to handle all library errors and not just
> let them through. So Gzip_write() needs to check the return value, and
> either call pg_fatal() or translate it to an error defined by the API.

While working on it, I thought it would be clearer and more consistent
for the pg_fatal() to be called by the caller of the individual functions.
Each individual function can keep track of the specifics of the error
internally. Then the caller upon detecting that there was an error by
checking the return value, can call pg_fatal() with a uniform error
message and then add the specifics by calling the get_error_func().

Thoughts?

Cheers,
//Georgios

>
> regards
>
> --
> Tomas Vondra
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company

Attachment Content-Type Size
v2-0002-Clean-up-constants-in-pg_dump-s-compression-API.patch text/x-patch 6.5 KB
v2-0003-Improve-compress_lz4-documentation.patch text/x-patch 2.9 KB
v2-0001-Improve-type-handling-in-pg_dump-s-compress-file-.patch text/x-patch 12.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2023-03-16 17:05:06 Re: gcc 13 warnings
Previous Message Kumar, Sachin 2023-03-16 16:56:38 RE: Initial Schema Sync for Logical Replication