Обсуждение: 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

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