Thread: pgsql: Allow extensions to add new backup targets.
Allow extensions to add new backup targets. Commit 3500ccc39b0dadd1068a03938e4b8ff562587ccc allowed for base backup targets, meaning that we could do something with the backup other than send it to the client, but all of those targets had to be baked in to the core code. This commit makes it possible for extensions to define additional backup targets. Patch by me, reviewed by Abhijit Menon-Sen. Discussion: http://postgr.es/m/CA+TgmoaqvdT-u3nt+_kkZ7bgDAyqDB0i-+XOMmr5JN2Rd37hxw@mail.gmail.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/e4ba69f3f4a1b997aa493cc02e563a91c0f35b87 Modified Files -------------- src/backend/replication/Makefile | 1 + src/backend/replication/Makefile.orig | 49 ++++++ src/backend/replication/basebackup.c | 82 ++++------ src/backend/replication/basebackup_target.c | 238 ++++++++++++++++++++++++++++ src/include/replication/basebackup_target.h | 66 ++++++++ 5 files changed, 381 insertions(+), 55 deletions(-)
Hi Robert,
On Tue, Mar 15, 2022 at 05:33:12PM +0000, Robert Haas wrote:
> Allow extensions to add new backup targets.
>
> Commit 3500ccc39b0dadd1068a03938e4b8ff562587ccc allowed for base backup
> targets, meaning that we could do something with the backup other than
> send it to the client, but all of those targets had to be baked in to
> the core code. This commit makes it possible for extensions to define
> additional backup targets.
I have noticed that this commit produces a warning when building with
MSVC, as of the end of BaseBackupGetTargetHandle() when the target
cannot be found. I guess that you'd better add a fake return NULL to
keep such compilers quiet about that, like that:
+++ b/src/backend/replication/basebackup_target.c
@@ -144,6 +144,9 @@ BaseBackupGetTargetHandle(char *target, char *target_detail)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("unrecognized target: \"%s\"", target)));
+
+ /* keep compiler quiet */
+ return NULL;
}
Thanks,
--
Michael
Attachment
On Wed, Mar 16, 2022 at 5:36 AM Michael Paquier <michael@paquier.xyz> wrote:
> I have noticed that this commit produces a warning when building with
> MSVC, as of the end of BaseBackupGetTargetHandle() when the target
> cannot be found. I guess that you'd better add a fake return NULL to
> keep such compilers quiet about that, like that:
> +++ b/src/backend/replication/basebackup_target.c
> @@ -144,6 +144,9 @@ BaseBackupGetTargetHandle(char *target, char *target_detail)
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("unrecognized target: \"%s\"", target)));
> +
> + /* keep compiler quiet */
> + return NULL;
> }
Done.
It sucks that I keep missing this point. And it also sucks that we
have to have this kind of stuff. :-(
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, Mar 16, 2022 at 09:52:41AM -0400, Robert Haas wrote:
> On Wed, Mar 16, 2022 at 5:36 AM Michael Paquier <michael@paquier.xyz> wrote:
> > I have noticed that this commit produces a warning when building with
> > MSVC, as of the end of BaseBackupGetTargetHandle() when the target
> > cannot be found. I guess that you'd better add a fake return NULL to
> > keep such compilers quiet about that, like that:
> > +++ b/src/backend/replication/basebackup_target.c
> > @@ -144,6 +144,9 @@ BaseBackupGetTargetHandle(char *target, char *target_detail)
> > ereport(ERROR,
> > (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > errmsg("unrecognized target: \"%s\"", target)));
> > +
> > + /* keep compiler quiet */
> > + return NULL;
> > }
>
> Done.
>
> It sucks that I keep missing this point. And it also sucks that we
> have to have this kind of stuff. :-(
FYI - at some point, CI will catch these in advance, per Andres' suggestion on
the big "basebackup" thread.
https://github.com/justinpryzby/postgres/commit/451e522a26f2b9a62976834ef7848279514ca700
--
Justin
On Wed, Mar 16, 2022 at 09:52:41AM -0400, Robert Haas wrote: > It sucks that I keep missing this point. So do I. No worries. Thanks for fixing it! -- Michael
Attachment
On Tue, Mar 15, 2022 at 7:33 PM Robert Haas <rhaas@postgresql.org> wrote: > > Allow extensions to add new backup targets. > > Commit 3500ccc39b0dadd1068a03938e4b8ff562587ccc allowed for base backup > targets, meaning that we could do something with the backup other than > send it to the client, but all of those targets had to be baked in to > the core code. This commit makes it possible for extensions to define > additional backup targets. > > Patch by me, reviewed by Abhijit Menon-Sen. > > Discussion: http://postgr.es/m/CA+TgmoaqvdT-u3nt+_kkZ7bgDAyqDB0i-+XOMmr5JN2Rd37hxw@mail.gmail.com > > Branch > ------ > master > > Details > ------- > https://git.postgresql.org/pg/commitdiff/e4ba69f3f4a1b997aa493cc02e563a91c0f35b87 > > Modified Files > -------------- > src/backend/replication/Makefile | 1 + > src/backend/replication/Makefile.orig | 49 ++++++ > src/backend/replication/basebackup.c | 82 ++++------ > src/backend/replication/basebackup_target.c | 238 ++++++++++++++++++++++++++++ > src/include/replication/basebackup_target.h | 66 ++++++++ > 5 files changed, 381 insertions(+), 55 deletions(-) This one has a tiny copy/paste error where the idenfiication comment for basebackup_target.c claims it's basebackup_gzip.c. I've pushed a fix. -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
On Mon, Mar 21, 2022 at 6:38 AM Magnus Hagander <magnus@hagander.net> wrote: > This one has a tiny copy/paste error where the idenfiication comment > for basebackup_target.c claims it's basebackup_gzip.c. I've pushed a > fix. Thanks. If that's the extent of the problems we're doing well! -- Robert Haas EDB: http://www.enterprisedb.com