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
signature.asc
Description: PGP signature
