Hi Vignesh,
Some minor review comments for patches v37-0005/0006 combined.
======
src/backend/replication/logical/conflict.c
1.
+/* Schema for the elements within the 'local_conflicts' JSON array */
+static const ConflictLogColumnDef LocalConflictSchema[] =
+{
+ { .attname = "xid", .atttypid = XIDOID },
+ { .attname = "commit_ts", .atttypid = TIMESTAMPTZOID },
+ { .attname = "origin", .atttypid = TEXTOID },
+ { .attname = "key", .atttypid = JSONOID },
+ { .attname = "tuple", .atttypid = JSONOID }
+};
+
+#define NUM_LOCAL_CONFLICT_ATTRS lengthof(LocalConflictSchema)
+
IMO this belongs *below* the ConflictLogSchema[], which is where
'local_conflicts' attribute was introduced, instead of above it.
~~~
2.
+
+
static int errcode_apply_conflict(ConflictType type);
~
There are some spurious blank lines here that should not be in the patch.
~~~
ProcessPendingConflictLogTuple:
3.
+ /* Open conflict log table and insert the tuple */
+ conflictlogrel = GetConflictLogDestAndTable(&dest);
+ Assert(CONFLICTS_LOGGED_TO_TABLE(dest));
Maybe here it's better to say Assert(conflictlogrel);
======
Kind Regards,
Peter Smith.
Fujitsu Australia
