Re: logicalrep_message_type throws an error

2023-07-25 Thread Amit Kapila
On Mon, Jul 24, 2023 at 6:14 PM Ashutosh Bapat wrote: > > On Sat, Jul 22, 2023 at 10:18 AM Amit Kapila wrote: > > > Right. I'll push and backpatch this till 15 by Tuesday unless you guys > > think otherwise. > > WFM. > Pushed. -- With Regards, Amit Kapila.

Re: logicalrep_message_type throws an error

2023-07-24 Thread Ashutosh Bapat
On Sat, Jul 22, 2023 at 10:18 AM Amit Kapila wrote: > > > > I think it could confuse us if an invalid message is not a printable > > character. > > > That's a good point. > Right. I'll push and backpatch this till 15 by Tuesday unless you guys > think otherwise. WFM. -- Best Wishes,

Re: logicalrep_message_type throws an error

2023-07-21 Thread Amit Kapila
On Fri, Jul 21, 2023 at 6:28 AM Masahiko Sawada wrote: > > On Fri, Jul 21, 2023 at 1:39 AM Ashutosh Bapat > wrote: > > > > On Thu, Jul 20, 2023 at 9:11 AM Amit Kapila wrote: > > > > > > > > Okay, changed it accordingly. > > > > > > > > > > oops, forgot to attach the patch. > > > > > > > WFM > >

Re: logicalrep_message_type throws an error

2023-07-20 Thread Masahiko Sawada
On Fri, Jul 21, 2023 at 1:39 AM Ashutosh Bapat wrote: > > On Thu, Jul 20, 2023 at 9:11 AM Amit Kapila wrote: > > > > > > Okay, changed it accordingly. > > > > > > > oops, forgot to attach the patch. > > > > WFM > > @@ -3367,7 +3367,7 @@ apply_dispatch(StringInfo s) > default: >

Re: logicalrep_message_type throws an error

2023-07-20 Thread Ashutosh Bapat
On Thu, Jul 20, 2023 at 9:11 AM Amit Kapila wrote: > > > > Okay, changed it accordingly. > > > > oops, forgot to attach the patch. > WFM @@ -3367,7 +3367,7 @@ apply_dispatch(StringInfo s) default: ereport(ERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), -

Re: logicalrep_message_type throws an error

2023-07-19 Thread Amit Kapila
On Thu, Jul 20, 2023 at 9:10 AM Amit Kapila wrote: > > On Wed, Jul 19, 2023 at 10:08 AM Ashutosh Bapat > wrote: > > > > On Wed, Jul 19, 2023 at 9:01 AM Amit Kapila wrote: > > > > > > On Tue, Jul 18, 2023 at 10:27 AM Masahiko Sawada > > > wrote: > > > > > > > > Or can we use snprintf() writing

Re: logicalrep_message_type throws an error

2023-07-19 Thread Amit Kapila
On Wed, Jul 19, 2023 at 10:08 AM Ashutosh Bapat wrote: > > On Wed, Jul 19, 2023 at 9:01 AM Amit Kapila wrote: > > > > On Tue, Jul 18, 2023 at 10:27 AM Masahiko Sawada > > wrote: > > > > > > Or can we use snprintf() writing "??? (%d)" to a fixed length char[8 + > > > 11] allocated on the stack

Re: logicalrep_message_type throws an error

2023-07-18 Thread Ashutosh Bapat
On Wed, Jul 19, 2023 at 9:01 AM Amit Kapila wrote: > > On Tue, Jul 18, 2023 at 10:27 AM Masahiko Sawada > wrote: > > > > On Tue, Jul 18, 2023 at 12:15 PM Amit Kapila > > wrote: > > > > > > On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera > > > wrote: > > > > > > > > > > I have tried to check

Re: logicalrep_message_type throws an error

2023-07-18 Thread Amit Kapila
On Tue, Jul 18, 2023 at 10:27 AM Masahiko Sawada wrote: > > On Tue, Jul 18, 2023 at 12:15 PM Amit Kapila wrote: > > > > On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera > > wrote: > > > > > > > I have tried to check whether we have such usage in any other error > > callbacks. Though I haven't

Re: logicalrep_message_type throws an error

