Horton

Re: adding partitioned tables to publications

amit <amitlangote09@gmail.com>

View thread
From: Amit Langote <amitlangote09@gmail.com>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-01-22T05:38:06Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix RELCACHE_FORCE_RELEASE issue

  2. Allow publishing partition changes via ancestors

  3. Add logical replication support to replicate into partitioned tables

  4. Refactor code to look up local replication tuple

  5. Some refactoring of logical/worker.c

  6. Prepare to support non-tables in publications

  7. Support adding partitioned tables to publication

Attachments

Hi Peter
,
Thanks for the review and sorry it took me a while to get back.

On Wed, Jan 8, 2020 at 7:54 PM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> Looking through 0001, I think perhaps there is a better way to structure
> some of the API changes.
>
> Instead of passing the root_target_rel to CheckValidResultRel() and
> CheckCmdReplicaIdentity(), which we only need to check the publication
> actions of the root table, how about changing
> GetRelationPublicationActions() to automatically include the publication
> information of the root table.  Then we have that information in the
> relcache once and don't need to check the base table and the partition
> root separately at each call site (of which there is only one right
> now).  (Would that work correctly with relcache invalidation?)
>
> Similarly, couldn't GetRelationPublications() just automatically take
> partitioning into account?  We don't need the separation between
> GetRelationPublications() and GetRelationAncestorPublications().  This
> would also avoid errors of omission, for example the
> GetRelationPublications() call in ATPrepChangePersistence() doesn't take
> GetRelationAncestorPublications() into account.

I have addressed these comments in the attached updated patch.

Other than that, the updated patch contains following significant changes:

* Changed pg_publication.c: GetPublicationRelations() so that any
published partitioned tables are expanded as needed

* Since the pg_publication_tables view is backed by
GetPublicationRelations(), that means subscriptioncmds.c:
fetch_table_list() no longer needs to craft a query to include
partitions when needed, because partitions are included at source.
That seems better, because it allows to limit the complexity
surrounding publication of partitioned tables to the publication side.

* Fixed the publication table DDL to spot more cases of tables being
added to a publication in a duplicative manner.  For example,
partition being added to a publication which already contains its
ancestor and a partitioned tables being added to a publication
(implying all of its partitions are added) which already contains a
partition

Only attaching 0001.  Will send the rest after polishing them a bit more.

Thanks,
Amit