On 6/20/17 00:10, Andres Freund wrote:
> On 2017-06-20 11:52:10 +0900, Tatsuo Ishii wrote:
>> If my understanding is correct, it would not be easy to fix, no?
>>
>>> We might be able to refine that, but there is a general problem that
>>> without an index and an operator class, we are just doing our random
>>> best to match the values.
>
> I don't see the problem as being big. We should just look up the
> default btree opclass and use the relevant operator. That's a how a
> number of things already work.
Patch for that.
Any thoughts about keeping datumAsEqual() as a first check? I did some
light performance tests, but it was inconclusive.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 00ea950753c070d440818c71d06e2bbc53b6c294 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Fri, 23 Jun 2017 08:55:13 -0400
Subject: [PATCH] Fix replication with replica identity full
The comparison with the target rows on the subscriber side was done with
datumIsEqual(), which can have false negatives. For instance, it didn't
work reliably for text columns. So use the equality operator provided
by the type cache as fallback.
Also add more user documentation about replica identity requirements.
Reported-by: Tatsuo Ishii <is...@sraoss.co.jp>
---
doc/src/sgml/logical-replication.sgml | 28 +++++++++++++++++++++++-----
src/backend/executor/execReplication.c | 22 +++++++++++++++++++++-
src/test/subscription/t/001_rep_changes.pl | 23 ++++++++++++++++++++---
3 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml
b/doc/src/sgml/logical-replication.sgml
index 92ec175af1..fa8ae536d9 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -110,11 +110,29 @@ <title>Publication</title>
Publications can choose to limit the changes they produce to
any combination of <command>INSERT</command>, <command>UPDATE</command>, and
<command>DELETE</command>, similar to how triggers are fired by
- particular event types. If a table without a <literal>REPLICA
- IDENTITY</literal> is added to a publication that
- replicates <command>UPDATE</command> or <command>DELETE</command>
- operations then subsequent <command>UPDATE</command>
- or <command>DELETE</command> operations will fail on the publisher.
+ particular event types. By default, all operation types are replicated.
+ </para>
+
+ <para>
+ A published table must have a <quote>replica identity</quote> configured in
+ order to be able to replicate <command>UPDATE</command>
+ and <command>DELETE</command> operations, so that appropriate rows to
+ update or delete can be identified on the subscriber side. By default,
+ this is the primary key, if there is one. Another unique index (with
+ certain additional requirements) can also be set to be the replica
+ identity. If the table does not have any suitable key, then it can be set
+ to replica identity <quote>full</quote>, which means the entire row becomes
+ the key. This, however, is very inefficient and should only be used as a
+ fallback if no other solution is possible. If a replica identity other
+ than <quote>full</quote> is set on the publisher side, a replica identity
+ comprising the same or fewer columns must also be set on the subscriber
+ side. See <xref linkend="SQL-CREATETABLE-REPLICA-IDENTITY"> for details on
+ how to set the replica identity. If a table without a replica identity is
+ added to a publication that replicates <command>UPDATE</command>
+ or <command>DELETE</command> operations then
+ subsequent <command>UPDATE</command> or <command>DELETE</command>
+ operations will cause an error on the publisher. <command>INSERT</command>
+ operations can proceed regardless of any replica identity.
</para>
<para>
diff --git a/src/backend/executor/execReplication.c
b/src/backend/executor/execReplication.c
index 6dae79a8f0..97da47a1e2 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -24,12 +24,14 @@
#include "parser/parsetree.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
+#include "utils/builtins.h"
#include "utils/datum.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+#include "utils/typcache.h"
#include "utils/tqual.h"
@@ -245,9 +247,27 @@ tuple_equals_slot(TupleDesc desc, HeapTuple tup,
TupleTableSlot *slot)
continue;
att = desc->attrs[attrnum];
+
+ /*
+ * To compare for equality, first try datumIsEqual(). If that
returns
+ * false, try the equality operator.
+ */
if (!datumIsEqual(values[attrnum], slot->tts_values[attrnum],
att->attbyval, att->attlen))
- return false;
+ {
+ TypeCacheEntry *typentry;
+
+ typentry = lookup_type_cache(att->atttypid,
TYPECACHE_EQ_OPR_FINFO);
+ if (!OidIsValid(typentry->eq_opr_finfo.fn_oid))
+ ereport(ERROR,
+
(errcode(ERRCODE_UNDEFINED_FUNCTION),
+ errmsg("could not identify an
equality operator for type %s",
+
format_type_be(att->atttypid))));
+ if (!DatumGetBool(FunctionCall2(&typentry->eq_opr_finfo,
+
values[attrnum],
+
slot->tts_values[attrnum])))
+ return false;
+ }
}
return true;
diff --git a/src/test/subscription/t/001_rep_changes.pl
b/src/test/subscription/t/001_rep_changes.pl
index f9cf5e4392..a63c679848 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -3,7 +3,7 @@
use warnings;
use PostgresNode;
use TestLib;
-use Test::More tests => 15;
+use Test::More tests => 16;
# Initialize publisher node
my $node_publisher = get_new_node('publisher');
@@ -23,6 +23,10 @@
$node_publisher->safe_psql('postgres',
"CREATE TABLE tab_full AS SELECT generate_series(1,10) AS a");
$node_publisher->safe_psql('postgres',
+ "CREATE TABLE tab_full2 (x text)");
+$node_publisher->safe_psql('postgres',
+ "INSERT INTO tab_full2 VALUES ('a'), ('b'), ('b')");
+$node_publisher->safe_psql('postgres',
"CREATE TABLE tab_rep (a int primary key)");
$node_publisher->safe_psql('postgres',
"CREATE TABLE tab_mixed (a int primary key, b text)");
@@ -33,6 +37,7 @@
$node_subscriber->safe_psql('postgres', "CREATE TABLE tab_notrep (a int)");
$node_subscriber->safe_psql('postgres', "CREATE TABLE tab_ins (a int)");
$node_subscriber->safe_psql('postgres', "CREATE TABLE tab_full (a int)");
+$node_subscriber->safe_psql('postgres', "CREATE TABLE tab_full2 (x text)");
$node_subscriber->safe_psql('postgres',
"CREATE TABLE tab_rep (a int primary key)");
# different column count and order than on publisher
@@ -45,7 +50,7 @@
$node_publisher->safe_psql('postgres',
"CREATE PUBLICATION tap_pub_ins_only WITH (publish = insert)");
$node_publisher->safe_psql('postgres',
- "ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full, tab_mixed");
+ "ALTER PUBLICATION tap_pub ADD TABLE tab_rep, tab_full, tab_full2,
tab_mixed");
$node_publisher->safe_psql('postgres',
"ALTER PUBLICATION tap_pub_ins_only ADD TABLE tab_ins");
@@ -109,12 +114,17 @@
$node_subscriber->safe_psql('postgres',
"ALTER TABLE tab_full REPLICA IDENTITY FULL");
$node_publisher->safe_psql('postgres',
+ "ALTER TABLE tab_full2 REPLICA IDENTITY FULL");
+$node_subscriber->safe_psql('postgres',
+ "ALTER TABLE tab_full2 REPLICA IDENTITY FULL");
+$node_publisher->safe_psql('postgres',
"ALTER TABLE tab_ins REPLICA IDENTITY FULL");
$node_subscriber->safe_psql('postgres',
"ALTER TABLE tab_ins REPLICA IDENTITY FULL");
-# and do the update
+# and do the updates
$node_publisher->safe_psql('postgres', "UPDATE tab_full SET a = a * a");
+$node_publisher->safe_psql('postgres', "UPDATE tab_full2 SET x = 'bb' WHERE x
= 'b'");
# Wait for subscription to catch up
$node_publisher->poll_query_until('postgres', $caughtup_query)
@@ -125,6 +135,13 @@
is($result, qq(20|1|100),
'update works with REPLICA IDENTITY FULL and duplicate tuples');
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT x FROM tab_full2 ORDER BY 1");
+is($result, qq(a
+bb
+bb),
+ 'update works with REPLICA IDENTITY FULL and text datums');
+
# check that change of connection string and/or publication list causes
# restart of subscription workers. Not all of these are registered as tests
# as we need to poll for a change but the test suite will fail none the less
--
2.13.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
