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
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
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
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
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
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
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
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
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
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
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
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
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:
>
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
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
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
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
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
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
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
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
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
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
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
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. That's true for almost all the code that
deals with wire protocol.
32 matches
Mail list logo