2023-07-17 Thread Masahiko Sawada
On Tue, Jul 18, 2023 at 12:15 PM Amit Kapila wrote: > > On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera > wrote: > > > > On 2023-Jul-17, Ashutosh Bapat wrote: > > > > > Prologue of psprintf() says > > > > > > * Errors are not returned to the caller, but are reported via elog(ERROR) > > > * in

Re: logicalrep_message_type throws an error

2023-07-17 Thread Amit Kapila
On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera wrote: > > On 2023-Jul-17, Ashutosh Bapat wrote: > > > Prologue of psprintf() says > > > > * Errors are not returned to the caller, but are reported via elog(ERROR) > > * in the backend, or printf-to-stderr-and-exit() in frontend builds. > > * One

Re: logicalrep_message_type throws an error

2023-07-17 Thread Alvaro Herrera
On 2023-Jul-17, Ashutosh Bapat wrote: > Prologue of psprintf() says > > * Errors are not returned to the caller, but are reported via elog(ERROR) > * in the backend, or printf-to-stderr-and-exit() in frontend builds. > * One should therefore think twice about using this in libpq. > > If an

Re: logicalrep_message_type throws an error

2023-07-17 Thread Ashutosh Bapat
On Sat, Jul 15, 2023 at 12:57 PM Amit Kapila wrote: > > > > Since the numerical value is important only in invalid message type > > cases, how about using a format like "??? (88)" in unknown message > > type cases, in both error and context messages? > > > > Do you have something like attached in

Re: logicalrep_message_type throws an error

2023-07-16 Thread Amit Kapila
On Sat, Jul 15, 2023 at 7:16 PM Euler Taveira wrote: > > On Sat, Jul 15, 2023, at 4:27 AM, Amit Kapila wrote: > > Do you have something like attached in mind? > > > WFM. I would change the comment that says > > This function is called to provide context in the error ... > > to > > This message

Re: logicalrep_message_type throws an error

2023-07-15 Thread Euler Taveira
On Sat, Jul 15, 2023, at 4:27 AM, Amit Kapila wrote: > Do you have something like attached in mind? WFM. I would change the comment that says This function is called to provide context in the error ... to This message provides context in the error ... because this comment is not at the

Re: logicalrep_message_type throws an error

2023-07-15 Thread Amit Kapila
On Tue, Jul 11, 2023 at 1:36 PM Masahiko Sawada wrote: > > On Thu, Jul 6, 2023 at 6:28 PM Amit Kapila wrote: > > > > One point to note is that the user may also get confused if the actual > > ERROR says message type as 'X' and the context says '???'. I feel in > > this case duplicate information

Re: logicalrep_message_type throws an error

2023-07-11 Thread Alvaro Herrera
On 2023-Jul-11, Masahiko Sawada wrote: > Since the numerical value is important only in invalid message type > cases, how about using a format like "??? (88)" in unknown message > type cases, in both error and context messages? +1 -- Álvaro Herrera 48°01'N 7°57'E —

Re: logicalrep_message_type throws an error

2023-07-11 Thread Masahiko Sawada
On Thu, Jul 6, 2023 at 6:28 PM Amit Kapila wrote: > > One point to note is that the user may also get confused if the actual > ERROR says message type as 'X' and the context says '???'. I feel in > this case duplicate information is better than different information. I agree. I think it would be

Re: logicalrep_message_type throws an error

2023-07-06 Thread Amit Kapila
On Wed, Jul 5, 2023 at 7:54 PM Ashutosh Bapat wrote: > > On Wed, Jul 5, 2023 at 7:07 PM Euler Taveira wrote: > > > > On Wed, Jul 5, 2023, at 7:56 AM, Alvaro Herrera wrote: > > > > On 2023-Jul-05, Amit Kapila wrote: > > > > > I think after returning "???" from logicalrep_message_type(), the > > >

Re: logicalrep_message_type throws an error

2023-07-06 Thread Amit Kapila
On Wed, Jul 5, 2023 at 10:45 PM Alvaro Herrera wrote: > > On 2023-Jul-05, Alvaro Herrera wrote: > > > On 2023-Jul-05, Euler Taveira wrote: > > > > > Isn't this numerical value already exposed in the error message (X = 88)? > > > In this example, it is: > > > > > > ERROR: invalid logical

