Subject: Re: adding partitioned tables to publications
To: Amit Langote <amitlangote09@gmail.com>
References: 
 <CA+HiwqH=Y85vRK3mOdjEkqFK+E=ST=eQiHdpj43L=_eJMOOznQ@mail.gmail.com>
 <CA+HiwqGkknVb+ccLVkgw1DoBYPzpuR43L+aT1eOT8dVEtiedtw@mail.gmail.com>
 <bb9427bd-9da9-68f6-108e-d1b21aa5a307@2ndquadrant.com>
 <CA+HiwqFA6V1QKLnHjyrVUBiDF4+K0U8MvymWWgkv4uWgeUfQrQ@mail.gmail.com>
 <CA+HiwqE2-6U=XzsBZ9efzPatFDgrgGybBEj7vm-kUQL8DM4BWw@mail.gmail.com>
 <64a2c1c7-3887-787b-e43b-ac15112bda82@2ndquadrant.com>
 <CA+HiwqGVmZtdwDrGy3ak-k5V=QGH6kpb46aKOtpbt3R57RPwFA@mail.gmail.com>
 <CA+HiwqEThKQh6bPOj68fqUO3s+PRBdUeAfe5GGDgoYdU_fb2Sw@mail.gmail.com>
 <f4a265b3-e1e4-4367-c7db-57fac0a26545@2ndquadrant.com>
 <CA+HiwqGKsb1S1GgvdCsvOmOgva=W1fAb8eAYHhLePinUf+XwKQ@mail.gmail.com>
 <21de0e53-0f5f-f31a-2dcb-6e4b35571c94@2ndquadrant.com>
 <CA+HiwqFyydvQ5g=qa54UM+Xjm77BdhX-nM4dXQkNOgH=zvDjoA@mail.gmail.com>
 <CA+HiwqEf_bZbiUXTanwbrTUbtN1hXb0s1orokXALKMNSs3E=pw@mail.gmail.com>
 <04d78603-edae-9243-9dde-fe3037176a7d@2ndquadrant.com>
 <CA+HiwqFWAVtvguFCHaCUxp2s4YtdFqhDfwB+0OnrWjJWjDpbxQ@mail.gmail.com>
Cc: Petr Jelinek <petr@2ndquadrant.com>,
 Rafia Sabih <rafia.pghackers@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Organization: 2ndQuadrant
Message-ID: <3d7d2aef-03ea-158d-d43a-e349df73f879@2ndquadrant.com>
Date: Fri, 17 Apr 2020 15:23:50 +0200
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:52.0)
 Gecko/20100101 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: 
 <CA+HiwqFWAVtvguFCHaCUxp2s4YtdFqhDfwB+0OnrWjJWjDpbxQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2020-04-09 09:28, Amit Langote wrote:
>> This patch makes the tests pass for me:
>>
>> diff --git a/src/backend/replication/pgoutput/pgoutput.c
>> b/src/backend/replication/pgoutput/pgoutput.c
>> index 5fbf2d4367..cf6e8629c1 100644
>> --- a/src/backend/replication/pgoutput/pgoutput.c
>> +++ b/src/backend/replication/pgoutput/pgoutput.c
>> @@ -305,7 +305,7 @@ maybe_send_schema(LogicalDecodingContext *ctx,
>>
>>           /* Map must live as long as the session does. */
>>           oldctx = MemoryContextSwitchTo(CacheMemoryContext);
>> -       relentry->map = convert_tuples_by_name(indesc, outdesc);
>> +       relentry->map =
>> convert_tuples_by_name(CreateTupleDescCopy(indesc),
>> CreateTupleDescCopy(outdesc));
>>           MemoryContextSwitchTo(oldctx);
>>           send_relation_and_attrs(ancestor, ctx);
>>           RelationClose(ancestor);
>>
>> Please check.
> 
> Thanks.  Yes, that's what I just found out too and was about to send a
> patch, which is basically same as yours as far as the fix for this
> issue is concerned.

I have committed my patch but not ...

> While figuring this out, I thought the nearby code could be rearranged
> a bit, especially to de-duplicate the code.  Also, I think
> get_rel_sync_entry() may be a better place to set the map, rather than
> maybe_send_schema().  Thoughts?

because I didn't really have an opinion on that at the time, but if you 
still want it considered or have any open thoughts on this thread, 
please resend or explain.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services