Re: Proposal: Conflict log history table for Logical Replication - Mailing list pgsql-hackers

From Peter Smith
Subject Re: Proposal: Conflict log history table for Logical Replication
Date
Msg-id CAHut+PuaqNDfDu_3xkZR4OYxw-B7ew_WjpLXCBvMcSBJz2K6Xg@mail.gmail.com
Whole thread Raw
In response to Re: Proposal: Conflict log history table for Logical Replication  (Dilip Kumar <dilipbalaut@gmail.com>)
Responses Re: Proposal: Conflict log history table for Logical Replication
List pgsql-hackers
Hi Dilip.

Some comments for the first 2 patches:

//////////
v24-00001
//////////

1.
+ /*
+ * Conflict log tables are managed by the system to record logical
+ * replication conflicts.  We do not allow locking rows in CONFLICT
+ * relations.
+ */
+ if (IsConflictNamespace(RelationGetNamespace(rel)))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in CONFLICT relation \"%s\"",
+ RelationGetRelationName(rel))));

AFAIK, this "CONFLICT relation" terminology is not used anywhere else.

Why not just call it what it is:

e.g.
cannot lock rows in conflict log table \"%s\"

~

OTOH, if you were attempting to future-proof the message for different
kinds of relations in the 'pg_conflict' namespace, I still felt it
might be better to refer to 'pg_conflict' instead of CONFLICT:

e.g.
cannot lock rows in 'pg_conflict' relation \"%s\"

//////////
v24-0002
//////////

1.
+static char *build_index_value_desc(EState *estate, Relation localrel,
+ TupleTableSlot *slot, Oid indexoid);

Declared twice?

======
Kind Regards,
Peter Smith.
Fujitsu Australia.



pgsql-hackers by date:

Previous
From: Amit Langote
Date:
Subject: Re: Batching in executor
Next
From: Ashutosh Bapat
Date:
Subject: Re: Remove PG_MMAP_FLAGS