logical replication fixes

From da4dc2807566958dd89cc9f05bf6a83a996e99c7 Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler@timbira.com.br>
Date: Wed, 12 Apr 2017 21:45:34 -0300
Subject: [PATCH 1/3] Cosmetic and spelling fixes

---
 src/backend/catalog/pg_publication.c        |  2 +-
 src/backend/catalog/pg_subscription.c       |  4 ++--
 src/backend/replication/logical/message.c   |  2 +-
 src/backend/replication/logical/origin.c    | 18 +++++++++---------
 src/backend/replication/logical/proto.c     |  4 ++--
 src/backend/replication/logical/tablesync.c |  8 ++++----
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 9330e23..1987401 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -312,7 +312,7 @@ GetAllTablesPublicationRelations(void)
 /*
  * Get publication using oid
  *
- * The Publication struct and it's data are palloced here.
+ * The Publication struct and its data are palloc'ed here.
  */
 Publication *
 GetPublication(Oid pubid)
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index a183850..22587a4 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -403,7 +403,7 @@ RemoveSubscriptionRel(Oid subid, Oid relid)
 /*
  * Get all relations for subscription.
  *
- * Returned list is palloced in current memory context.
+ * Returned list is palloc'ed in current memory context.
  */
 List *
 GetSubscriptionRelations(Oid subid)
@@ -450,7 +450,7 @@ GetSubscriptionRelations(Oid subid)
 /*
  * Get all relations for subscription that are not in a ready state.
  *
- * Returned list is palloced in current memory context.
+ * Returned list is palloc'ed in current memory context.
  */
 List *
 GetSubscriptionNotReadyRelations(Oid subid)
diff --git a/src/backend/replication/logical/message.c b/src/backend/replication/logical/message.c
index 0dc3a9b..ef7d6c5 100644
--- a/src/backend/replication/logical/message.c
+++ b/src/backend/replication/logical/message.c
@@ -20,7 +20,7 @@
  * Non-transactional messages are sent to the plugin at the time when the
  * logical decoding reads them from XLOG. This also means that transactional
  * messages won't be delivered if the transaction was rolled back but the
- * non-transactional one will be delivered always.
+ * non-transactional one will always be delivered.
  *
  * Every message carries prefix to avoid conflicts between different decoding
  * plugins. The plugin authors must take extra care to use unique prefix,
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index 5eaf863..cf1a692 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -24,7 +24,7 @@
  * two bytes allow us to be more space efficient.
  *
  * Replication progress is tracked in a shared memory table
- * (ReplicationStates) that's dumped to disk every checkpoint. Entries
+ * (ReplicationState) that's dumped to disk every checkpoint. Entries
  * ('slots') in this table are identified by the internal id. That's the case
  * because it allows to increase replication progress during crash
  * recovery. To allow doing so we store the original LSN (from the originating
@@ -48,7 +48,7 @@
  *	 pg_replication_slot is required for the duration. That allows us to
  *	 safely and conflict free assign new origins using a dirty snapshot.
  *
- * * When creating an in-memory replication progress slot the ReplicationOirgin
+ * * When creating an in-memory replication progress slot the ReplicationOrigin
  *	 LWLock has to be held exclusively; when iterating over the replication
  *	 progress a shared lock has to be held, the same when advancing the
  *	 replication progress of an individual backend that has not setup as the
@@ -162,8 +162,8 @@ static ReplicationState *replication_states;
 static ReplicationStateCtl *replication_states_ctl;
 
 /*
- * Backend-local, cached element from ReplicationStates for use in a backend
- * replaying remote commits, so we don't have to search ReplicationStates for
+ * Backend-local, cached element from ReplicationState for use in a backend
+ * replaying remote commits, so we don't have to search ReplicationState for
  * the backends current RepOriginId.
  */
 static ReplicationState *session_replication_state = NULL;
@@ -441,7 +441,7 @@ ReplicationOriginShmemSize(void)
 	/*
 	 * XXX: max_replication_slots is arguably the wrong thing to use, as here
 	 * we keep the replay state of *remote* transactions. But for now it seems
-	 * sufficient to reuse it, lest we introduce a separate guc.
+	 * sufficient to reuse it, lest we introduce a separate GUC.
 	 */
 	if (max_replication_slots == 0)
 		return size;
@@ -497,7 +497,7 @@ ReplicationOriginShmemInit(void)
  *
  * So its just the magic, followed by the statically sized
  * ReplicationStateOnDisk structs. Note that the maximum number of
- * ReplicationStates is determined by max_replication_slots.
+ * ReplicationState is determined by max_replication_slots.
  * ---------------------------------------------------------------------------
  */
 void
@@ -1250,7 +1250,7 @@ pg_replication_origin_session_is_setup(PG_FUNCTION_ARGS)
  * Return the replication progress for origin setup in the current session.
  *
  * If 'flush' is set to true it is ensured that the returned value corresponds
- * to a local transaction that has been flushed. this is useful if asynchronous
+ * to a local transaction that has been flushed. This is useful if asynchronous
  * commits are used when replaying replicated transactions.
  */
 Datum
@@ -1324,7 +1324,7 @@ pg_replication_origin_advance(PG_FUNCTION_ARGS)
 	 * set up the initial replication state, but not for replay.
 	 */
 	replorigin_advance(node, remote_commit, InvalidXLogRecPtr,
-					   true /* go backward */ , true /* wal log */ );
+					   true /* go backward */ , true /* WAL log */ );
 
 	UnlockRelationOid(ReplicationOriginRelationId, RowExclusiveLock);
 
