On 12/8/19 10:25 AM, Mark Dilger wrote:
I was
still expecting multiple patches, perhaps named along the
lines of:
unshadow.RedoRecPtr.patch.1
unshadow.wal_segment_size.patch.1
unshadow.synchronous_commit.patch.1
unshadow.wrconn.patch.1
unshadow.progname.patch.1
unshadow.am_syslogger.patch.1
unshadow.days.patch.1
unshadow.months.patch.1
etc. I'm uncomfortable giving you negative feedback of this
sort, since I think you are working hard to improve postgres
and I really appreciate it, so later tonight I'll try to come
back, split your patch for you as described, add an entry to
the commitfest if you haven't already, and mark myself as a
reviewer.
To start off, I've taken just six of the 22 or so variables
that you renamed and created patches for them. I'm not
endorsing these in any way. I chose these mostly based on
which ones showed up first in your patch file, with one
exception.
I stopped when I got to 'progname' => 'prog_name' as the
whole exercise was getting too absurd even for me. That
clearly looks like one where the structure of the code
needs to be reconsidered, rather than just renaming stuff.
I'll create the commitfest entry based on this email once
this has been sent.
Patches attached.
--
Mark Dilger
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index e4e9ce7398..ae6f6261f5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7158,11 +7158,11 @@ StartupXLOG(void)
if (info == XLOG_CHECKPOINT_SHUTDOWN)
{
- CheckPoint checkPoint;
+ CheckPoint xcheckPoint;
- memcpy(&checkPoint,
XLogRecGetData(xlogreader), sizeof(CheckPoint));
- newTLI =
checkPoint.ThisTimeLineID;
- prevTLI =
checkPoint.PrevTimeLineID;
+ memcpy(&xcheckPoint,
XLogRecGetData(xlogreader), sizeof(CheckPoint));
+ newTLI =
xcheckPoint.ThisTimeLineID;
+ prevTLI =
xcheckPoint.PrevTimeLineID;
}
else if (info == XLOG_END_OF_RECOVERY)
{
diff --git a/src/interfaces/ecpg/preproc/descriptor.c
b/src/interfaces/ecpg/preproc/descriptor.c
index a29f530327..da3291baca 100644
--- a/src/interfaces/ecpg/preproc/descriptor.c
+++ b/src/interfaces/ecpg/preproc/descriptor.c
@@ -73,7 +73,7 @@ ECPGnumeric_lvalue(char *name)
static struct descriptor *descriptors;
void
-add_descriptor(char *name, char *connection)
+add_descriptor(char *name, char *conn_str)
{
struct descriptor *new;
@@ -85,18 +85,18 @@ add_descriptor(char *name, char *connection)
new->next = descriptors;
new->name = mm_alloc(strlen(name) + 1);
strcpy(new->name, name);
- if (connection)
+ if (conn_str)
{
- new->connection = mm_alloc(strlen(connection) + 1);
- strcpy(new->connection, connection);
+ new->connection = mm_alloc(strlen(conn_str) + 1);
+ strcpy(new->connection, conn_str);
}
else
- new->connection = connection;
+ new->connection = conn_str;
descriptors = new;
}
void
-drop_descriptor(char *name, char *connection)
+drop_descriptor(char *name, char *conn_str)
{
struct descriptor *i;
struct descriptor **lastptr = &descriptors;
@@ -108,9 +108,9 @@ drop_descriptor(char *name, char *connection)
{
if (strcmp(name, i->name) == 0)
{
- if ((!connection && !i->connection)
- || (connection && i->connection
- && strcmp(connection, i->connection) ==
0))
+ if ((!conn_str && !i->connection)
+ || (conn_str && i->connection
+ && strcmp(conn_str, i->connection) ==
0))
{
*lastptr = i->next;
if (i->connection)
@@ -126,7 +126,7 @@ drop_descriptor(char *name, char *connection)
struct descriptor
*
-lookup_descriptor(char *name, char *connection)
+lookup_descriptor(char *name, char *conn_str)
{
struct descriptor *i;
@@ -137,9 +137,9 @@ lookup_descriptor(char *name, char *connection)
{
if (strcmp(name, i->name) == 0)
{
- if ((!connection && !i->connection)
- || (connection && i->connection
- && strcmp(connection, i->connection) ==
0))
+ if ((!conn_str && !i->connection)
+ || (conn_str && i->connection
+ && strcmp(conn_str, i->connection) ==
0))
return i;
}
}
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 6bc1a6b46d..e4e9ce7398 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -892,8 +892,8 @@ static int emode_for_corrupt_record(int emode, XLogRecPtr
RecPtr);
static void XLogFileClose(void);
static void PreallocXlogFiles(XLogRecPtr endptr);
static void RemoveTempXlogFiles(void);
-static void RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr RedoRecPtr,
XLogRecPtr endptr);
-static void RemoveXlogFile(const char *segname, XLogRecPtr RedoRecPtr,
XLogRecPtr endptr);
+static void RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr pRedoRecPtr,
XLogRecPtr endptr);
+static void RemoveXlogFile(const char *segname, XLogRecPtr pRedoRecPtr,
XLogRecPtr endptr);
static void UpdateLastRemovedPtr(char *filename);
static void ValidateXLOGDirectoryStructure(void);
static void CleanupBackupHistory(void);
@@ -2299,7 +2299,7 @@ assign_checkpoint_completion_target(double newval, void
*extra)
* XLOG segments? Returns the highest segment that should be preallocated.
*/
static XLogSegNo
-XLOGfileslop(XLogRecPtr RedoRecPtr)
+XLOGfileslop(XLogRecPtr pRedoRecPtr)
{
XLogSegNo minSegNo;
XLogSegNo maxSegNo;
@@ -2311,9 +2311,9 @@ XLOGfileslop(XLogRecPtr RedoRecPtr)
* correspond to. Always recycle enough segments to meet the minimum,
and
* remove enough segments to stay below the maximum.
*/
- minSegNo = RedoRecPtr / wal_segment_size +
+ minSegNo = pRedoRecPtr / wal_segment_size +
ConvertToXSegs(min_wal_size_mb, wal_segment_size) - 1;
- maxSegNo = RedoRecPtr / wal_segment_size +
+ maxSegNo = pRedoRecPtr / wal_segment_size +
ConvertToXSegs(max_wal_size_mb, wal_segment_size) - 1;
/*
@@ -2328,7 +2328,7 @@ XLOGfileslop(XLogRecPtr RedoRecPtr)
/* add 10% for good measure. */
distance *= 1.10;
- recycleSegNo = (XLogSegNo) ceil(((double) RedoRecPtr + distance) /
+ recycleSegNo = (XLogSegNo) ceil(((double) pRedoRecPtr + distance) /
wal_segment_size);
if (recycleSegNo < minSegNo)
@@ -3954,7 +3954,7 @@ RemoveTempXlogFiles(void)
* whether we want to recycle rather than delete no-longer-wanted log files.
*/
static void
-RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
+RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr pRedoRecPtr, XLogRecPtr endptr)
{
DIR *xldir;
struct dirent *xlde;
@@ -3997,7 +3997,7 @@ RemoveOldXlogFiles(XLogSegNo segno, XLogRecPtr
RedoRecPtr, XLogRecPtr endptr)
/* Update the last removed location in shared
memory first */
UpdateLastRemovedPtr(xlde->d_name);
- RemoveXlogFile(xlde->d_name, RedoRecPtr,
endptr);
+ RemoveXlogFile(xlde->d_name, pRedoRecPtr,
endptr);
}
}
}
@@ -4078,7 +4078,7 @@ RemoveNonParentXlogFiles(XLogRecPtr switchpoint,
TimeLineID newTLI)
* somewhat arbitrarily, 10 future segments.
*/
static void
-RemoveXlogFile(const char *segname, XLogRecPtr RedoRecPtr, XLogRecPtr endptr)
+RemoveXlogFile(const char *segname, XLogRecPtr pRedoRecPtr, XLogRecPtr endptr)
{
char path[MAXPGPATH];
#ifdef WIN32
@@ -4094,10 +4094,10 @@ RemoveXlogFile(const char *segname, XLogRecPtr
RedoRecPtr, XLogRecPtr endptr)
* Initialize info about where to try to recycle to.
*/
XLByteToSeg(endptr, endlogSegNo, wal_segment_size);
- if (RedoRecPtr == InvalidXLogRecPtr)
+ if (pRedoRecPtr == InvalidXLogRecPtr)
recycleSegNo = endlogSegNo + 10;
else
- recycleSegNo = XLOGfileslop(RedoRecPtr);
+ recycleSegNo = XLOGfileslop(pRedoRecPtr);
}
else
recycleSegNo = 0; /* keep compiler quiet */
diff --git a/src/backend/commands/subscriptioncmds.c
b/src/backend/commands/subscriptioncmds.c
index 5408edcfc2..681d208f81 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -57,7 +57,7 @@ static void
parse_subscription_options(List *options, bool *connect, bool *enabled_given,
bool *enabled, bool
*create_slot,
bool *slot_name_given, char
**slot_name,
- bool *copy_data, char
**synchronous_commit,
+ bool *copy_data, char
**synchr_commit,
bool *refresh)
{
ListCell *lc;
@@ -85,8 +85,8 @@ parse_subscription_options(List *options, bool *connect, bool
*enabled_given,
}
if (copy_data)
*copy_data = true;
- if (synchronous_commit)
- *synchronous_commit = NULL;
+ if (synchr_commit)
+ *synchr_commit = NULL;
if (refresh)
*refresh = true;
@@ -150,17 +150,17 @@ parse_subscription_options(List *options, bool *connect,
bool *enabled_given,
*copy_data = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "synchronous_commit") == 0 &&
- synchronous_commit)
+ synchr_commit)
{
- if (*synchronous_commit)
+ if (*synchr_commit)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting or
redundant options")));
- *synchronous_commit = defGetString(defel);
+ *synchr_commit = defGetString(defel);
/* Test if the given value is valid for
synchronous_commit GUC. */
- (void) set_config_option("synchronous_commit",
*synchronous_commit,
+ (void) set_config_option("synchronous_commit",
*synchr_commit,
PGC_BACKEND, PGC_S_TEST, GUC_ACTION_SET,
false,
0, false);
}
@@ -317,7 +317,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
bool enabled_given;
bool enabled;
bool copy_data;
- char *synchronous_commit;
+ char *synchr_commit;
char *conninfo;
char *slotname;
bool slotname_given;
@@ -332,7 +332,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
*/
parse_subscription_options(stmt->options, &connect, &enabled_given,
&enabled,
&create_slot, &slotname_given,
- &slotname,
©_data, &synchronous_commit,
+ &slotname,
©_data, &synchr_commit,
NULL);
/*
@@ -375,8 +375,8 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
slotname = stmt->subname;
/* The default for synchronous_commit of subscriptions is off. */
- if (synchronous_commit == NULL)
- synchronous_commit = "off";
+ if (synchr_commit == NULL)
+ synchr_commit = "off";
conninfo = stmt->conninfo;
publications = stmt->publication;
@@ -407,7 +407,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
else
nulls[Anum_pg_subscription_subslotname - 1] = true;
values[Anum_pg_subscription_subsynccommit - 1] =
- CStringGetTextDatum(synchronous_commit);
+ CStringGetTextDatum(synchr_commit);
values[Anum_pg_subscription_subpublications - 1] =
publicationListToArray(publications);
@@ -668,11 +668,11 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
{
char *slotname;
bool slotname_given;
- char *synchronous_commit;
+ char *synchr_commit;
parse_subscription_options(stmt->options, NULL,
NULL, NULL,
NULL, &slotname_given, &slotname,
-
NULL, &synchronous_commit, NULL);
+
NULL, &synchr_commit, NULL);
if (slotname_given)
{
@@ -690,10 +690,10 @@ AlterSubscription(AlterSubscriptionStmt *stmt)
replaces[Anum_pg_subscription_subslotname - 1] = true;
}
- if (synchronous_commit)
+ if (synchr_commit)
{
values[Anum_pg_subscription_subsynccommit - 1] =
-
CStringGetTextDatum(synchronous_commit);
+
CStringGetTextDatum(synchr_commit);
replaces[Anum_pg_subscription_subsynccommit - 1] = true;
}
diff --git a/src/backend/access/transam/xlogreader.c
b/src/backend/access/transam/xlogreader.c
index 67418b05f1..bc8156fcd6 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -70,7 +70,7 @@ report_invalid_record(XLogReaderState *state, const char
*fmt,...)
* Returns NULL if the xlogreader couldn't be allocated.
*/
XLogReaderState *
-XLogReaderAllocate(int wal_segment_size, const char *waldir,
+XLogReaderAllocate(int wallog_segment_size, const char *waldir,
XLogPageReadCB pagereadfunc, void
*private_data)
{
XLogReaderState *state;
@@ -99,7 +99,7 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir,
}
/* Initialize segment info. */
- WALOpenSegmentInit(&state->seg, &state->segcxt, wal_segment_size,
+ WALOpenSegmentInit(&state->seg, &state->segcxt, wallog_segment_size,
waldir);
state->read_page = pagereadfunc;
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 0193611b7f..d8991d1a79 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -209,7 +209,7 @@ struct XLogReaderState
};
/* Get a new XLogReader */
-extern XLogReaderState *XLogReaderAllocate(int wal_segment_size,
+extern XLogReaderState *XLogReaderAllocate(int wallog_segment_size,
const char *waldir,
XLogPageReadCB pagereadfunc,
void *private_data);
diff --git a/src/backend/commands/subscriptioncmds.c
b/src/backend/commands/subscriptioncmds.c
index 681d208f81..61ed59d4f1 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -44,7 +44,7 @@
#include "utils/memutils.h"
#include "utils/syscache.h"
-static List *fetch_table_list(WalReceiverConn *wrconn, List *publications);
+static List *fetch_table_list(WalReceiverConn *walrconn, List *publications);
/*
* Common option parsing function for CREATE and ALTER SUBSCRIPTION commands.
@@ -430,14 +430,14 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
{
XLogRecPtr lsn;
char *err;
- WalReceiverConn *wrconn;
+ WalReceiverConn *walrconn;
List *tables;
ListCell *lc;
char table_state;
/* Try to connect to the publisher. */
- wrconn = walrcv_connect(conninfo, true, stmt->subname, &err);
- if (!wrconn)
+ walrconn = walrcv_connect(conninfo, true, stmt->subname, &err);
+ if (!walrconn)
ereport(ERROR,
(errmsg("could not connect to the
publisher: %s", err)));
@@ -453,7 +453,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
* Get the table list from publisher and build local
table status
* info.
*/
- tables = fetch_table_list(wrconn, publications);
+ tables = fetch_table_list(walrconn, publications);
foreach(lc, tables)
{
RangeVar *rv = (RangeVar *) lfirst(lc);
@@ -478,7 +478,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
{
Assert(slotname);
- walrcv_create_slot(wrconn, slotname, false,
+ walrcv_create_slot(walrconn, slotname, false,
CRS_NOEXPORT_SNAPSHOT, &lsn);
ereport(NOTICE,
(errmsg("created replication
slot \"%s\" on publisher",
@@ -487,7 +487,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
}
PG_FINALLY();
{
- walrcv_disconnect(wrconn);
+ walrcv_disconnect(walrconn);
}
PG_END_TRY();
}
@@ -835,7 +835,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool
isTopLevel)
char originname[NAMEDATALEN];
char *err = NULL;
RepOriginId originid;
- WalReceiverConn *wrconn = NULL;
+ WalReceiverConn *walrconn = NULL;
StringInfoData cmd;
Form_pg_subscription form;
@@ -982,8 +982,8 @@ DropSubscription(DropSubscriptionStmt *stmt, bool
isTopLevel)
initStringInfo(&cmd);
appendStringInfo(&cmd, "DROP_REPLICATION_SLOT %s WAIT",
quote_identifier(slotname));
- wrconn = walrcv_connect(conninfo, true, subname, &err);
- if (wrconn == NULL)
+ walrconn = walrcv_connect(conninfo, true, subname, &err);
+ if (walrconn == NULL)
ereport(ERROR,
(errmsg("could not connect to publisher when
attempting to "
"drop the replication slot
\"%s\"", slotname),
@@ -996,7 +996,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool
isTopLevel)
{
WalRcvExecResult *res;
- res = walrcv_exec(wrconn, cmd.data, 0, NULL);
+ res = walrcv_exec(walrconn, cmd.data, 0, NULL);
if (res->status != WALRCV_OK_COMMAND)
ereport(ERROR,
@@ -1012,7 +1012,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool
isTopLevel)
}
PG_FINALLY();
{
- walrcv_disconnect(wrconn);
+ walrcv_disconnect(walrconn);
}
PG_END_TRY();
@@ -1124,7 +1124,7 @@ AlterSubscriptionOwner_oid(Oid subid, Oid newOwnerId)
* publisher connection.
*/
static List *
-fetch_table_list(WalReceiverConn *wrconn, List *publications)
+fetch_table_list(WalReceiverConn *walrconn, List *publications)
{
WalRcvExecResult *res;
StringInfoData cmd;
@@ -1154,7 +1154,7 @@ fetch_table_list(WalReceiverConn *wrconn, List
*publications)
}
appendStringInfoChar(&cmd, ')');
- res = walrcv_exec(wrconn, cmd.data, 2, tableRow);
+ res = walrcv_exec(walrconn, cmd.data, 2, tableRow);
pfree(cmd.data);
if (res->status != WALRCV_OK_TUPLES)
