Re: adding partitioned tables to publications

On 2020-04-07 08:44, Amit Langote wrote:
> I updated the patch to make the following changes:
> 
> * Rewrote the tests to match in style with those committed yesterday
> * Renamed all variables that had pubasroot in it to have pubviaroot
> instead to match the publication parameter
> * Updated pg_publication catalog documentation

Thanks.  I have some further questions:

The change in nodeModifyTable.c to add CheckValidResultRel() is unclear. 
  It doesn't seem to do anything, and it's not clear how it's related to 
this patch.

The changes in GetRelationPublications() are confusing to me:

+   if (published_rels)
+   {
+       num = list_length(result);
+       for (i = 0; i < num; i++)
+           *published_rels = lappend_oid(*published_rels, relid);
+   }

This adds relid to the output list "num" times, where num is the number 
of publications found.  Shouldn't "i" be used in the loop somehow? 
Similarly later in the function.

The descriptions of the new fields in RelationSyncEntry don't seem to 
match the code accurately, or at least it's confusing. 
replicate_as_relid is always filled in with an ancestor, even if 
pubviaroot is not set.

I think the pubviaroot field is actually not necessary.  We only need 
replicate_as_relid.

There is a markup typo in logical-replication.sgml:

     <xref linkend=="sql-createpublication"/>

In pg_dump, you missed updating a branch for an older version.  See 
attached patch.

Also attached a patch to rephrase the psql output a bit to make it not 
so long.

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