@@ -1336,7 +1336,7 @@ pg_replication_origin_advance(PG_FUNCTION_ARGS)
  * Return the replication progress for an individual replication origin.
  *
  * If 'flush' is set to true it is ensured that the returned value corresponds
- * to a local transaction that has been flushed. this is useful if asynchronous
+ * to a local transaction that has been flushed. This is useful if asynchronous
  * commits are used when replaying replicated transactions.
  */
 Datum
diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index bb607b6..716cc0b 100644
--- a/src/backend/replication/logical/proto.c
+++ b/src/backend/replication/logical/proto.c
@@ -377,7 +377,7 @@ logicalrep_read_typ(StringInfo in, LogicalRepTyp *ltyp)
 {
 	ltyp->remoteid = pq_getmsgint(in, 4);
 
-	/* Read tupe name from stream */
+	/* read type name from stream */
 	ltyp->nspname = pstrdup(logicalrep_read_namespace(in));
 	ltyp->typname = pstrdup(pq_getmsgstring(in));
 }
@@ -459,7 +459,7 @@ logicalrep_read_tuple(StringInfo in, LogicalRepTupleData *tuple)
 	int			i;
 	int			natts;
 
-	/* Get of attributes. */
+	/* Get number of attributes */
 	natts = pq_getmsgint(in, 2);
 
 	memset(tuple->changed, 0, sizeof(tuple->changed));
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 108326b..d126692 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -33,12 +33,12 @@
  *		 When the desired state appears it will compare its position in the
  *		 stream with the SYNCWAIT position and based on that changes the
  *		 state to based on following rules:
- *		  - if the apply is in front of the sync in the wal stream the new
+ *		  - if the apply is in front of the sync in the WAL stream the new
  *			state is set to CATCHUP and apply loops until the sync process
  *			catches up to the same LSN as apply
- *		  - if the sync is in front of the apply in the wal stream the new
+ *		  - if the sync is in front of the apply in the WAL stream the new
  *			state is set to SYNCDONE
- *		  - if both apply and sync are at the same position in the wal stream
+ *		  - if both apply and sync are at the same position in the WAL stream
  *			the state of the table is set to READY
  *	   - If the state was set to CATCHUP sync will read the stream and
  *		 apply changes until it catches up to the specified stream
@@ -698,7 +698,7 @@ copy_table(Relation rel)
 /*
  * Start syncing the table in the sync worker.
  *
- * The returned slot name is palloced in current memory context.
+ * The returned slot name is palloc'ed in current memory context.
  */
 char *
 LogicalRepSyncTableStart(XLogRecPtr *origin_startpos)
-- 
2.1.4

From 9aa042a750b0c755c90d66c86e114039bdc661ff Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler@timbira.com.br>
Date: Mon, 17 Apr 2017 22:12:02 -0300
Subject: [PATCH 2/3] Fix query that gets remote relation info

Publisher relation can be incorrectly chosen, if there are more than
one relation in different schemas with the same name.
---
 src/backend/replication/logical/tablesync.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index d126692..f41f633 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -560,8 +560,9 @@ fetch_remote_table_info(char *nspname, char *relname,
 	/* First fetch Oid and replica identity. */
 	initStringInfo(&cmd);
 	appendStringInfo(&cmd, "SELECT c.oid, c.relreplident"
-						   "  FROM pg_catalog.pg_class c,"
-						   "       pg_catalog.pg_namespace n"
+						   "  FROM pg_catalog.pg_class c"
+						   "  INNER JOIN pg_catalog.pg_namespace n"
+						   "        ON (c.relnamespace = n.oid)"
 						   " WHERE n.nspname = %s"
 						   "   AND c.relname = %s"
 						   "   AND c.relkind = 'r'",
-- 
2.1.4

From a6f6d9f7fc5d07d1c96f0361885a3461740f7e87 Mon Sep 17 00:00:00 2001
From: Euler Taveira <euler@timbira.com.br>
Date: Tue, 18 Apr 2017 12:37:44 -0300
Subject: [PATCH 3/3] ALTER SUBSCRIPTION documentation fixes

WITH is optional for REFRESH PUBLICATION. Also, remove a spurious
bracket and fix a punctuation.
---
 doc/src/sgml/ref/alter_subscription.sgml | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index f71ee38..35aeb6a 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  <refsynopsisdiv>
 <synopsis>
-ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="PARAMETER">suboption</replaceable> [, ... ] ) ]
+ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> WITH ( <replaceable class="PARAMETER">suboption</replaceable> [, ... ] )
 
 <phrase>where <replaceable class="PARAMETER">suboption</replaceable> can be:</phrase>
 
@@ -29,7 +29,7 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> WITH ( <rep
     | SYNCHRONOUS_COMMIT = <replaceable class="PARAMETER">synchronous_commit</replaceable>
 
 ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> SET PUBLICATION <replaceable class="PARAMETER">publication_name</replaceable> [, ...] { REFRESH WITH ( <replaceable class="PARAMETER">puboption</replaceable> [, ... ] ) | NOREFRESH }
-ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> REFRESH PUBLICATION WITH ( <replaceable class="PARAMETER">puboption</replaceable> [, ... ] )
+ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> REFRESH PUBLICATION [ WITH ( <replaceable class="PARAMETER">puboption</replaceable> [, ... ] ) ]
 
 <phrase>where <replaceable class="PARAMETER">puboption</replaceable> can be:</phrase>
 
@@ -54,7 +54,7 @@ ALTER SUBSCRIPTION <replaceable class="PARAMETER">name</replaceable> DISABLE
 
   <para>
    To alter the owner, you must also be a direct or indirect member of the
-   new owning role. The new owner has to be a superuser
+   new owning role. The new owner has to be a superuser.
   </para>
  </refsect1>
 
-- 
2.1.4