On Thu, Apr 29, 2021 at 02:39:42PM -0400, Alvaro Herrera wrote:
> On 2021-Apr-29, Tom Lane wrote:
> > Alvaro Herrera <alvhe...@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).

Also, this is a roundabout and too-verbose way to invert a boolean:
| transformCheckConstraints(&cxt, !is_foreign_table ? true : false);

-- 
Justin

PS. It's also not pythonic ;)


Reply via email to