Re: Non-text mode for pg_dumpall

From: Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Guillaume Lelarge <guillaume(at)lelarge(dot)info>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Subject: Re: Non-text mode for pg_dumpall
Date: 2025-01-11 07:45:49
Message-ID: CAKYtNAqDhqXF0RwF=nHcB=M+uPhxHn+6p+_w+bwAfmMzADN7yQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, 11 Jan 2025 at 11:19, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>

Thanks Jian for the review and testing.

> in src/bin/pg_dump/pg_dumpall.c main
> i think you need do
>
> archDumpFormat = parseDumpFormat(formatName);
> /*
> * Open the output file if required, otherwise use stdout. If required,
> * then create new files with global.dat and map.dat names.
> */
> if (archDumpFormat != archNull)
> {
> char toc_path[MAXPGPATH];
> /*
> * If directory/tar/custom format is specified then we must provide the
> * file name to create one main directory.
> */
> if (!filename || strcmp(filename, "") == 0)
> pg_fatal("no output directory specified");
> /* TODO: accept the empty existing directory. */
> if (mkdir(filename, 0700) < 0)
> pg_fatal("could not create directory \"%s\": %m",
> filename);
> snprintf(toc_path, MAXPGPATH, "%s/global.dat", filename);
> OPF = fopen(toc_path, "w");
> if (!OPF)
> pg_fatal("could not open global.dat file: %s", strerror(errno));
> }
> else if (filename)
> {
> OPF = fopen(filename, PG_BINARY_W);
> if (!OPF)
> pg_fatal("could not open output file \"%s\": %m",
> filename);
> }
> else
> OPF = stdout;
>
> before connectDatabase call.

Okay. I will add an error check before connectDatabase call in the next version.

>
> otherwise if the cluster is not setting up.
> ``pg_dumpall --format=d``
> error would be about connection error, not
> "pg_dumpall: error: no output directory specified"
>
> we want ``pg_dumpall --format`` invalid options
> to error out even if the cluster is not setting up.
>
> attached are two invalid option test cases.

Thanks.
I am also working on test cases. I will add all error test cases in
the next version and will include these two also.

>
> you also need change
> <varlistentry>
> <term><option>-f <replaceable
> class="parameter">filename</replaceable></option></term>
> <term><option>--file=<replaceable
> class="parameter">filename</replaceable></option></term>
> <listitem>
> <para>
> Send output to the specified file. If this is omitted, the
> standard output is used.
> </para>
> </listitem>
> </varlistentry>
> ?
>
> since if --format=d,
> <option>--file=<replaceable class="parameter">filename</replaceable></option>
> can not be omitted.

Okay. I will fix it.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2025-01-11 08:44:39 Re: Non-text mode for pg_dumpall
Previous Message jian he 2025-01-11 05:48:27 Re: Non-text mode for pg_dumpall