From: | Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> |
---|---|
To: | Euler Taveira <euler(at)eulerto(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Subject: | Re: logicalrep_message_type throws an error |
Date: | 2023-07-05 14:24:36 |
Message-ID: | CAExHW5sR_sQ4wVB9+_8G3bPDd0q1JuM5xybsOcudaOvM=p+--w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jul 5, 2023 at 7:07 PM Euler Taveira <euler(at)eulerto(dot)com> 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
> > message type "X"" from apply_dispatch(), right? If so, then what about
> > the case when we forgot to handle some message in
> > logicalrep_message_type() but handled it in apply_dispatch()?
apply_dispatch() has a default case in switch() so it can
theoretically forget to handle some message type. I think we should
avoid default case in that function to catch missing message type in
that function. But if logicalrep_message_type() doesn't use default
case, it won't forget to handle a known message type. So what you are
suggesting is not possible.
It might happen that the upstream may send an unknown message type
that both apply_dispatch() and logicalrep_message_type() can not
handle.
> ERROR: invalid logical replication message type "X"
> CONTEXT: processing remote data for replication origin "pg_16638" during message type "??? (88)" in transaction 796, finished at 0/1626698
>
> IMO it could be confusing if we provide two representations of the same data (X
> and 88). Since we already provide "X" I don't think we need "88". Another
> option, is to remove "X" from apply_dispatch() and add "??? (88)" to
> logicalrep_message_type().
I think we don't need message type to be mentioned in the context for
an error about invalid message type. I think what needs to be done is
to set
apply_error_callback_arg.command = 0 before calling ereport in the
default case of apply_dispatch().
apply_error_callback() will just return without providing a context.
If we need a context and have all the other necessary fields, we can
improve apply_error_callback() to provide context when
apply_error_callback_arg.command = 0 .
--
Best Wishes,
Ashutosh Bapat
From | Date | Subject | |
---|---|---|---|
Next Message | Andrey Lepikhov | 2023-07-05 14:28:59 | Re: Removing unneeded self joins |
Previous Message | Greg Sabino Mullane | 2023-07-05 14:14:22 | Re: Prevent psql \watch from running queries that return no rows |