From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Heikki Linnakangas <hlinnaka@iki.fi>,
        Tomas Vondra <tomas.vondra@2ndquadrant.com>,
        pgsql-hackers@postgresql.org
Subject: Re: WIP: Faster Expression Processing v4
In-reply-to: <20170325221142.n6exw2jsubo4ywvg@alap3.anarazel.de>
References: <8011.1490305255@sss.pgh.pa.us> <26088.1490315792@sss.pgh.pa.us>
 <20170324004734.3a2dokbfofiz6ylu@alap3.anarazel.de>
 <30270.1490318779@sss.pgh.pa.us>
 <20170324013412.qy7ltnnqv5m7qiuc@alap3.anarazel.de>
 <31825.1490320683@sss.pgh.pa.us>
 <20170324020003.4rt63xty7xpb4akq@alap3.anarazel.de>
 <22215.1490369187@sss.pgh.pa.us>
 <20170324182742.3r4qndpugtx6ga3t@alap3.anarazel.de>
 <22833.1490390175@sss.pgh.pa.us>
 <20170325221142.n6exw2jsubo4ywvg@alap3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Sat, 25 Mar 2017 15:11:42 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4833.1490480275.1@sss.pgh.pa.us>
Date: Sat, 25 Mar 2017 18:17:55 -0400
Message-ID: <4834.1490480275@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Andres Freund <andres@anarazel.de> writes:
> - The !caseExpr->defresult result branch is currently unreachable (and
>   its equivalent was before the patch) because transformCaseExpr()
>   generates a default expression.  I'm inclined to replace the dead code
>   with an assertion. Any reason not to do that?

Ah-hah.  I'd noted that that code wasn't being reached when I did some
code coverage checks this morning, but I hadn't found the cause yet.
Yeah, replacing the if-test with an assert seems fine.

> I think there's some argument to be made that we should move all of
> execSRF.c's logic to their respective callsites.  There's not really
> much shared code: ExecEvalFuncArgs(), tupledesc_match(), init_sexpr() -
> and at least the latter would even become a bit simpler if we didn't
> support both callers.

Possibly.  All that code could stand to be rethought, probably, now that
its mission is just to handle the SRF case.  But at least all the cruft is
in one place for the moment.  And I don't think it's anything we have to
fix before v10, it's just cosmetic.

> Thanks a lot for your work on the patch!

You're welcome!

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers