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-22 14:41:06
Message-ID: 31150.1490193666@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I've been busily hacking away on this, trying to make things cleaner and
fix a couple of bugs I stumbled across. (Confusion between ExecQual and
ExecCheck, for instance - we apparently lack regression tests exercising
constraints-returning-null in corner cases such as table rewrite.) It
will probably be a day or two more before I'm done.

A couple of naming questions:

* I concur with Heikki's dislike for the file name execInterpExpr.c.
Would it make it any better to switch to execExprInterp.c? I think
that having all this stuff under a common pattern execExpr*.c would
be a good thing (and I notice you've already got one comment referring
to them that way ...)

* execQual.c doesn't seem to have a unifying reason to exist anymore.
It certainly has little to do with evaluating typical qual expressions;
what's left in there seems to be mostly concerned with SRFs. I feel
like it might be a good idea to rename it, but to what? execExprUtils.c
perhaps? Or maybe we should destroy it altogether, shoving the SRF
stuff into nodeFunctionscan.c and moving what little remains into
execUtils.c.

* I do not like the function name ExecInstantiateExpr(). Webster's
defines "instantiate" as "to represent (an abstraction) by a concrete
instance", which does not seem to me to have a lot to do with what this
function actually does. There's nothing very abstract about its input.
More, the implication of "instantiate" is that you can instantiate any
number of representatives of the same abstraction, but this scribbles
on the input in a one-way fashion. I think perhaps something like
ExecPrepareExpr or ExecFinishExpr or something along that line would
be better, but nothing is really standing out as le mot juste.

Thoughts?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2017-03-22 14:41:27 Re: increasing the default WAL segment size
Previous Message Simon Riggs 2017-03-22 14:39:57 Re: Patch to improve performance of replay of AccessExclusiveLock