Re: Deduplicate logicalrep_read_tuple()

On Fri, Mar 3, 2023 at 10:04 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 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.
>

No more comments. Patch v2 LGTM.

------
Kind Regards,
Peter Smith.
Fujitsu Australia