Thread: pgsql: pg_basebackup: Fix a couple of recently-introduced bugs.
pg_basebackup: Fix a couple of recently-introduced bugs. The server expects the compression level to be between 1 and 9, but Z_DEFAULT_COMPRESSION is -1, so we must not try to send that value to the server. Because pg_basebackup's -R option is implemented on the client side, it can't be used in combination with a backup target. Error out if someone tries that, instead of silently ignoring the option. Both issues were reported by Tushar Ahuja; patch by me. Discussion: http://postgr.es/m/CA+TgmoaMwgdx8HxBjF8hmbohVvPL_0H5LqNrSq0uU+7BKp_Q2A@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/dabf63bc9a5b71f61c2c8712c85d33fdc06efa8c Modified Files -------------- src/bin/pg_basebackup/pg_basebackup.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
On Thu, Jan 27, 2022 at 04:15:45PM +0000, Robert Haas wrote: > pg_basebackup: Fix a couple of recently-introduced bugs. > > The server expects the compression level to be between 1 and 9, but > Z_DEFAULT_COMPRESSION is -1, so we must not try to send that value > to the server. If I'm not wrong, this is causing failures on cfbot. https://cirrus-ci.com/task/6312181619752960?logs=build#L2056 [17:24:48.348] c:\cirrus\src\backend\replication\basebackup_gzip.c(90): warning C4715: 'bbsink_gzip_new': not all controlpaths return a value [c:\cirrus\postgres.vcxproj] [17:24:48.348] c:\cirrus\src\bin\pg_basebackup\pg_basebackup.c(1922,67): error C2065: 'Z_DEFAULT_COMPRESSION': undeclaredidentifier [c:\cirrus\pg_basebackup.vcxproj] The warning seems to be what's fixed by David's patch here. https://www.postgresql.org/message-id/CA%2BTgmobSeRFM84pwa5HfZCGRx%3DQ-iFfPFZfq-4%3Dt1p04yxg_Og%40mail.gmail.com -- Justin
On Thu, Jan 27, 2022 at 12:45 PM Justin Pryzby <pryzby@telsasoft.com> wrote: > If I'm not wrong, this is causing failures on cfbot. > https://cirrus-ci.com/task/6312181619752960?logs=build#L2056 > > [17:24:48.348] c:\cirrus\src\backend\replication\basebackup_gzip.c(90): warning C4715: 'bbsink_gzip_new': not all controlpaths return a value [c:\cirrus\postgres.vcxproj] > [17:24:48.348] c:\cirrus\src\bin\pg_basebackup\pg_basebackup.c(1922,67): error C2065: 'Z_DEFAULT_COMPRESSION': undeclaredidentifier [c:\cirrus\pg_basebackup.vcxproj] > > The warning seems to be what's fixed by David's patch here. > https://www.postgresql.org/message-id/CA%2BTgmobSeRFM84pwa5HfZCGRx%3DQ-iFfPFZfq-4%3Dt1p04yxg_Og%40mail.gmail.com I can commit that patch (though so could David) but I don't understand why you think it's connected to this commit particularly. -- Robert Haas EDB: http://www.enterprisedb.com
On Thu, Jan 27, 2022 at 02:19:30PM -0500, Robert Haas wrote: > On Thu, Jan 27, 2022 at 12:45 PM Justin Pryzby <pryzby@telsasoft.com> wrote: > > If I'm not wrong, this is causing failures on cfbot. > > https://cirrus-ci.com/task/6312181619752960?logs=build#L2056 > > > > [17:24:48.348] c:\cirrus\src\backend\replication\basebackup_gzip.c(90): warning C4715: 'bbsink_gzip_new': not all controlpaths return a value [c:\cirrus\postgres.vcxproj] > > [17:24:48.348] c:\cirrus\src\bin\pg_basebackup\pg_basebackup.c(1922,67): error C2065: 'Z_DEFAULT_COMPRESSION': undeclaredidentifier [c:\cirrus\pg_basebackup.vcxproj] > > > > The warning seems to be what's fixed by David's patch here. > > https://www.postgresql.org/message-id/CA%2BTgmobSeRFM84pwa5HfZCGRx%3DQ-iFfPFZfq-4%3Dt1p04yxg_Og%40mail.gmail.com > > I can commit that patch (though so could David) but I don't understand > why you think it's connected to this commit particularly. The C2065: is connected to this commit, right ? I included the warning since it's 1 line away and evidences that the warning is easily visible , and not due to some obscure combination of windows settings. -- Justin
On Thu, Jan 27, 2022 at 2:22 PM Justin Pryzby <pryzby@telsasoft.com> wrote: > On Thu, Jan 27, 2022 at 02:19:30PM -0500, Robert Haas wrote: > > On Thu, Jan 27, 2022 at 12:45 PM Justin Pryzby <pryzby@telsasoft.com> wrote: > > > If I'm not wrong, this is causing failures on cfbot. > > > https://cirrus-ci.com/task/6312181619752960?logs=build#L2056 > > > > > > [17:24:48.348] c:\cirrus\src\backend\replication\basebackup_gzip.c(90): warning C4715: 'bbsink_gzip_new': not all controlpaths return a value [c:\cirrus\postgres.vcxproj] > > > [17:24:48.348] c:\cirrus\src\bin\pg_basebackup\pg_basebackup.c(1922,67): error C2065: 'Z_DEFAULT_COMPRESSION': undeclaredidentifier [c:\cirrus\pg_basebackup.vcxproj] > > > > > > The warning seems to be what's fixed by David's patch here. > > > https://www.postgresql.org/message-id/CA%2BTgmobSeRFM84pwa5HfZCGRx%3DQ-iFfPFZfq-4%3Dt1p04yxg_Og%40mail.gmail.com > > > > I can commit that patch (though so could David) but I don't understand > > why you think it's connected to this commit particularly. > > The C2065: is connected to this commit, right ? > > I included the warning since it's 1 line away and evidences that the warning is > easily visible , and not due to some obscure combination of windows settings. Oh ... I see, I misread your email. Yeah, this commit introduced Z_DEFAULT_COMPRESSION outside of a HAVE_LIBZ block. Whoops. -- Robert Haas EDB: http://www.enterprisedb.com