Hi, Daniel On Wed, 03 Jun 2026 at 10:14, Daniel Gustafsson <[email protected]> wrote: >> On 3 Jun 2026, at 09:58, Japin Li <[email protected]> wrote: >> >> >> Hi, >> >> I noticed that in pg_backup_archiver.c, RestoreArchive() writes the dump >> header >> using a hard-coded string: >> >> ahprintf(AH, "--\n-- PostgreSQL database dump\n--\n\n"); >> >> However, the macro TEXT_DUMP_HEADER (defined in the same file) already >> contains >> exactly the same content. To keep the code consistent and maintainable, this >> patch replaces the hard-coded string with the macro. > > Seems reasonable. >
Thanks for your review. >> A hard-coded version of TEXT_DUMPALL_HEADER exists in pg_dumpall.c, but since >> it spans multiple files, it is left untouched. > > The alternative would be to move the definitions to pg_backup_archiver.h and > use them consistently. > Fixed as you suggested. > -- > Daniel Gustafsson -- Regards, Japin Li ChengDu WenWu Information Technology Co., Ltd.
>From ccbe0dbbf62b68169546db7daa0a5c5c035ae820 Mon Sep 17 00:00:00 2001 From: Japin Li <[email protected]> Date: Wed, 3 Jun 2026 15:18:20 +0800 Subject: [PATCH v2] Replace the hard-coded dump headers with macros This commit repleaces the hard-coded dump headers with the existing macros to improve maintainability and consistency: - In pg_backup_archiver.c, use TEXT_DUMP_HEADER instead of a duplicate hard-coded string for database dump header. - Move TEXT_DUMPALL_HEADER from pg_backup_archiver.c to pg_backup_archiver.h so it can be shared across files. - In pg_dumpall.c, replace the hard-coded cluster dump header with the newly exposed TEXT_DUMPALL_HEADER macro. This avoids unnecessary duplication and ensures that any future changes to the header text only need to be made in one place. Author: Japin Li <[email protected]> Reviewed-by: Daniel Gustafsson <[email protected]> --- src/bin/pg_dump/pg_backup_archiver.c | 3 +-- src/bin/pg_dump/pg_backup_archiver.h | 2 ++ src/bin/pg_dump/pg_dumpall.c | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 2fd773ad84f..85d103464be 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -47,7 +47,6 @@ #include "pgtar.h" #define TEXT_DUMP_HEADER "--\n-- PostgreSQL database dump\n--\n\n" -#define TEXT_DUMPALL_HEADER "--\n-- PostgreSQL database cluster dump\n--\n\n" #define TOC_PREFIX_NONE "" #define TOC_PREFIX_DATA "Data for " @@ -466,7 +465,7 @@ RestoreArchive(Archive *AHX, bool append_data) if (ropt->filename || ropt->compression_spec.algorithm != PG_COMPRESSION_NONE) SetOutput(AH, ropt->filename, ropt->compression_spec, append_data); - ahprintf(AH, "--\n-- PostgreSQL database dump\n--\n\n"); + ahprintf(AH, TEXT_DUMP_HEADER); /* * If generating plain-text output, enter restricted mode to block any diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h index 1218bf6a6a1..21f226295a5 100644 --- a/src/bin/pg_dump/pg_backup_archiver.h +++ b/src/bin/pg_dump/pg_backup_archiver.h @@ -30,6 +30,8 @@ #include "pg_backup.h" #include "pqexpbuffer.h" +#define TEXT_DUMPALL_HEADER "--\n-- PostgreSQL database cluster dump\n--\n\n" + #define LOBBUFSIZE 16384 /* Data block types */ diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index c1f43113c53..49ae222211e 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -740,7 +740,7 @@ main(int argc, char *argv[]) } else { - fprintf(OPF, "--\n-- PostgreSQL database cluster dump\n--\n\n"); + fprintf(OPF, TEXT_DUMPALL_HEADER); if (verbose) dumpTimestamp("Started on"); -- 2.53.0
