some additional (small) problems with pg_combinebackup and tablespaces
Tomas Vondra pointed out to me a couple of mistakes that I made with
regard to pg_combinebackup and tablespaces.
One is that I screwed up the long_options array by specifying
tablespace-mapping as no_argument rather than required_argument. That
doesn't break the tests I just committed because, in the actual string
passed to getopt_long(), I wrote "T:", which means the short form of
the option works; only the long form does not.
The other is that the documentation says that --tablespace-mapping is
applied to the pathnames from the first backup specified on the
command line. It should have said "final" rather than "first".
Here is a very small patch correcting these regrettable errors.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v1-0001-Fix-small-mistakes-around-pg_combinebackup-with-t.patchapplication/octet-stream; name=v1-0001-Fix-small-mistakes-around-pg_combinebackup-with-t.patchDownload+2-3
On 24 Apr 2024, at 19:59, Robert Haas <robertmhaas@gmail.com> wrote:
Here is a very small patch correcting these regrettable errors.
Patch LGTM.
In addition to those, unless I'm reading it wrong the current coding seems to
include a "-P" short option which is missing in the command parsing switch
statement (or in the help output):
while ((c = getopt_long(argc, argv, "dnNPo:T:",
Should that be removed?
A tiny nit-pick in the same area: while not wrong, it reads a bit odd to have
"don't" and "do not" on lines directly next to each other:
printf(_(" -n, --dry-run don't actually do anything\n"));
printf(_(" -N, --no-sync do not wait for changes to be written safely to disk\n"));
--
Daniel Gustafsson
On Wed, Apr 24, 2024 at 3:20 PM Daniel Gustafsson <daniel@yesql.se> wrote:
Patch LGTM.
Thanks. Here's an updated version with fixes for the other issues you mentioned.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v2-0001-Minor-fixes-to-pg_combinebackup-and-its-documenta.patchapplication/octet-stream; name=v2-0001-Minor-fixes-to-pg_combinebackup-and-its-documenta.patchDownload+4-5
On 25 Apr 2024, at 17:48, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Apr 24, 2024 at 3:20 PM Daniel Gustafsson <daniel@yesql.se> wrote:
Patch LGTM.
Thanks. Here's an updated version with fixes for the other issues you mentioned.
LGTM, only one small error in the commitmessage: s/Gustaffson/Gustafsson/
--
Daniel Gustafsson
On Thu, Apr 25, 2024 at 2:03 PM Daniel Gustafsson <daniel@yesql.se> wrote:
LGTM, only one small error in the commitmessage: s/Gustaffson/Gustafsson/
Oh no! You're in danger of becoming the second person on this project
whose name I chronically misspell. Fixed (I hope) and committed.
--
Robert Haas
EDB: http://www.enterprisedb.com