Horton

Re: adding partitioned tables to publications

Rafia Sabih <rafia.pghackers@gmail.com>

View thread
From: Rafia Sabih <rafia.pghackers@gmail.com>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-01-06T11:25:32Z
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

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@gmail.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