MIME-Version: 1.0
References: <cb103623-8ee6-4ba5-a2c9-f32e3a4933fa@dunslane.net>
 <ZmcKMF61K4Ck51dK@nathan> <CABUevExoQ26jo+aQ9QZq+UMA1aD6gfpm9xBnh_t5e0DhaCeRYA@mail.gmail.com>
 <ZmcVqPOGjoccIh-i@nathan> <CABUevEyMEaaY37_eMW76F97TX2voxdtPNkm3FGqN9c3ZHy3GMg@mail.gmail.com>
 <740696.1718036463@sss.pgh.pa.us> <CABUevEwv4kQ5of9e+3mZ=Q9ZfkNtDbidPCLuDU_sm00d2Yf-rQ@mail.gmail.com>
 <CAKYtNAryL7Y-JEasiDCts+iiMRufNi74HpPvf=T4b9a1NSa_PQ@mail.gmail.com>
 <CAKYtNArH1=1gf=1t-OErx5G0y2P1nog1atQ4HMyxY2S5ocdnSA@mail.gmail.com>
 <Z3wUepoqtow7EeeR@nathan> <CAKYtNArQDOnTLCo6u8dU+32v-P5qe_naG+-hGrL=S2ZgMyS-_Q@mail.gmail.com>
 <CAKYtNAokugMDYMvqZLuhS7ghD55J9yeYoQzrSNRY9=bPz3Lbtw@mail.gmail.com>
 <CAKYtNAr+xdxtTdTTnjA8B+qkZMrjw6m9geMHcFFQH1itederoQ@mail.gmail.com>
 <CAECtzeXHvNNmVPwysJsCFoqfej4fx-bbzf+myjsM5Umj7kG_Pg@mail.gmail.com>
 <CAKYtNArMEsup-8QF9cBREwXjAOaqptGWF1eX1H=dRuTx3BWtwg@mail.gmail.com>
 <CAKYtNAp9vOtydXL3_pnGJ+TetZtN=FYSnZSMCqXceU3mkHPxPg@mail.gmail.com> <CACJufxGUucc2wscPFoZHHA9Ea4--wz-Lm86zm14s5TfTPZSqbw@mail.gmail.com>
In-Reply-To: <CACJufxGUucc2wscPFoZHHA9Ea4--wz-Lm86zm14s5TfTPZSqbw@mail.gmail.com>
From: Mahendra Singh Thalor <mahi6run@gmail.com>
Date: Sat, 11 Jan 2025 13:15:49 +0530
Message-ID: <CAKYtNAqDhqXF0RwF=nHcB=M+uPhxHn+6p+_w+bwAfmMzADN7yQ@mail.gmail.com>
Subject: Re: Non-text mode for pg_dumpall
To: jian he <jian.universality@gmail.com>
Cc: Guillaume Lelarge <guillaume@lelarge.info>, Nathan Bossart <nathandbossart@gmail.com>, 
	Magnus Hagander <magnus@hagander.net>, Tom Lane <tgl@sss.pgh.pa.us>, 
	Andrew Dunstan <andrew@dunslane.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>, 
	Dilip Kumar <dilipbalaut@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://www.postgresql.org/message-id/CAKYtNAqDhqXF0RwF%3DnHcB%3DM%2BuPhxHn%2B6p%2B_w%2BbwAfmMzADN7yQ%40mail.gmail.com>
Precedence: bulk

On Sat, 11 Jan 2025 at 11:19, jian he <jian.universality@gmail.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