pgsql: Move pg_lzcompress.c to src/common.

On Thu, Dec 25, 2014 at 8:47 PM, Fujii Masao <fujii@postgresql.org> wrote:

Move pg_lzcompress.c to src/common.

Exposing compression and decompression APIs of pglz makes possible its
use by extensions and contrib modules. pglz_decompress contained a call
to elog to emit an error message in case of corrupted data. This function
is changed to return a status code to let its callers return an error instead.

This commit is required for upcoming WAL compression feature so that
the WAL reader facility can decompress the WAL data by using pglz_decompress.

Hmm... the buildfarm member prairiedog doesn't like this change. Because
pg_lzcompress.c uses the macros (like VARSIZE) for varlena datatypes which
are defined in postgres.h which client-side should not include. ISTM that
pg_lzcompress.c should be changed to be "varlena-free", in order to push it
in src/common.

Regards,

--
Fujii Masao

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

On 2014-12-25 22:39:58 +0900, Fujii Masao wrote:

On Thu, Dec 25, 2014 at 8:47 PM, Fujii Masao <fujii@postgresql.org> wrote:

Move pg_lzcompress.c to src/common.

Exposing compression and decompression APIs of pglz makes possible its
use by extensions and contrib modules. pglz_decompress contained a call
to elog to emit an error message in case of corrupted data. This function
is changed to return a status code to let its callers return an error instead.

This commit is required for upcoming WAL compression feature so that
the WAL reader facility can decompress the WAL data by using pglz_decompress.

Hmm... the buildfarm member prairiedog doesn't like this change. Because
pg_lzcompress.c uses the macros (like VARSIZE) for varlena datatypes which
are defined in postgres.h which client-side should not include. ISTM that
pg_lzcompress.c should be changed to be "varlena-free", in order to push it
in src/common.

Temporarily this can be solved by including postgres.h the way
e.g. pg_resetxlog does:

/*
* We have to use postgres.h not postgres_fe.h here, because there's so much
* backend-only stuff in the XLOG include files we need. But we need a
* frontend-ish environment otherwise. Hence this ugly hack.
*/
#define FRONTEND 1

#include "postgres.h"

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

On Thu, Dec 25, 2014 at 10:43 PM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2014-12-25 22:39:58 +0900, Fujii Masao wrote:

On Thu, Dec 25, 2014 at 8:47 PM, Fujii Masao <fujii@postgresql.org> wrote:

Move pg_lzcompress.c to src/common.

Exposing compression and decompression APIs of pglz makes possible its
use by extensions and contrib modules. pglz_decompress contained a call
to elog to emit an error message in case of corrupted data. This function
is changed to return a status code to let its callers return an error instead.

This commit is required for upcoming WAL compression feature so that
the WAL reader facility can decompress the WAL data by using pglz_decompress.

Hmm... the buildfarm member prairiedog doesn't like this change. Because
pg_lzcompress.c uses the macros (like VARSIZE) for varlena datatypes which
are defined in postgres.h which client-side should not include. ISTM that
pg_lzcompress.c should be changed to be "varlena-free", in order to push it
in src/common.

Temporarily this can be solved by including postgres.h the way
e.g. pg_resetxlog does:

/*
* We have to use postgres.h not postgres_fe.h here, because there's so much
* backend-only stuff in the XLOG include files we need. But we need a
* frontend-ish environment otherwise. Hence this ugly hack.
*/
#define FRONTEND 1

#include "postgres.h"

Urgh.. Would that mean keeping a local copy of SET_VARSIZE_4B_C and
VARSIZE_4B if this hack is not used? Looking at the git history, I am
seeing similar things in 2008 where pg_crc stuff was moved to src/port
(5c9c08d).
--
Michael

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

On 2014-12-25 23:46:52 +0900, Michael Paquier wrote:

On Thu, Dec 25, 2014 at 10:43 PM, Andres Freund <andres@2ndquadrant.com> wrote:

/*
* We have to use postgres.h not postgres_fe.h here, because there's so much
* backend-only stuff in the XLOG include files we need. But we need a
* frontend-ish environment otherwise. Hence this ugly hack.
*/
#define FRONTEND 1

#include "postgres.h"

Urgh.. Would that mean keeping a local copy of SET_VARSIZE_4B_C and
VARSIZE_4B if this hack is not used? Looking at the git history, I am
seeing similar things in 2008 where pg_crc stuff was moved to src/port
(5c9c08d).

Surely not. It seems like a much better idea to not have lzcompress deal
with varlena at all but pass that responsibility one layer upwards. That
shouldn't be very hard.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-12-25 23:46:52 +0900, Michael Paquier wrote:

Urgh.. Would that mean keeping a local copy of SET_VARSIZE_4B_C and
VARSIZE_4B if this hack is not used? Looking at the git history, I am
seeing similar things in 2008 where pg_crc stuff was moved to src/port
(5c9c08d).

Surely not. It seems like a much better idea to not have lzcompress deal
with varlena at all but pass that responsibility one layer upwards. That
shouldn't be very hard.

All files in src/common should be using the coding pattern

#ifndef FRONTEND
#include "postgres.h"
#else
#include "postgres_fe.h"
#endif

and as noted, it's going to take additional refactoring to make it
possible to do that in pg_lzcompress. In view of all the potentially
unportable stuff Andres committed today, we need the buildfarm to be
at full strength; so I've reverted this commit until it can be worked
out a bit better.

regards, tom lane

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

On Fri, Dec 26, 2014 at 3:31 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@2ndquadrant.com> writes:

Surely not. It seems like a much better idea to not have lzcompress deal
with varlena at all but pass that responsibility one layer upwards. That
shouldn't be very hard.

I'll figure out something.

and as noted, it's going to take additional refactoring to make it
possible to do that in pg_lzcompress. In view of all the potentially
unportable stuff Andres committed today, we need the buildfarm to be
at full strength; so I've reverted this commit until it can be worked
out a bit better.

Makes sense.
--
Michael

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers