Re: adding partitioned tables to publications

From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>
Cc: Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: adding partitioned tables to publications
Date: 2020-03-25 12:29:52
Message-ID: c0d69728-c3d8-ede8-ad31-941b2bbaff66@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2020-03-23 06:02, Amit Langote wrote:
> Okay, added some tests.
>
> Attached updated patches.

I have committed the worker.c refactoring patch.

"Add subscription support to replicate into partitioned tables" still
has lacking test coverage. Your changes in relation.c are not exercised
at all because the partitioned table branch in apply_handle_update() is
never taken. This is critical and tricky code, so I would look for
significant testing.

The code looks okay to me. I would remove this code

+ memset(entry->attrmap->attnums, -1,
+ entry->attrmap->maplen * sizeof(AttrNumber));

because the entries are explicitly filled right after anyway, and
filling the bytes with -1 has an unclear effect. There is also
seemingly some fishiness in this code around whether attribute numbers
are zero- or one-based. Perhaps this could be documented briefly.
Maybe I'm misunderstanding something.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2020-03-25 12:41:55 Re: error context for vacuum to include block number
Previous Message Masahiko Sawada 2020-03-25 12:27:44 Re: error context for vacuum to include block number