Re: WIP: Faster Expression Processing v4

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: WIP: Faster Expression Processing v4
Date: 2017-03-24 21:16:15
Message-ID: 22833.1490390175@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Attached is an updated patch. I believe this is committable, but
since I whacked it around quite a bit, I'm sure you'll want to look
it over first.

Please be sure the commit message notes that function EXECUTE permissions
are now checked at executor startup not first call. We need to document
that in the v10 release notes as an incompatibility, and I'm sure we'll
forget if the commit log doesn't point it out.

Some loose ends that remain to be looked at, though I do not think any
of these are reasons to postpone commit:

* I'm concerned that we now do not have enough check_stack_depth() calls.
In our off-list discussion I wanted to add one to ExecProcNode, and you
were unhappy about that ... but it'd still be a lot less stack checking
than we do now.

* I still think we should look into removing the EEOP_FETCHSOME op types,
or at least finding some other way to perform the calculation of the last
attnums in the mainline expression compilation path.

* As we discussed, ExecEvalWholeRowVar is now using a pretty inefficient
method for flattening tuples into datums. I will take a to-do item to
fix this.

* ExecInitCheck is really just ExecInitExpr with a make_ands_explicit
call in front of it. It turned out that most of the places that were
(or should have been) calling it were doing a make_ands_implicit call
for no other reason than to satisfy its API. I changed most of those
to just call ExecInitExpr directly. There are just a couple of call
sites left, and I think probably those are likewise not really that
happy with this API --- but I didn't take the time to chase down where
the expressions were coming from in those cases. It seems possible
that we could remove ExecInitCheck/ExecPrepareCheck entirely. I'll
look into this later, too.

* As we discussed, CaseTestValue/DomainValue are pretty messy and need
to be rethought. That might not get done for v10 though.

* I'm not very happy that execSRF.c has two somewhat different
implementations of largely similar functionality, and
SetExprState.elidedFuncState seems like a wart. That's mostly a
pre-existing problem of course. I'm satisfied with leaving it as it is
for now, but eventually some refactoring there would be good.

The attached patch is against HEAD as of last night (commit 457a44487).

regards, tom lane

Attachment Content-Type Size
faster-expressions-v5.patch.gz application/x-gzip 131.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2017-03-24 21:39:10 Re: delta relations in AFTER triggers
Previous Message Stephen Frost 2017-03-24 20:56:26 Re: comment/security label for publication/subscription