Re: adding partitioned tables to publications

Hi Amit,

I went through this patch set once again today and here are my two cents.

On Mon, 16 Dec 2019 at 10:19, Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>
> Attached updated patches.
- differently partitioned setup. Attempts to replicate tables other than
- base tables will result in an error.
+ Replication is only supported by regular and partitioned tables, although
+ the table kind must match between the two servers, that is, one cannot

I find the phrase 'table kind' a bit odd, how about something like
type of the table.

/* Only plain tables can be aded to publications. */
- if (tbinfo->relkind != RELKIND_RELATION)
+ /* Only plain and partitioned tables can be added to publications. */
IMHO using regular instead of plain would be more consistent.

+ /*
+ * Find a partition for the tuple contained in remoteslot.
+ *
+ * For insert, remoteslot is tuple to insert. For update and delete, it
+ * is the tuple to be replaced and deleted, repectively.
+ */
Misspelled 'respectively'.

+static void
+apply_handle_tuple_routing(ResultRelInfo *relinfo,
+ LogicalRepRelMapEntry *relmapentry,
+ EState *estate, CmdType operation,
+ TupleTableSlot *remoteslot,
+ LogicalRepTupleData *newtup)
+{
+ Relation rel = relinfo->ri_RelationDesc;
+ ModifyTableState *mtstate = NULL;
+ PartitionTupleRouting *proute = NULL;
+ ResultRelInfo *partrelinfo,
+ *partrelinfo1;

IMHO, partrelinfo1 can be better named to improve readability.

Otherwise, as Dilip already mentioned, there is a rebase required
particularly for 0003 and 0004.

--
Regards,
Rafia Sabih