| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
|---|---|
| To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Deduplicate logicalrep_read_tuple() |
| Date: | 2023-03-03 10:43:23 |
| Message-ID: | CALj2ACV1N+_HvTVxWmCe5qBd3mUR0rWr-Q_7mBLipdz62O19NQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Deduplicate-logicalrep_read_tuple.patch | application/octet-stream | 1.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alvaro Herrera | 2023-03-03 10:47:28 | Re: cataloguing NOT NULL constraints |
| Previous Message | David Rowley | 2023-03-03 10:41:46 | Re: min/max aggregation for jsonb |