| From: | Mahendra Singh Thalor <mahi6run(at)gmail(dot)com> |
|---|---|
| To: | Nitin Motiani <nitinmotiani(at)google(dot)com> |
| Cc: | Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Hannu Krosing <hannuk(at)google(dot)com>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Adding pg_dump flag for parallel export to pipes |
| Date: | 2026-03-14 21:37:33 |
| Message-ID: | CAKYtNApb_aL=psF+=VwjppTd717F4dGk_MVG9XW06OOW8LHRUQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 4 Mar 2026 at 15:27, Nitin Motiani <nitinmotiani(at)google(dot)com> wrote:
>
> Rebased and added some extra logs for testing.
>
> Thanks
Thanks Nitin for the updated patch.
+1 for this idea.
Here are my few review comments.
*Comment1*:
02 is not applying so this should be rebased.
fix:
if (!no_globals)
n_errors = restore_global_objects(global_path,
tmpopts, filespec_is_pipe);
*Comment2*:
03 patch has test cases. Please change these error messages as per other
errors in code.
+command_fails_like(
+ [ 'pg_dump', '-Fd', '--pipe-command="cat"', '--compress=lz4',
'test'],
+ qr/\Qpg_dump: hint: Option --pipe-command is not supported with any
compression type\E/,
+ 'pg_dump: hint: Option --pipe-command is not supported with any
compression type'
+);
Above should be:
+command_fails_like(
+ [ 'pg_dump', '-Fd', '--pipe-command="cat"', '--compress=lz4',
'test'],
+ qr/\Qpg_dump: error: option --pipe-command is not supported with
any compression type\E/,
+ 'pg_dump option --pipe-command is not supported with any
compression type'
+);
*Comment3*:
Please can we rename "--pipe-command" to "--pipe" only.
*Commen4*: --pipe or --pipe-command should be added into the usage function
in pg_dump.c and pg_restore.c files.
*Comment5*: I think we can support this new pipe option with pg_dumpall
also as we support directory mode in pg_dumpall from v19.
*Comment6:*
> mst(at)localhost:~/pg60/postgres/inst/bin$ ./pg_dump -Fd --pipe-command="a"
> -d postgres
> sh: line 1: a: command not found
> pg_dump: error: could not close file: Success
> pg_dump: error: could not close TOC file: Success
> mst(at)localhost:~/pg60/postgres/inst/bin$
we should add more processing for the "--pipe-command" argument so that we
don't get the above error. I think we should validate the path.
*Comment7*: + bool path_is_pipe_command)
I think we can rename to is_pipe, similarly filespec_is_pipe ->
is_pipe, fSpecIsPipe->is_pipe
*Comment8*:
> + This option is only supported with the directory output
> + format. It can be used to write to multiple streams which
> + otherwise would not be possible with the directory mode.
> + For each stream, it starts a process which runs the
> + specified command and pipes the pg_dump output to this
> + process.
> + This option is not valid if <option>--file</option>
> + is also specified.
> + </para>
> + <para>
> + The pipe-command can be used to perform operations like compress
> + using a custom algorithm, filter, or write the output to a cloud
> + storage etc. The user would need a way to pipe the final output of
> + each stream to a file. To handle that, the pipe command supports
> a format
> + specifier %f. And all the instances of %f in the command string
> + will be replaced with the corresponding file name which
> + would have been used in the directory mode with
> <option>--file</option>.
> + See <xref linkend="pg-dump-examples"/> below.
> + </para>
> + </listitem>
> + </varlistentry>
>
please use 80 chars in all lines.
*Comment9*:
mst(at)localhost:~/pg60/postgres/inst/bin$ ./pg_restore dumpdir
--pipe-command="cat"
pg_restore: hint: Only one of [filespec, --pipe-command] allowed
Above error should be similar to the other errors in pg_restore.
*Comment10*:
mst(at)localhost:~/pg60/postgres/inst/bin$ ./pg_restore --pipe-command="cat"
-d postgres --format=tar
pg_restore: error: could not open TOC file "cat" for input: No such file or
directory
mst(at)localhost:~/pg60/postgres/inst/bin$
we should add an error for non-directory format. If no format is specified,
then we can continue or for unknown format also we can report an error.
I will do some more review and testing.
--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-03-14 21:49:13 | Proposal: common explicit lists for installed headers |
| Previous Message | Corey Huinker | 2026-03-14 21:13:19 | Re: Add starelid, attnum to pg_stats and leverage this in pg_dump |