Hi,
While inspecting the logical replication code, I found a bug that could
pick the wrong remote relation if they have the same name but different
schemas. Also, I did some spelling/cosmetic changes and fixed an oversight
in the ALTER SUBSCRIPTION documentation. Patches attached.
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
<http://www.timbira.com.br>
From da4dc2807566958dd89cc9f05bf6a83a996e99c7 Mon Sep 17 00:00:00 2001
From: Euler Taveira <eu...@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 <eu...@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 <eu...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
