Re: adding partitioned tables to publications - Mailing list pgsql-hackers

From Amit Langote
Subject Re: adding partitioned tables to publications
Date
Msg-id CA+HiwqFWAVtvguFCHaCUxp2s4YtdFqhDfwB+0OnrWjJWjDpbxQ@mail.gmail.com
Whole thread Raw
In response to Re: adding partitioned tables to publications  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
Responses Re: adding partitioned tables to publications  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
On Thu, Apr 9, 2020 at 4:14 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> On 2020-04-09 05:39, Amit Langote wrote:
> > sub_viaroot ERROR:  number of columns (2601) exceeds limit (1664)
> > sub_viaroot CONTEXT:  slot "sub_viaroot", output plugin "pgoutput", in
> > the change callback, associated LSN 0/1621010
>
> I think the problem is that in maybe_send_schema(),
> RelationClose(ancestor) releases the relcache entry, but the tuple
> descriptors, which are part of the relcache entry, are still pointed to
> by the tuple map.
>
> 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.

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?

-- 

Amit Langote
EnterpriseDB: http://www.enterprisedb.com

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Vacuum o/p with (full 1, parallel 0) option throwing an error
Next
From: Fujii Masao
Date:
Subject: Re: [HACKERS] make async slave to wait for lsn to be replayed