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-level ExprState 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 initialized with ExecInitQual/ExecInitCheck(). That way the shortcut behaviour and such can be retained, while also being faster. - 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 possible during initialization. - I retained some ugly hackering around innermost_caseval|null/domainval|null - I had started down a path of removing the use of those in random parts of the system, but I ended up not going for it. 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
