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
