Re: WIP: Faster Expression Processing v4

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, pgsql-hackers@postgresql.org
Date: 2017-03-22T14:52:40Z
Lists: pgsql-hackers
On 2017-03-22 10:41:06 -0400, Tom Lane wrote:
> 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.

Thanks!


> 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 ...)

That works for me.


> * 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.

Yea, I was wondering about that too.  What would we do with
GetAttributeByName/Num?


> * 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.

It perhaps makes a *bit* more sense if you view it from the POV that,
with the future JIT support (WIP versions of which I posted previously),
it'd actually create a compiled function which'd largely be independent
of the of ->steps (except for the non-hotpath functions, which'd still
end up using it).  So one of the above "conrete instances" would be the
interpeted version, another the compiled one.   No, not an entirely
convincing argument.


> 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.

Either of those work, but they don't strike me as perfect either, but I
can't come up with something better (ExecReadyExprForExec()?).

Greetings,

Andres Freund


Commits

  1. Improve performance of ExecEvalWholeRowVar.

  2. Remove unreachable code in expression evaluation.

  3. Faster expression evaluation and targetlist projection.

  4. Avoid syntax error on platforms that have neither LOCALE_T nor ICU.

  5. Add configure test to see if the C compiler has gcc-style computed gotos.

  6. Improve regression test coverage for TID scanning.

  7. Improve expression evaluation test coverage.

  8. Fix two errors with nested CASE/WHEN constructs.