On 2017-03-10 09:00:14 -0500, Peter Eisentraut wrote:
> I have also looked at the 0002 and 0003 patches, and they seem OK, but
> they are obviously not of much use without 0004. What is your ambition
> for getting 0004 reviewed and committed for PG10?
I'd really like to get it in. The performance improvements on its own
are significant, and it provides the basis for significantly larger
improvements again (JIT) - those followup improvements are large patches
again though, so I'd rather not do all of that next cycle.
My next step (over the weekend) is to add tests to execQual.c to get it
a good chunk closer to 100% test coverage, and then do the same for the
new implementation (there's probably very little additional tests needed
after the conversion). Given all tests pass before/after, and there's a
lot of them, I think we can have a reasonable confidence of a low bug
density.
Large parts of the changes are fairly mechanical, the interesting bits
probably are:
- there's a lot fewer "full blown" node types for expression evaluation anymore. In most cases there's now only a
top-levelExprState node that's nodeTag() able. The actual information to execute an instruction now is in
ExprState->steps[i]
- because of the previous point, it now isn't legal anymore to call ExecInitExpr() on a list of expressions -
ExecInitExprList()is a convenience wrapper around manually iterating over the list (gets rid of ugly casts, too)
- Because they behave differently on an actual expression evaluation basis, quals / check constraint, now have to be
initializedwith ExecInitQual/ExecInitCheck(). That way the shortcut behaviour and such can be retained, while also
beingfaster.
- PlanState->targetlist is gone. Because expressions aren't initialized on their own anymore, but as part of
ExecProject,there's no need for it.
- The seperation between ExecInitExprRec() and ExecEvalExpr/ExecInterpExpr() is different - we try to do as much as
possibleduring initialization.
- I retained some ugly hackering around innermost_caseval|null/domainval|null - I had started down a path of removing
theuse of those in random parts of the system, but I ended up not going for it.
Greetings,
Andres Freund
