Re: WIP: Faster Expression Processing v4

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: WIP: Faster Expression Processing v4
Date: 2017-03-25 22:17:55
Message-ID: 4834.1490480275@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2017-03-25 22:22:02 Re: WIP: Faster Expression Processing v4
Previous Message Andres Freund 2017-03-25 22:11:42 Re: WIP: Faster Expression Processing v4