|From:||Marina Polyakova <m(dot)polyakova(at)postgrespro(dot)ru>|
|To:||Andres Freund <andres(at)anarazel(dot)de>|
|Cc:||Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Subject:||Re: WIP Patch: Precalculate stable functions, infrastructure v1|
|Views:||Raw Message | Whole Thread | Download mbox|
> I've not followed this thread, but just scanned this quickly because it
> affects execExpr* stuff.
Thank you very much for your comments! Thanks to them I have made v4 of
the patches (as in the previous one, only planning and execution part is
> Looks like having something like struct CachedExprState would be
> than these separate allocations? That also allows to aleviate some
> concerns when adding new fields (see below)
> I'd rather not have this on function scope - a) the stack pressure in
> ExecInterpExpr is quite noticeable in profiles already b) this is going
> to trigger warnings because of unused vars, because the compiler
> understand that EEOP_CACHEDEXPR_IF_CACHED always follows
> How about instead storing oldcontext in the expression itself?
Thanks, in new version I did all of it in this way.
> I'm also not sure how acceptable it is to just assume it's ok to leave
> stuff in per_query_memory, in some cases that could prove to be
I agree with you and in new version context is changed only for copying
datum of result value (if it's a pointer, its data should be allocated
in per_query_memory, or we will lost it for next tuples).
> Is this actually a meaningful path? Shouldn't always have done const
> evaluation before adding CachedExpr's?
eval_const_expressions_mutator is used several times, and one of them in
functions for selectivity evaluation (set_baserel_size_estimates ->
clauselist_selectivity -> clause_selectivity -> restriction_selectivity
-> ... -> get_restriction_variable -> estimate_expression_value ->
eval_const_expressions_mutator). In set_baserel_size_estimates function
right after selectivity evaluation there's costs evaluation and cached
expressions should be replaced before costs. I'm not sure that it is a
good idea to insert cached expressions replacement in
set_baserel_size_estimates, because in comments to it it's said "The
rel's targetlist and restrictinfo list must have been constructed
already, and rel->tuples must be set." and its file costsize.c is
entitled as "Routines to compute (and set) relation sizes and path
costs". So I have inserted cached expressions replacement just before it
(but I'm not sure that I have seen all places where it should be
inserted). What do you think about all of this?
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
|Next Message||Andres Freund||2017-05-22 15:42:27||Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression|
|Previous Message||Andres Freund||2017-05-22 15:30:48||plpgsql, caching, resowners and jit|