On Thu, Nov 26, 2020 at 10:15 AM Amit Kapila
wrote:
> On Wed, Nov 25, 2020 at 2:52 PM Amit Kapila
> wrote:
> >
> > On Wed, Nov 25, 2020 at 2:26 PM Peter Smith
> wrote:
> > >
> > > Hi Hackers.
> > >
> > > Last month there was a commit [1] for replacing logical replication
> > > message type char
On Wed, Nov 25, 2020 at 2:52 PM Amit Kapila wrote:
>
> On Wed, Nov 25, 2020 at 2:26 PM Peter Smith wrote:
> >
> > Hi Hackers.
> >
> > Last month there was a commit [1] for replacing logical replication
> > message type characters with enums of equivalent values.
> >
> > I was revisiting this code
On Wed, Nov 25, 2020 at 2:26 PM Peter Smith wrote:
>
> Hi Hackers.
>
> Last month there was a commit [1] for replacing logical replication
> message type characters with enums of equivalent values.
>
> I was revisiting this code recently and I think due to oversight that
> initial patch was incomp
Hi Hackers.
Last month there was a commit [1] for replacing logical replication
message type characters with enums of equivalent values.
I was revisiting this code recently and I think due to oversight that
initial patch was incomplete. IIUC there are several more enum
substitutions which should
Thanks Amit.
On Mon, 2 Nov 2020 at 14:15, Amit Kapila wrote:
> On Fri, Oct 30, 2020 at 5:52 PM Ashutosh Bapat
> wrote:
> >
> > On Fri, 30 Oct 2020 at 17:37, Amit Kapila
> wrote:
> >>
> >>
> >>
> >> Other than that the patch looks good to me.
> >>
> >
> > Patch with updated commit message and a
On Fri, Oct 30, 2020 at 5:52 PM Ashutosh Bapat
wrote:
>
> On Fri, 30 Oct 2020 at 17:37, Amit Kapila wrote:
>>
>>
>>
>> Other than that the patch looks good to me.
>>
>
> Patch with updated commit message and also the list of reviewers
>
Thanks, pushed!
--
With Regards,
Amit Kapila.
On Fri, 30 Oct 2020 at 17:37, Amit Kapila wrote:
>
> I don't like the word 'Enumize' in commit message. How about changing
> it to something like: (a) Add defines for logical replication protocol
> messages, or (b) Associate names with logical replication protocol
> messages.
>
I have used "Use
On Fri, Oct 30, 2020 at 5:05 PM Ashutosh Bapat
wrote:
>
>
>
> On Fri, 30 Oct 2020 at 09:16, Amit Kapila wrote
>>
>> I think we can simply use 'return apply_handle_begin;' instead of
>> adding return in another line. Again, I think we changed this handling
>> in apply_dispatch() to improve the cas
On Fri, 30 Oct 2020 at 14:59, Amit Kapila wrote:
> On Fri, Oct 30, 2020 at 11:50 AM Amit Kapila
> wrote:
> >
> > On Fri, Oct 30, 2020 at 10:37 AM Peter Smith
> wrote:
> > >
> > > IIUC getting rid of the default from the switch can make the missing
> > > enum detection easier because then you ca
On Fri, 30 Oct 2020 at 09:16, Amit Kapila wrote
>
> 1.
> + LOGICAL_REP_MSG_STREAM_ABORT = 'A',
> +} LogicalRepMsgType;
>
> There is no need for a comma after the last message.
>
> Done. Thanks for noticing it.
> 2.
> +/*
> + * Logical message types
> + *
> + * Used by logical replication wire p
On Fri, Oct 30, 2020 at 11:50 AM Amit Kapila wrote:
>
> On Fri, Oct 30, 2020 at 10:37 AM Peter Smith wrote:
> >
> > IIUC getting rid of the default from the switch can make the missing
> > enum detection easier because then you can use -Wswitch option to
> > expose the problem (instead of -Wswitc
On Fri, Oct 30, 2020 at 10:37 AM Peter Smith wrote:
>
> On Fri, Oct 30, 2020 at 2:46 PM Amit Kapila wrote:
>
> Hi Amit
>
> > You mentioned in the beginning that you prefer to use Enum instead of
> > define so that switch cases can detect any remaining items but I have
> > tried adding extra enum
On Fri, Oct 30, 2020 at 2:46 PM Amit Kapila wrote:
Hi Amit
> You mentioned in the beginning that you prefer to use Enum instead of
> define so that switch cases can detect any remaining items but I have
> tried adding extra enum value at the end and didn't handle that in
> switch case but I didn
On Fri, Oct 23, 2020 at 6:26 PM Ashutosh Bapat
wrote:
>
>
>
> On Fri, 23 Oct 2020 at 18:23, Amit Kapila wrote:
>>
>> On Fri, Oct 23, 2020 at 11:50 AM Kyotaro Horiguchi
>> wrote:
>> >
>> > At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera
>> > wrote in
>> > > On 2020-Oct-22, Ashutosh Bapat wro
On Fri, 23 Oct 2020 at 18:23, Amit Kapila wrote:
> On Fri, Oct 23, 2020 at 11:50 AM Kyotaro Horiguchi
> wrote:
> >
> > At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera <
> alvhe...@alvh.no-ip.org> wrote in
> > > On 2020-Oct-22, Ashutosh Bapat wrote:
> > >
> > > > On Thu, 22 Oct 2020 at 14:46,
On Fri, 23 Oct 2020 at 17:02, Kyotaro Horiguchi
wrote:
> At Fri, 23 Oct 2020 19:53:00 +1100, Peter Smith
> wrote in
> > On Fri, Oct 23, 2020 at 5:20 PM Kyotaro Horiguchi
> > wrote:
> > >
> > > At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera <
> alvhe...@alvh.no-ip.org> wrote in
> > > > On 20
On Fri, Oct 23, 2020 at 11:50 AM Kyotaro Horiguchi
wrote:
>
> At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera
> wrote in
> > On 2020-Oct-22, Ashutosh Bapat wrote:
> >
> > > On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi
> > > wrote:
> >
> > > > pg_send_logicalrep_msg_type() looks somewhat t
On Fri, 23 Oct 2020 at 06:50, Kyotaro Horiguchi
wrote:
>
> Those two switch()es are apparently redundant. That code is exactly
> equivalent to:
>
> apply_dispatch(s)
> {
> LogicalRepMsgType msgtype = pq_getmsgtype(s);
>
> switch (msgtype)
> {
> case LOGICAL_REP_MSG_BEGIN:
> app
At Fri, 23 Oct 2020 19:53:00 +1100, Peter Smith wrote
in
> On Fri, Oct 23, 2020 at 5:20 PM Kyotaro Horiguchi
> wrote:
> >
> > At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera
> > wrote in
> > > On 2020-Oct-22, Ashutosh Bapat wrote:
> > >
> > > > On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguc
On Fri, Oct 23, 2020 at 5:20 PM Kyotaro Horiguchi
wrote:
>
> At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera
> wrote in
> > On 2020-Oct-22, Ashutosh Bapat wrote:
> >
> > > On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi
> > > wrote:
> >
> > > > pg_send_logicalrep_msg_type() looks somewhat to
At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera
wrote in
> On 2020-Oct-22, Ashutosh Bapat wrote:
>
> > On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi
> > wrote:
>
> > > pg_send_logicalrep_msg_type() looks somewhat too-much. If we need
> > > something like that we shouldn't do this refact
On 2020-Oct-22, Ashutosh Bapat wrote:
> On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi
> wrote:
> > pg_send_logicalrep_msg_type() looks somewhat too-much. If we need
> > something like that we shouldn't do this refactoring, I think.
>
> Enum is an integer, and we want to send byte. The functi
At Fri, 23 Oct 2020 10:08:44 +0900 (JST), Kyotaro Horiguchi
wrote in
> At Thu, 22 Oct 2020 16:37:18 +0530, Ashutosh Bapat
> wrote in
> > On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi
> > wrote:
> > pg_get_logicalrep_msg_type() seems doing the same check (that the
> > > value is compared ag
At Thu, 22 Oct 2020 16:37:18 +0530, Ashutosh Bapat
wrote in
> On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi
> wrote:
>
> >
> >
> > We shouldn't have the default: in the switch() block in
> > apply_dispatch(). That prevents compilers from checking
> > completeness. The content of the default:
On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi
wrote:
>
>
> We shouldn't have the default: in the switch() block in
> apply_dispatch(). That prevents compilers from checking
> completeness. The content of the default: should be moved out to after
> the switch() block.
>
> apply_dispatch()
> {
>
At Thu, 22 Oct 2020 12:13:40 +0530, Ashutosh Bapat
wrote in
> Thanks Andres for your review. Thanks Li, Horiguchi-san and Amit for your
> comments.
>
> On Tue, 20 Oct 2020 at 04:57, Andres Freund wrote:
>
> > Hi,
> >
> > On 2020-10-16 12:55:26 +0530, Ashutosh Bapat wrote:
> > > Here's a patch
Thanks Andres for your review. Thanks Li, Horiguchi-san and Amit for your
comments.
On Tue, 20 Oct 2020 at 04:57, Andres Freund wrote:
> Hi,
>
> On 2020-10-16 12:55:26 +0530, Ashutosh Bapat wrote:
> > Here's a patch simplifying that for top level logical replication
> > messages.
>
> I think tha
Hi,
On 2020-10-16 12:55:26 +0530, Ashutosh Bapat wrote:
> Here's a patch simplifying that for top level logical replication
> messages.
I think that's a good plan. One big benefit for me is that it's much
easier to search for an enum than for a single letter
constant. Including searching for all
On Fri, Oct 16, 2020 at 12:55 PM Ashutosh Bapat
wrote:
>
> Hi All,
> Logical replication protocol uses single byte character to identify
> different chunks of logical repliation messages. The code uses
> character literals for the same. These literals are used as bare
> constants in code as well.
On Fri, 16 Oct 2020 at 14:06, Kyotaro Horiguchi
wrote:
> At Fri, 16 Oct 2020 08:08:40 +, Li Japin wrote
> in
> >
> > > On Oct 16, 2020, at 3:25 PM, Ashutosh Bapat <
> ashutosh.bapat@gmail.com> wrote:
>
> >
> > What about ’N’ for new tuples, ‘O’ for old tuple follows, ‘K’ for old
> key fo
At Fri, 16 Oct 2020 08:08:40 +, Li Japin wrote in
>
> > On Oct 16, 2020, at 3:25 PM, Ashutosh Bapat
> > wrote:
> >
> > Hi All,
> > Logical replication protocol uses single byte character to identify
> > different chunks of logical repliation messages. The code uses
> > character literals
> On Oct 16, 2020, at 3:25 PM, Ashutosh Bapat
> wrote:
>
> Hi All,
> Logical replication protocol uses single byte character to identify
> different chunks of logical repliation messages. The code uses
> character literals for the same. These literals are used as bare
> constants in code as wel
32 matches
Mail list logo
