On Mon, Jul 3, 2023, at 7:30 AM, Ashutosh Bapat wrote: > logicalrep_message_type() is used to convert logical message type code > into name while prepared error context or details. Thus when this > function is called probably an ERROR is already raised. If > logicalrep_message_type() gets an unknown message type, it will throw > an error, which will suppress the error for which we are building > context or details. That's not useful. I think instead > logicalrep_message_type() should return "unknown" when it encounters > an unknown message type and let the original error message be thrown > as is.
Hmm. Good catch. The current behavior is: ERROR: invalid logical replication message type "X" LOG: background worker "logical replication worker" (PID 71800) exited with exit code 1 ... that hides the details. After providing a default message type: ERROR: invalid logical replication message type "X" CONTEXT: processing remote data for replication origin "pg_16638" during message type "???" in transaction 796, finished at 0/16266F8 Masahiko, since abc0910e2e0 is your patch maybe you want to take a look at it. -- Euler Taveira EDB https://www.enterprisedb.com/
From aaeb2d7474a30a363b69441397b2d7dd91bfba30 Mon Sep 17 00:00:00 2001 From: Euler Taveira <euler.tave...@enterprisedb.com> Date: Mon, 3 Jul 2023 08:54:10 -0300 Subject: [PATCH] uncover logical change details The commit abc0910e2e0 adds logical change details to error context. However, the function logicalrep_message_type() introduces an elog(ERROR) that can hide these details. Instead, avoid elog() and use ??? (that is a synonym for unknown). Spotted by Ashutosh Bapat. Discussion: https://postgr.es/m/CAExHW5suAEDW-mBZt_qu4RVxWZ1vL54-L%2Bci2zreYWebpzxYsA%40mail.gmail.com --- src/backend/replication/logical/proto.c | 8 +++----- src/include/replication/logicalproto.h | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c index f308713275..572ef0a1aa 100644 --- a/src/backend/replication/logical/proto.c +++ b/src/backend/replication/logical/proto.c @@ -1213,7 +1213,7 @@ logicalrep_read_stream_abort(StringInfo in, /* * Get string representing LogicalRepMsgType. */ -char * +const char * logicalrep_message_type(LogicalRepMsgType action) { switch (action) @@ -1256,9 +1256,7 @@ logicalrep_message_type(LogicalRepMsgType action) return "STREAM ABORT"; case LOGICAL_REP_MSG_STREAM_PREPARE: return "STREAM PREPARE"; + default: + return "???"; } - - elog(ERROR, "invalid logical replication message type \"%c\"", action); - - return NULL; /* keep compiler quiet */ } diff --git a/src/include/replication/logicalproto.h b/src/include/replication/logicalproto.h index 0ea2df5088..c5be981eae 100644 --- a/src/include/replication/logicalproto.h +++ b/src/include/replication/logicalproto.h @@ -269,6 +269,6 @@ extern void logicalrep_write_stream_abort(StringInfo out, TransactionId xid, extern void logicalrep_read_stream_abort(StringInfo in, LogicalRepStreamAbortData *abort_data, bool read_abort_info); -extern char *logicalrep_message_type(LogicalRepMsgType action); +extern const char *logicalrep_message_type(LogicalRepMsgType action); #endif /* LOGICAL_PROTO_H */ -- 2.30.2
