Re: Remove redundant variable from transformCreateStmt - Mailing list pgsql-hackers

From Amul Sul
Subject Re: Remove redundant variable from transformCreateStmt
Date
Msg-id CAAJ_b940QUvZ8+1ZgTWSOJLEagz5euQZt6NERP+rvGfn_3z2xw@mail.gmail.com
Whole thread Raw
In response to Re: Remove redundant variable from transformCreateStmt  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
List pgsql-hackers
On Fri, Apr 30, 2021 at 10:49 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Fri, Apr 30, 2021 at 7:07 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > On Thu, Apr 29, 2021 at 02:39:42PM -0400, Alvaro Herrera wrote:
> > > On 2021-Apr-29, Tom Lane wrote:
> > > > Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> > > > > I'd do it like this.  Note I removed an if/else block in addition to
> > > > > your changes.
> > > >
> > > > > I couldn't convince myself that this is worth pushing though; either we
> > > > > push it to all branches (which seems unwarranted) or we create
> > > > > back-patching hazards.
> > > >
> > > > Yeah ... an advantage of the if/else coding is that it'd likely be
> > > > simple to extend to cover additional statement types, should we ever
> > > > wish to do that.  The rendering you have here is nice and compact,
> > > > but it would not scale up well.
> > >
> > > That makes sense.  But that part is not in Amul's patch -- he was only
> > > on about removing the is_foreign_table Boolean.  If I remove the if/else
> > > block change, does the rest of the patch looks something we'd want to
> > > have?  I kinda agree that the redundant variable is "ugly".  Is it worth
> > > removing?  My hunch is no.
> >
> > Getting rid of a redundant, boolean variable is good not because it's more
> > efficient but because it's one fewer LOC to read and maintain (and an
> > opportunity for inconsistency, I suppose).
>
> Yes.
>
> > Also, this is a roundabout and too-verbose way to invert a boolean:
> > | transformCheckConstraints(&cxt, !is_foreign_table ? true : false);
>
> I agree to remove only the redundant variable, is_foreign_table but
> not the if else block as Tom said: it's not scalable.

+1.

Regards,
Amul



pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: WIP: WAL prefetch (another approach)
Next
From: Bharath Rupireddy
Date:
Subject: Logical Replication - behavior of TRUNCATE ... CASCADE