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

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.


Andres Freund

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

Reply via email to