Re: Deduplicate logicalrep_read_tuple()

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Deduplicate logicalrep_read_tuple()
Date: 2023-03-03 11:04:30
Message-ID: CAA4eK1KTPUw0f=9fzDr31i1qeLm9yTXp0abu5TPbFdjNM-zyyA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Mar 3, 2023 at 4:13 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> On Thu, Jan 19, 2023 at 8:36 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > On Wed, Jan 18, 2023 at 6:26 PM Bharath Rupireddy
> > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >
> > > Hi,
> > >
> > > logicalrep_read_tuple() duplicates code for LOGICALREP_COLUMN_TEXT and
> > > LOGICALREP_COLUMN_BINARY introduced by commit 9de77b5. While it
> > > doesn't hurt anyone, deduplication makes code a bit leaner by 57 bytes
> > > [1]. I've attached a patch for $SUBJECT.
> > >
> > > Thoughts?
> > >
> >
> > The code looks the same but there is a subtle comment difference where
> > previously only LOGICALREP_COLUMN_BINARY case said:
> > /* not strictly necessary but per StringInfo practice */
> >
> > So if you de-duplicate the code then should that comment be modified to say
> > /* not strictly necessary for LOGICALREP_COLUMN_BINARY but per
> > StringInfo practice */
>
> Thanks. Done so in the attached v2.
>

LGTM. Unless Peter or someone has any comments on this, I'll push this
early next week.

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2023-03-03 11:12:21 Re: Add documentation for coverage reports with meson
Previous Message John Naylor 2023-03-03 11:03:53 Re: [PoC] Improve dead tuple storage for lazy vacuum