Thread: [HACKERS] Different table schema in logical replication crashes

[HACKERS] Different table schema in logical replication crashes

From
Euler Taveira
Date:
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

Re: [HACKERS] Different table schema in logical replication crashes

From
Petr Jelinek
Date:
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



Re: [HACKERS] Different table schema in logical replication crashes

From
Peter Eisentraut
Date:
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



Re: [HACKERS] Different table schema in logical replication crashes

From
Petr Jelinek
Date:

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



Re: [HACKERS] Different table schema in logical replication crashes

From
Petr Jelinek
Date:
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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

Re: [HACKERS] Different table schema in logical replication crashes

From
Euler Taveira
Date:

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

Re: [HACKERS] Different table schema in logical replication crashes

From
Peter Eisentraut
Date:
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



Re: [HACKERS] Different table schema in logical replication crashes

From
Peter Eisentraut
Date:
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