Обсуждение: pgsql: Move pg_lzcompress.c to src/common.
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. Michael Paquier Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/60838df922345b26a616e49ac9fab808a35d1f85 Modified Files -------------- src/backend/access/heap/tuptoaster.c | 11 +- src/backend/utils/adt/Makefile | 4 +- src/backend/utils/adt/pg_lzcompress.c | 779 -------------------------------- src/common/Makefile | 3 +- src/common/pg_lzcompress.c | 786 +++++++++++++++++++++++++++++++++ src/include/common/pg_lzcompress.h | 112 +++++ src/include/utils/pg_lzcompress.h | 112 ----- src/tools/msvc/Mkvcbuild.pm | 3 +- 8 files changed, 911 insertions(+), 899 deletions(-)
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
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
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
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
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
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