Thread: 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
Attachment
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
Re: Refactoring simplify_function (was: Caching constant stable expressions)
From
Marti Raudsepp
Date:
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