On Wed, 4 Mar 2026 at 15:27, Nitin Motiani <[email protected]> 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@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@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@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@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@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
