From 93903a9f5b06826ff8bdd166319d85618c858495 Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Thu, 17 Sep 2026 14:07:16 +0000 Subject: [PATCH v74 2/2] POC Only store RI in conflict log table --- doc/src/sgml/logical-replication.sgml | 26 ++------- src/backend/replication/logical/conflict.c | 66 +++++----------------- src/test/regress/expected/subscription.out | 5 +- src/test/subscription/t/035_conflicts.pl | 12 ++++ 4 files changed, 34 insertions(+), 75 deletions(-) diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index f36c878ca03..39bc3f7f273 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -2371,18 +2371,12 @@ DETAIL: detailed_explanation[: replica_identity_full boolean - Indicates whether replica_identity represents a full tuple (true) or key values of a replica identity index (false). This is NULL when replica_identity is not logged. + Indicates whether the conflicting relation uses REPLICA IDENTITY FULL (true) or a replica identity index (false). This is NULL when replica identity information is not applicable. replica_identity json - The JSON representation of the replica identity key values or full tuple. - - - remote_tuple - json - The JSON representation of the incoming remote row that caused - the conflict. + The JSON representation of the replica identity key values identifying the conflicting row. This is NULL when replica_identity_full is true or when replica identity is not applicable. local_conflicts @@ -2391,9 +2385,8 @@ DETAIL: detailed_explanation[: xid), commit timestamp - (commit_ts), origin (origin), - and the full local row image (tuple). This is - NULL if there are no local conflicting rows + (commit_ts), and origin (origin). + This is NULL if there are no local conflicting rows (e.g., in update_missing and delete_missing conflicts). @@ -2403,17 +2396,10 @@ DETAIL: detailed_explanation[: - The conflicting row data, including the incoming remote row (remote_tuple) - and the associated local conflict details (local_conflicts), is stored in + The replica identity key values (replica_identity) + and the associated local conflict details (local_conflicts) are stored in json formats for flexible querying and analysis. - - - Note that virtual generated column values appear as - null in these JSON columns, because such columns have no - stored value on the subscriber. In the server log, such columns are instead - shown with the constant value virtual. - diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c index 8997e550ea0..d140f6b160a 100644 --- a/src/backend/replication/logical/conflict.c +++ b/src/backend/replication/logical/conflict.c @@ -61,9 +61,9 @@ typedef struct ConflictLogColumnDef * type OID; the table is created in this column order by * create_conflict_log_table(). * - * The tuple/key columns (replica_identity, remote_tuple, local_conflicts) are - * typed json rather than jsonb on purpose: they hold an exact audit snapshot - * of the applied tuples and replica identity, and json preserves the verbatim + * The replica_identity and local_conflicts columns are typed json rather than + * jsonb on purpose: they hold an exact audit snapshot of the replica identity + * key values and local conflict metadata, and json preserves the verbatim * representation whereas jsonb would normalize it. Indexing them (jsonb's main * advantage) wouldn't help anyway, as the conflict log is looked up by its * scalar columns (relid, conflict_type, commit timestamp) while these json @@ -85,7 +85,6 @@ static const ConflictLogColumnDef ConflictLogSchema[] = { {.attname = "remote_origin", .atttypid = TEXTOID}, {.attname = "replica_identity_full", .atttypid = BOOLOID}, {.attname = "replica_identity", .atttypid = JSONOID}, - {.attname = "remote_tuple", .atttypid = JSONOID}, {.attname = "local_conflicts", .atttypid = JSONARRAYOID} }; @@ -98,8 +97,7 @@ static const ConflictLogColumnDef LocalConflictSchema[] = { {.attname = "xid", .atttypid = XIDOID}, {.attname = "commit_ts", .atttypid = TIMESTAMPTZOID}, - {.attname = "origin", .atttypid = TEXTOID}, - {.attname = "tuple", .atttypid = JSONOID} + {.attname = "origin", .atttypid = TEXTOID} }; #define NUM_LOCAL_CONFLICT_ATTRS lengthof(LocalConflictSchema) @@ -138,7 +136,6 @@ static void build_index_datums_from_slot(EState *estate, Relation localrel, bool *isnull); static char *build_index_value_desc(EState *estate, Relation localrel, TupleTableSlot *slot, Oid indexoid); -static Datum tuple_table_slot_to_json_datum(TupleTableSlot *slot); static Datum tuple_table_slot_to_indextup_json(EState *estate, Relation localrel, Oid replica_index, @@ -149,8 +146,7 @@ static void insert_conflict_log_tuple(EState *estate, Relation rel, Relation conflictlogrel, ConflictType conflict_type, TupleTableSlot *searchslot, - List *conflicttuples, - TupleTableSlot *remoteslot); + List *conflicttuples); /* * Builds the TupleDesc for the conflict log table. @@ -431,8 +427,7 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel, conflictlogrel, type, searchslot, - conflicttuples, - remoteslot); + conflicttuples); table_close(conflictlogrel, NoLock); } } @@ -1009,29 +1004,6 @@ build_index_value_desc(EState *estate, Relation localrel, TupleTableSlot *slot, return index_value; } -/* - * tuple_table_slot_to_json_datum - * - * Helper function to convert a TupleTableSlot to JSON. - */ -static Datum -tuple_table_slot_to_json_datum(TupleTableSlot *slot) -{ - HeapTuple tuple; - Datum datum; - Datum json; - - Assert(slot != NULL); - - tuple = ExecCopySlotHeapTuple(slot); - datum = heap_copy_tuple_as_datum(tuple, slot->tts_tupleDescriptor); - - json = DirectFunctionCall1(row_to_json, datum); - heap_freetuple(tuple); - - return json; -} - /* * tuple_table_slot_to_indextup_json * @@ -1124,7 +1096,7 @@ build_local_conflicts_tupledesc(void) * ConflictTupleInfo objects. * * Example output structure: - * [ { "xid": "1001", "commit_ts": "...", "origin": "...", "tuple": {...} }, ... ] + * [ { "xid": "1001", "commit_ts": "...", "origin": "..." }, ... ] */ static Datum build_local_conflicts_json_array(List *conflicttuples) @@ -1180,13 +1152,7 @@ build_local_conflicts_json_array(List *conflicttuples) else nulls[attno++] = true; - /* Convert conflicting tuple to JSON datum. */ - if (conflicttuple->slot) - values[attno] = tuple_table_slot_to_json_datum(conflicttuple->slot); - else - nulls[attno] = true; - - Assert(attno + 1 == NUM_LOCAL_CONFLICT_ATTRS); + Assert(attno == NUM_LOCAL_CONFLICT_ATTRS); tuple = heap_form_tuple(tupdesc, values, nulls); @@ -1231,8 +1197,7 @@ insert_conflict_log_tuple(EState *estate, Relation rel, Relation conflictlogrel, ConflictType conflict_type, TupleTableSlot *searchslot, - List *conflicttuples, - TupleTableSlot *remoteslot) + List *conflicttuples) { Datum values[NUM_CONFLICT_ATTRS] = {0}; bool nulls[NUM_CONFLICT_ATTRS] = {0}; @@ -1289,8 +1254,10 @@ insert_conflict_log_tuple(EState *estate, Relation rel, /* * If the table has a valid replica identity index, build the index - * JSON datum from key value. Otherwise, construct it from the - * complete tuple in REPLICA IDENTITY FULL cases. + * JSON datum from key value. Otherwise, in REPLICA IDENTITY FULL + * cases, set replica_identity_full to true and leave replica_identity + * NULL to avoid serializing full tuples that could exceed memory + * allocation limits. */ if (OidIsValid(replica_index)) { @@ -1302,7 +1269,7 @@ insert_conflict_log_tuple(EState *estate, Relation rel, else { values[attno++] = BoolGetDatum(true); - values[attno++] = tuple_table_slot_to_json_datum(searchslot); + nulls[attno++] = true; } } else @@ -1311,11 +1278,6 @@ insert_conflict_log_tuple(EState *estate, Relation rel, nulls[attno++] = true; } - if (!TupIsNull(remoteslot)) - values[attno++] = tuple_table_slot_to_json_datum(remoteslot); - else - nulls[attno++] = true; - /* * In update_missing and delete_missing conflicts, there are no local * conflicting rows, so set local_conflicts to NULL. diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 681dc66dca6..49026eb2cb7 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -704,9 +704,8 @@ WHERE s.subname = 'regress_conflict_test1' AND a.attnum > 0 8 | remote_origin 9 | replica_identity_full 10 | replica_identity - 11 | remote_tuple - 12 | local_conflicts -(12 rows) + 11 | local_conflicts +(11 rows) -- Changing the subscription owner should also update the owner -- of the associated conflict log table. diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index 8cecde2aa71..5b8eeeb83fc 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -339,6 +339,10 @@ my $clt_check_ba = $node_B->poll_query_until('postgres', "SELECT count(*) > 0 FROM $clt_BA WHERE conflict_type = 'delete_origin_differs';"); is($clt_check_ba, 1, 'delete_origin_differs logged into CLT on Node B'); +my $clt_row_ba = $node_B->safe_psql('postgres', + "SELECT replica_identity_full, replica_identity::text, (local_conflicts[1]->>'xid') IS NOT NULL FROM $clt_BA WHERE conflict_type = 'delete_origin_differs';"); +is($clt_row_ba, 'f|{"a":1}|t', 'delete_origin_differs records RI key columns and local conflict xid'); + $log_location = -s $node_A->logfile; $node_A->safe_psql('postgres', "ALTER SUBSCRIPTION $subname_AB ENABLE;"); @@ -359,6 +363,10 @@ my $clt_check_ab = $node_A->poll_query_until('postgres', "SELECT count(*) > 0 FROM $clt_AB WHERE conflict_type = 'update_deleted';"); is($clt_check_ab, 1, 'update_deleted logged into CLT on Node A'); +my $clt_row_ab = $node_A->safe_psql('postgres', + "SELECT replica_identity_full, replica_identity::text, (local_conflicts[1]->>'xid') IS NOT NULL FROM $clt_AB WHERE conflict_type = 'update_deleted';"); +is($clt_row_ab, 'f|{"a":1}|t', 'update_deleted records RI key columns and local conflict xid'); + # Remember the next transaction ID to be assigned my $next_xid = $node_A->safe_psql('postgres', "SELECT txid_current() + 1;"); @@ -406,6 +414,10 @@ like( .*The row to be updated was deleted locally in transaction [0-9]+ at .*/, 'update target row was deleted in tab'); +my $clt_row_ab_full = $node_A->safe_psql('postgres', + "SELECT replica_identity_full, replica_identity IS NULL, (local_conflicts[1]->>'xid') IS NOT NULL FROM $clt_AB WHERE replica_identity_full = true;"); +is($clt_row_ab_full, 't|t|t', 'update_deleted with REPLICA IDENTITY FULL sets replica_identity_full=true and replica_identity=NULL'); + ############################################################################### # Check that the xmin value of the conflict detection slot can be advanced when # the subscription has no tables. -- 2.49.0