From: | Michael Paquier <michael(at)paquier(dot)xyz> |
---|---|
To: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
Cc: | Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Liudmila Mantrova <l(dot)mantrova(at)postgrespro(dot)ru>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, David Steele <david(at)pgmasters(dot)net>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, vladimirlesk(at)yandex-team(dot)ru, dsarafan(at)yandex-team(dot)ru |
Subject: | Re: [Patch] pg_rewind: options to use restore_command from recovery.conf or command line |
Date: | 2020-02-27 01:52:38 |
Message-ID: | 20200227015238.GA1969@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Feb 27, 2020 at 12:43:55AM +0300, Alexander Korotkov wrote:
> Regarding text split change, it was made by pgindent. I didn't notice
> it belongs to unchanged part of code. Sure, we shouldn't include this
> into the patch.
I have read through v17 (not tested, sorry), and spotted a couple of
issues that need to be addressed.
+ "--source-pgdata=$standby_pgdata",
+ "--target-pgdata=$master_pgdata",
+ "--no-sync", "--no-ensure-shutdown",
FWIW, I think that perl indenting would reshape this part. I would
recommend to run src/tools/pgindent/pgperltidy and
./src/tools/perlcheck/pgperlcritic before commit.
+ * Copyright (c) 2020, PostgreSQL Global Development Group
Wouldn't it be better to just use the full copyright here? I mean the
following:
Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
Portions Copyright (c) 1994, The Regents of the University of California
+++ b/src/common/archive.c
[...]
+#include "postgres.h"
+
+#include "common/archive.h"
This is incorrect. All files shared between the backend and the
frontend in src/common/ have to include the following set of headers:
#ifndef FRONTEND
#include "postgres.h"
#else
#include "postgres_fe.h"
#endif
+++ b/src/common/fe_archive.c
[...]
+#include "postgres_fe.h"
This is incomplete. The following piece should be added:
#ifndef FRONTEND
#error "This file is not expected to be compiled for backend code"
#endif
+ snprintf(postgres_cmd, sizeof(postgres_cmd), "%s -D %s
-C restore_command",
+ postgres_exec_path, datadir_target);
+
I think that this is missing proper quoting.
I would rename ConstructRestoreCommand() to BuildRestoreCommand()
while on it..
I think that it would be saner to check the return status of
ConstructRestoreCommand() in xlogarchive.c as a sanity check, with an
elog(ERROR) if not 0, as that should never happen.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2020-02-27 02:24:33 | Re: Quoting issues with createdb |
Previous Message | Yugo NAGATA | 2020-02-27 01:18:16 | Re: Allow auto_explain to log plans before queries are executed |