Re: logicalrep_message_type throws an error

2023-07-05 Thread Alvaro Herrera
On 2023-Jul-05, Alvaro Herrera wrote: > On 2023-Jul-05, Euler Taveira wrote: > > > Isn't this numerical value already exposed in the error message (X = 88)? > > In this example, it is: > > > > ERROR: invalid logical replication message type "X" > > CONTEXT: processing remote data for

Re: logicalrep_message_type throws an error

2023-07-05 Thread Alvaro Herrera
On 2023-Jul-05, Euler Taveira wrote: > Isn't this numerical value already exposed in the error message (X = 88)? > In this example, it is: > > ERROR: invalid logical replication message type "X" > CONTEXT: processing remote data for replication origin "pg_16638" during > message type "???

Re: logicalrep_message_type throws an error

2023-07-05 Thread Ashutosh Bapat
On Wed, Jul 5, 2023 at 7:07 PM Euler Taveira wrote: > > On Wed, Jul 5, 2023, at 7:56 AM, Alvaro Herrera wrote: > > On 2023-Jul-05, Amit Kapila wrote: > > > I think after returning "???" from logicalrep_message_type(), the > > above is possible when we get the error: "invalid logical replication >

Re: logicalrep_message_type throws an error

2023-07-05 Thread Euler Taveira
On Wed, Jul 5, 2023, at 7:56 AM, Alvaro Herrera wrote: > On 2023-Jul-05, Amit Kapila wrote: > > > I think after returning "???" from logicalrep_message_type(), the > > above is possible when we get the error: "invalid logical replication > > message type "X"" from apply_dispatch(), right? If so,

Re: logicalrep_message_type throws an error

2023-07-05 Thread Amit Kapila
On Wed, Jul 5, 2023 at 4:26 PM Alvaro Herrera wrote: > > On 2023-Jul-05, Amit Kapila wrote: > > > I think after returning "???" from logicalrep_message_type(), the > > above is possible when we get the error: "invalid logical replication > > message type "X"" from apply_dispatch(), right? If so,

Re: logicalrep_message_type throws an error

2023-07-05 Thread Alvaro Herrera
On 2023-Jul-05, Amit Kapila wrote: > I think after returning "???" from logicalrep_message_type(), the > above is possible when we get the error: "invalid logical replication > message type "X"" from apply_dispatch(), right? If so, then what about > the case when we forgot to handle some message

Re: logicalrep_message_type throws an error

2023-07-04 Thread Amit Kapila
On Mon, Jul 3, 2023 at 6:32 PM Euler Taveira wrote: > > 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

Re: logicalrep_message_type throws an error

2023-07-03 Thread Euler Taveira
On Mon, Jul 3, 2023, at 10:57 AM, Ashutosh Bapat wrote: > On Mon, Jul 3, 2023 at 6:52 PM Ashutosh Bapat > wrote: > > > > The switch is on action which is an enum. So without default it will > > provide a compilation warning for missing enums. Adding "default" case > > defeats that purpose. I

Re: logicalrep_message_type throws an error

2023-07-03 Thread Ashutosh Bapat
On Mon, Jul 3, 2023 at 6:52 PM Ashutosh Bapat wrote: > > Thanks Euler for the patch. > > On Mon, Jul 3, 2023 at 6:32 PM Euler Taveira wrote: > > > > Masahiko, since abc0910e2e0 is your patch maybe you want to take a look at > > it. > > > > A couple of comments. > > -char * > +const char * > >

Re: logicalrep_message_type throws an error

2023-07-03 Thread Ashutosh Bapat
Thanks Euler for the patch. On Mon, Jul 3, 2023 at 6:32 PM Euler Taveira wrote: > > Masahiko, since abc0910e2e0 is your patch maybe you want to take a look at it. > A couple of comments. -char * +const char * Nice improvement. logicalrep_message_type(LogicalRepMsgType action) { switch

Re: logicalrep_message_type throws an error

2023-07-03 Thread Euler Taveira
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

logicalrep_message_type throws an error

2023-07-03 Thread Ashutosh Bapat
Hi All, 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