Different table schema in logical replication crashes
Hi,
If a certain table has different schemas and the subscriber table has an
unmatched column with a not null constraint, the logical replication
crashes with the above stack trace.
-- publisher
CREATE TABLE test (a integer, b varchar not null, c numeric not null,
PRIMARY KEY(a));
-- subscriber
CREATE TABLE test (a integer, b varchar not null, c numeric not null, d
integer not null, PRIMARY KEY(a));
Program terminated with signal SIGSEGV, Segmentation fault.
#0 list_nth_cell (n=0, list=0x0) at list.c:411
411 {
(gdb) bt
#0 list_nth_cell (n=0, list=0x0) at list.c:411
#1 list_nth (list=0x0, n=0) at list.c:413
#2 0x00000000005ddc6b in ExecConstraints
(resultRelInfo=resultRelInfo@entry=0xf96868,
slot=slot@entry=0xf984d8, estate=estate@entry=0xfc3808) at execMain.c:1881
#3 0x000000000057b0ba in CopyFrom (cstate=0xf980c8) at copy.c:2652
#4 0x00000000006ae3bb in copy_table (rel=<optimized out>) at
tablesync.c:682
#5 LogicalRepSyncTableStart (origin_startpos=0x7ffe9c340640) at
tablesync.c:789
#6 0x00000000006afb0f in ApplyWorkerMain (main_arg=<optimized out>) at
worker.c:1521
#7 0x0000000000684813 in StartBackgroundWorker () at bgworker.c:838
#8 0x000000000068f6a2 in do_start_bgworker (rw=0xf0cbb0) at
postmaster.c:5577
#9 maybe_start_bgworker () at postmaster.c:5761
#10 0x0000000000690195 in sigusr1_handler (postgres_signal_arg=<optimized
out>) at postmaster.c:5015
#11 <signal handler called>
#12 0x00007fcd075f6873 in __select_nocancel () at
../sysdeps/unix/syscall-template.S:81
#13 0x0000000000476c0c in ServerLoop () at postmaster.c:1693
#14 0x0000000000691342 in PostmasterMain (argc=argc@entry=1,
argv=argv@entry=0xee4eb0)
at postmaster.c:1337
#15 0x0000000000478684 in main (argc=1, argv=0xee4eb0) at main.c:228
Are we prepared to support different schemas in v10? Or should we disallow
it for v10 and add a TODO?
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
<http://www.timbira.com.br>
On 13/04/17 05:04, Euler Taveira wrote:
Hi,
If a certain table has different schemas and the subscriber table has an
unmatched column with a not null constraint, the logical replication
crashes with the above stack trace.[snip]
Are we prepared to support different schemas in v10? Or should we
disallow it for v10 and add a TODO?
Ah nuts, yes it's supposed to be supported, we seem to not initialize
cstate->range_table in tablesync which causes this bug. The CopyState
struct is private to copy.c so we can't easily set cstate->range_table
externally. I wonder if tablesync should just construct CopyStmt instead
of calling the lower level API.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/14/17 08:49, Petr Jelinek wrote:
Are we prepared to support different schemas in v10? Or should we
disallow it for v10 and add a TODO?Ah nuts, yes it's supposed to be supported, we seem to not initialize
cstate->range_table in tablesync which causes this bug. The CopyState
struct is private to copy.c so we can't easily set cstate->range_table
externally. I wonder if tablesync should just construct CopyStmt instead
of calling the lower level API.
Maybe pass the range_table to BeginCopyFrom so that it can write it into
cstate?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 14/04/17 17:33, Peter Eisentraut wrote:
On 4/14/17 08:49, Petr Jelinek wrote:
Are we prepared to support different schemas in v10? Or should we
disallow it for v10 and add a TODO?Ah nuts, yes it's supposed to be supported, we seem to not initialize
cstate->range_table in tablesync which causes this bug. The CopyState
struct is private to copy.c so we can't easily set cstate->range_table
externally. I wonder if tablesync should just construct CopyStmt instead
of calling the lower level API.Maybe pass the range_table to BeginCopyFrom so that it can write it into
cstate?
That would work. The reason why I am thinking of creating CopyStmt
instead is that to create the range_table, we'll basically have to
duplicate the code from DoCopy verbatim. Obviously making CopyStmt isn't
without troubles either as it would have to newly support the callback
input.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 14/04/17 17:33, Peter Eisentraut wrote:
On 4/14/17 08:49, Petr Jelinek wrote:
Are we prepared to support different schemas in v10? Or should we
disallow it for v10 and add a TODO?Ah nuts, yes it's supposed to be supported, we seem to not initialize
cstate->range_table in tablesync which causes this bug. The CopyState
struct is private to copy.c so we can't easily set cstate->range_table
externally. I wonder if tablesync should just construct CopyStmt instead
of calling the lower level API.Maybe pass the range_table to BeginCopyFrom so that it can write it into
cstate?
I tried something bit different which seems cleaner to me - use the
pstate->r_table instead of ad-hock locally made up range table and fill
that using standard addRangeTableEntryForRelation. Both in tablesync and
in DoCopy instead of the old coding.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
Set-range-table-for-CopyFrom-in-tablesync.patchtext/plain; charset=UTF-8; name=Set-range-table-for-CopyFrom-in-tablesync.patchDownload
From 359c10959f639c1dd3b01f90d459ee4f03ccbd0b Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Sat, 15 Apr 2017 03:27:32 +0200
Subject: [PATCH] Set range table for CopyFrom in tablesync
This changes the mechanism of how range table is passed to CopyFrom
executor state. We used to generate the range table and one entry for
the relation manually inside DoCopy.Now we use
addRangeTableEntryForRelation to setup the range table and relation
entry for the ParseState which is then passed down by BeginCopyFrom.
---
src/backend/commands/copy.c | 17 +++++++----------
src/backend/replication/logical/tablesync.c | 13 +++++++++++--
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index b5af2be..a786d7b 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -34,6 +34,7 @@
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
+#include "parser/parse_relation.h"
#include "optimizer/clauses.h"
#include "optimizer/planner.h"
#include "nodes/makefuncs.h"
@@ -787,7 +788,6 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
Relation rel;
Oid relid;
RawStmt *query = NULL;
- List *range_table = NIL;
/* Disallow COPY to/from file or program except to superusers. */
if (!pipe && !superuser())
@@ -809,7 +809,6 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
if (stmt->relation)
{
TupleDesc tupDesc;
- AclMode required_access = (is_from ? ACL_INSERT : ACL_SELECT);
List *attnums;
ListCell *cur;
RangeTblEntry *rte;
@@ -822,12 +821,8 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
relid = RelationGetRelid(rel);
- rte = makeNode(RangeTblEntry);
- rte->rtekind = RTE_RELATION;
- rte->relid = RelationGetRelid(rel);
- rte->relkind = rel->rd_rel->relkind;
- rte->requiredPerms = required_access;
- range_table = list_make1(rte);
+ rte = addRangeTableEntryForRelation(pstate, rel, NULL, false, false);
+ rte->requiredPerms = (stmt->is_from ? ACL_INSERT : ACL_SELECT);
tupDesc = RelationGetDescr(rel);
attnums = CopyGetAttnums(tupDesc, rel, stmt->attlist);
@@ -841,7 +836,7 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
else
rte->selectedCols = bms_add_member(rte->selectedCols, attno);
}
- ExecCheckRTPerms(range_table, true);
+ ExecCheckRTPerms(pstate->p_rtable, true);
/*
* Permission check for row security policies.
@@ -977,7 +972,6 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
cstate = BeginCopyFrom(pstate, rel, stmt->filename, stmt->is_program,
NULL, stmt->attlist, stmt->options);
- cstate->range_table = range_table;
*processed = CopyFrom(cstate); /* copy from file to database */
EndCopyFrom(cstate);
}
@@ -2921,6 +2915,9 @@ BeginCopyFrom(ParseState *pstate,
cstate->raw_buf = (char *) palloc(RAW_BUF_SIZE + 1);
cstate->raw_buf_index = cstate->raw_buf_len = 0;
+ /* Assign range table, we'll need it in CopyFrom. */
+ cstate->range_table = pstate->p_rtable;
+
tupDesc = RelationGetDescr(cstate->rel);
attr = tupDesc->attrs;
num_phys_attrs = tupDesc->natts;
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index d1f2734..9ba2c01 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -93,6 +93,8 @@
#include "commands/copy.h"
+#include "parser/parse_relation.h"
+
#include "replication/logicallauncher.h"
#include "replication/logicalrelation.h"
#include "replication/walreceiver.h"
@@ -648,6 +650,8 @@ copy_table(Relation rel)
StringInfoData cmd;
CopyState cstate;
List *attnamelist;
+ ParseState *pstate;
+ RangeTblEntry *rte;
/* Get the publisher relation info. */
fetch_remote_table_info(get_namespace_name(RelationGetNamespace(rel)),
@@ -674,9 +678,14 @@ copy_table(Relation rel)
copybuf = makeStringInfo();
- /* Create CopyState for ingestion of the data from publisher. */
+ pstate = make_parsestate(NULL);
+ pstate->p_target_relation = rel;
+ rte = addRangeTableEntryForRelation(pstate, pstate->p_target_relation,
+ NULL, false, false);
+ pstate->p_target_rangetblentry = rte;
+
attnamelist = make_copy_attnamelist(relmapentry);
- cstate = BeginCopyFrom(NULL, rel, NULL, false, copy_read_data, attnamelist, NIL);
+ cstate = BeginCopyFrom(pstate, rel, NULL, false, copy_read_data, attnamelist, NIL);
/* Do the copy */
(void) CopyFrom(cstate);
--
2.7.4
2017-04-14 22:36 GMT-03:00 Petr Jelinek <petr.jelinek@2ndquadrant.com>:
I tried something bit different which seems cleaner to me - use the
pstate->r_table instead of ad-hock locally made up range table and fill
that using standard addRangeTableEntryForRelation. Both in tablesync and
in DoCopy instead of the old coding.
Patch works fine. However, I don't see any documentation about supporting
different schemas for logical replication. Is it an oversight?
--
Euler Taveira Timbira -
http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
<http://www.timbira.com.br>
On 4/14/17 21:36, Petr Jelinek wrote:
I tried something bit different which seems cleaner to me - use the
pstate->r_table instead of ad-hock locally made up range table and fill
that using standard addRangeTableEntryForRelation. Both in tablesync and
in DoCopy instead of the old coding.
committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/17/17 08:47, Euler Taveira wrote:
Patch works fine. However, I don't see any documentation about
supporting different schemas for logical replication. Is it an oversight?
I have added more documentation about that.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers