Date: Wed, 22 Mar 2017 07:52:40 -0700
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
Subject: Re: WIP: Faster Expression Processing v4
Message-ID: <20170322145240.vt2iolwt6vpmx33z@alap3.anarazel.de>
References: <20170307030137.lpo4japv5oqgbqpz@alap3.anarazel.de>
 <2090fd00-4dec-7b42-f700-64308958504c@2ndquadrant.com>
 <20170313080351.4g5oubxcj4xj5y7t@alap3.anarazel.de>
 <fcdaa51b-e905-72e0-39dd-5e22f535a253@2ndquadrant.com>
 <20170314065259.ffef4tfhgbsaieoe@alap3.anarazel.de>
 <69b7bb0e-8dab-3d39-1a89-3c6ab6409f64@iki.fi>
 <20170314224706.nxvtapenky6eom3z@alap3.anarazel.de>
 <13062.1489622943@sss.pgh.pa.us>
 <20170320062511.hp5qeurtxrwsvfxr@alap3.anarazel.de>
 <31150.1490193666@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <31150.1490193666@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

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


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