Re: [HACKERS] Re: [PATCH] Caching for stable expressions with constant arguments v3
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: I wonder if it would be better to add the CacheExpr nodes to the tree as a separate pass, instead of shoehorning it into eval_const_expressions? I think would be more readable that way, even though a separate pass would be more expensive. A separate pass would be very considerably more expensive, because (1) it would require making a whole new copy of each expression tree, and (2) it would require looking up the volatility status of each function and operator. eval_const_expressions already has to do the latter, or has to do it in a lot of cases anyway, so I think it's probably the best place to add this. If it weren't for (2) I would suggest adding the work to setrefs.c instead, but as it is I think we'd better suck it up and deal with any fallout in the later stages of the planner. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH] Caching for stable expressions with constant arguments v3
On 25.09.2011 05:09, Marti Raudsepp wrote: This is the third version of my CacheExpr patch. This seems to have bitrotted, thanks to the recent refactoring in eval_const_expressions(). For explanation about design decisions, please read these earlier messages: http://archives.postgresql.org/pgsql-hackers/2011-09/msg00579.php http://archives.postgresql.org/pgsql-hackers/2011-09/msg00812.php http://archives.postgresql.org/pgsql-hackers/2011-09/msg00833.php I wonder if it would be better to add the CacheExpr nodes to the tree as a separate pass, instead of shoehorning it into eval_const_expressions? I think would be more readable that way, even though a separate pass would be more expensive. And there are callers of eval_const_expressions() that have no use for the caching, like process_implied_equality(). This comment in RelationGetExpressions() also worries me: /* * Run the expressions through eval_const_expressions. This is not just an * optimization, but is necessary, because the planner will be comparing * them to similarly-processed qual clauses, and may fail to detect valid * matches without this. We don't bother with canonicalize_qual, however. */ result = (List *) eval_const_expressions(NULL, (Node *) result); Do the injected CacheExprs screw up that equality? Or the constraint exclusion logic in predtest.c? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH] Caching for stable expressions with constant arguments v3
All, I'd love to see someone evaluate the impact of Marti's patch on JDBC applications which use named prepared statements. Anyone have a benchmark handy? --Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
