Re: WIP Patch: Precalculate stable functions, infrastructure v1

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Marina Polyakova <polyakova(dot)marina69(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>
Subject: Re: WIP Patch: Precalculate stable functions, infrastructure v1
Date: 2018-01-16 22:05:01
Message-ID: 29643.1516140301@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

[ I'm sending this comment separately because I think it's an issue
Andres might take an interest in. ]

Marina Polyakova <m(dot)polyakova(at)postgrespro(dot)ru> writes:
> [ v7-0001-Precalculate-stable-and-immutable-functions.patch ]

Another thing that's bothering me is that the execution semantics
you're proposing for CachedExpr seem rather inflexible. AFAICS, once a
CachedExpr has run once, it will hang on to the result value and keep
returning that for the entire lifespan of the compiled expression.
We already noted that that breaks plpgsql's "simple expression"
logic, and it seems inevitable to me that it will be an issue for
other places as well. I think it'd be a better design if we had some
provision for resetting the cached values, short of recompiling the
expression from scratch.

One way that occurs to me to do this is to replace the simple boolean
isExecuted flags with a generation counter, and add a master generation
counter to ExprState. The rule for executing CachedExpr would be "if my
generation counter is different from the ExprState's counter, then
evaluate the subexpression and copy the ExprState's counter into mine".
Then the procedure for forcing recalculation of cached values is just to
increment the ExprState's counter. There are other ways one could imagine
doing this --- for instance, I initially thought of keeping the master
counter in the ExprContext being used to run the expression. But you need
some way to remember what counter value was used last with a particular
expression, so probably keeping it in ExprState is better.

Or we could just brute-force it by providing a function that runs through
a compiled expression step list and resets the isExecuted flag for each
EEOP_CACHEDEXPR_IF_CACHED step it finds. A slightly less brute-force
way is to link those steps together in a list, so that the function
doesn't have to visit irrelevant steps. If the reset function were seldom
used then the extra cycles for this wouldn't be very expensive. But I'm
not sure it will be seldom used --- it seems like plpgsql simple
expressions will be doing this every time --- so I think the counter
approach might be a better idea.

I'm curious to know whether Andres has some other ideas, or whether he
feels this is all a big wart on the compiled-expression concept. I don't
think there are any existing cases where we keep any meaningful state
across executions of a compiled-expression data structure; maybe that's
a bad idea in itself.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-01-16 23:08:19 Re: [HACKERS] Proposal: Local indexes for partitioned table
Previous Message Tom Lane 2018-01-16 21:30:46 Re: WIP Patch: Precalculate stable functions, infrastructure v1