Date: Tue, 14 Mar 2023 11:07:25 +0000
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
From: gkokolatos@pm.me
Cc: Justin Pryzby <pryzby@telsasoft.com>, shiy.fnst@fujitsu.com, Michael Paquier <michael@paquier.xyz>, pgsql-hackers@lists.postgresql.org, Rachel Heaton <rachelmheaton@gmail.com>
Subject: Re: Add LZ4 compression in pg_dump
Message-ID: <LtyfSKFLLHwZB8M9HDPXJ3yqzyiWwGRe5gADfUVZo0MNCl8eWV43tDpZHlGhgiObPaFWZmIpZxgB714UFh8k8X-9fcADJP5IReT75a1Jhco=@pm.me>
In-Reply-To: <c1493de3-7d97-c3c9-dd39-5a781e10a7b8@enterprisedb.com>
References: <AcPUJxzDanXfXdqYOrrZ0L9J9qcnCblkc5z5x9IgKUm7d89pa6B7y8fhUOb2JGDef8h3VP6Ts7j-BYSMmtn50jVuHAT2nBleGosIdVyhG7Q=@pm.me> <mYBKRYonRy5OiDDYxcxOOJ5xM9iLnuDsGP92zC7mjw8ew1wiccYZIpTvQEmtgRoAXRhaj91SAKwtFluUtRDZ3r6pRdGYLncWK7rXIQrHhFY=@pm.me> <22cb80ed-ec87-54c7-fd5b-cdcf79766bb1@enterprisedb.com> <8421f53d-c9f7-c02e-3278-9f9522a8e8f3@enterprisedb.com> <20230228235834.GC30529@telsasoft.com> <lsZgBfZRB5w5slcXnKwoL9qgpzdlAC_UwYHXfbj1oioWsNkckwr2BcLvvCi9-x7eV261s8AP5OHwROZ-QYtpeixxt73DQ_d6-LNfghYOaIQ=@pm.me> <b0ced9ea-e92d-f9d1-7a2f-075881b755b2@enterprisedb.com> <ZAfosKXCewHVENaZ@telsasoft.com> <c1493de3-7d97-c3c9-dd39-5a781e10a7b8@enterprisedb.com>
Feedback-ID: 6383013:user:proton
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/LtyfSKFLLHwZB8M9HDPXJ3yqzyiWwGRe5gADfUVZo0MNCl8eWV43tDpZHlGhgiObPaFWZmIpZxgB714UFh8k8X-9fcADJP5IReT75a1Jhco%3D%40pm.me>
Precedence: bulk


------- Original Message -------
On Monday, March 13th, 2023 at 10:47 PM, Tomas Vondra <tomas.vondra@enterpr=
isedb.com> wrote:


>=20
> > Change pg_fatal() to an assertion+comment;
>=20
>=20
> Yeah, that's reasonable. I'd even ditch the assert/comment, TBH. We
> could add such protections against "impossible" stuff to a zillion other
> places and the confusion likely outweighs the benefits.
>=20

A minor note to add is to not ignore the lessons learned from a7885c9bb.

For example, as the testing framework stands, one can not test that the
contents of the custom format are indeed compressed. One can infer it by
examining the header of the produced dump and searching for the
compression flag. The code responsible for writing the header and the
code responsible for actually dealing with data, is not the same. Also,
the compression library itself will happily read and write uncompressed
data.

A pg_fatal, assertion, or similar, is the only guard rail against this
kind of error. Without it, the tests will continue passing even after
e.g. a wrong initialization of the API. It was such a case that lead to
a7885c9bb in the first place. I do think that we wish it to be an
"impossible" case. Also it will be an untested case with some history
without such a guard rail.

Of course I will not object to removing it, if you think that is more
confusing than useful.

Cheers,
//Georgios

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