Re: [HACKERS] Handling dropped attributes in pglogical_proto

2016-09-29 Thread Craig Ringer
On 29 September 2016 at 12:53, Petr Jelinek  wrote:
> On 29/09/16 05:33, Michael Paquier wrote:
>> On Wed, Sep 28, 2016 at 11:25 PM, Konstantin Knizhnik
>>  wrote:
>>> But if table was just altered and some attribute was removed from the table,
>>> then rel->natts can be greater than natts.
>>
>> This is part of pglogical, so you may want to reply on the dedicated
>> thread or send directly a patch to them. By the way, this code path
>> may need to care as well about attisdropped. It is never good not to
>> check for it.
>>
>
> Agreed this does not belong to hackers and should reported on github.
>
> But just as note the rel variable is not Relation, it's
> PGLogicalRelation which gets populated by relation message from the
> upstream so if you observe the behavior mentioned in the original email,
> you are probably doing something unexpected there (Konstantin is not
> using vanilla pglogical). Also the attmap should never contain
> attisdropped attributes.

Yeah, and if you're relying on attno equivalence you're also going to
have to deal with it in dump/restore unless you only create new nodes
using physical copy. (See bdr's bdr_dump).

BDR relies on attno equivalence. pglogical avoids doing because of the
pain it caused in BDR.

On a side note, do NOT report bugs on ANY software when you're talking
about a modified version unless you clearly disclose that it's
modified. You'll waste everyone's time. We already spent a while
looking into a logical decoding issue you reported that turned out to
be caused by your team's patches to add 2PC decoding support, which
you didn't mention in the report. If you want anyone to listen to your
reports you really need to be very explicit about whether it's a
modified version or not. If it's modified, make sure you can reproduce
the problem in the unmodified version or explain the situation more.

If you want to ask things about BDR and pglogical, now that it's clear
that in-core logical replication is going in another direction so
-hackers isn't the place for it, please use bdr-l...@2ndquadrant.com .

-- 
 Craig Ringer   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


Re: [HACKERS] Handling dropped attributes in pglogical_proto

2016-09-28 Thread Petr Jelinek
On 29/09/16 05:33, Michael Paquier wrote:
> On Wed, Sep 28, 2016 at 11:25 PM, Konstantin Knizhnik
>  wrote:
>> But if table was just altered and some attribute was removed from the table,
>> then rel->natts can be greater than natts.
> 
> This is part of pglogical, so you may want to reply on the dedicated
> thread or send directly a patch to them. By the way, this code path
> may need to care as well about attisdropped. It is never good not to
> check for it.
> 

Agreed this does not belong to hackers and should reported on github.

But just as note the rel variable is not Relation, it's
PGLogicalRelation which gets populated by relation message from the
upstream so if you observe the behavior mentioned in the original email,
you are probably doing something unexpected there (Konstantin is not
using vanilla pglogical). Also the attmap should never contain
attisdropped attributes.

-- 
  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


Re: [HACKERS] Handling dropped attributes in pglogical_proto

2016-09-28 Thread Michael Paquier
On Wed, Sep 28, 2016 at 11:25 PM, Konstantin Knizhnik
 wrote:
> But if table was just altered and some attribute was removed from the table,
> then rel->natts can be greater than natts.

This is part of pglogical, so you may want to reply on the dedicated
thread or send directly a patch to them. By the way, this code path
may need to care as well about attisdropped. It is never good not to
check for it.
-- 
Michael


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