Refactoring simplify_function (was: Caching constant stable expressions)
Hi list,
Per Tom's request, I split out this refactoring from my CacheExpr patch.
Basically I'm just centralizing the eval_const_expressions_mutator()
call on function arguments, from multiple different places to a single
location. Without this, it would be a lot harder to implement argument
caching logic in the CacheExpr patch.
The old call tree goes like:
case T_FuncExpr:
-> eval_const_expressions_mutator(args)
-> simplify_function
-> reorder_function_arguments
-> eval_const_expressions_mutator(args)
-> add_function_defaults
-> eval_const_expressions_mutator(args)
New call tree:
case T_FuncExpr:
-> simplify_function
-> simplify_copy_function_arguments
-> reorder_function_arguments
-> add_function_defaults
-> eval_const_expressions_mutator(args)
Assumptions being made:
* The code didn't depend on expanding existing arguments before adding defaults
* operator calls don't need to expand default arguments -- it's
currently impossible to CREATE OPERATOR with a function that has
unspecified arguments
Other changes:
* simplify_function no longer needs a 'bool has_named_args' argument
(it finds out itself).
* I added 'bool mutate_args' in its place, since some callers need to
mutate args beforehand.
* reorder_function_arguments no longer needs to keep track of which
default args were added.
Regards,
Marti
Attachments:
refactor-simplify-function.patchtext/x-patch; charset=US-ASCII; name=refactor-simplify-function.patchDownload+146-171
Marti Raudsepp <marti@juffo.org> writes:
Per Tom's request, I split out this refactoring from my CacheExpr patch.
Basically I'm just centralizing the eval_const_expressions_mutator()
call on function arguments, from multiple different places to a single
location. Without this, it would be a lot harder to implement argument
caching logic in the CacheExpr patch.
I've applied a slightly-modified version of this after reconciling it
with the protransform fixes. I assume you are going to submit a rebased
version of the main CacheExpr patch?
regards, tom lane
On Sat, Mar 24, 2012 at 01:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I've applied a slightly-modified version of this after reconciling it
with the protransform fixes.
Cool, thanks!
I assume you are going to submit a rebased
version of the main CacheExpr patch?
Yeah, I'm still working on addressing the comments from your last email.
Haven't had much time to work on it for the last 2 weeks, but I hope
to finish most of it this weekend.
Regards,
Marti
On Fri, Mar 23, 2012 at 7:41 PM, Marti Raudsepp <marti@juffo.org> wrote:
Yeah, I'm still working on addressing the comments from your last email.
Haven't had much time to work on it for the last 2 weeks, but I hope
to finish most of it this weekend.
Since the updated patch seems never to have been posted, I think this
one is dead for 9.2. I'll mark it Returned with Feedback.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company