Re: [HACKERS] WIP Patch: Precalculate stable functions,infrastructure v1

> Thanks for your patch, looks quite interesting!

Glad to hear it :)

>> To not send big patch I have split it (that's why version starts
> with the
>> first again) and here I send infrastructure patch which includes:
> 
> Yeah, but it's still 18k lines :)

Here 13k lines - 2 sets of expected results for regression tests..)

> After the first quick glance I have
> a few
> small questions.
> 
> If I call a stable function from a query and subquery, looks like it's
> cached:
> 
> ```
> =# select stable_with_int(1) from (select stable_with_int(1) from x)
> q;
> NOTICE:  00000: stable with int
> LOCATION:  exec_stmt_raise, pl_exec.c:3353
>  stable_with_int
> -----------------
>                1
>                1
>                1
>                1
> (4 rows)
> ```
> 
> But the same from CTE works different, is it supposed to be like that?
> 
> ```
> =# with data as (select stable_with_int(1) from x) select
> stable_with_int(1) from data;
> NOTICE:  00000: stable with int
> LOCATION:  exec_stmt_raise, pl_exec.c:3353
> NOTICE:  00000: stable with int
> LOCATION:  exec_stmt_raise, pl_exec.c:3353
>  stable_with_int
> -----------------
>                1
>                1
>                1
>                1
> (4 rows)
> ```

The function is always cached, but in the first example the plan is 
simplified so you only get one call of the function in the entire plan. 
(In the function subquery_planner, CTE are processed separately by 
calling the function SS_process_ctes. Subqueries are simplified a little 
later by calling the function pull_up_subqueries; in our case the 
function pull_up_simple_subquery is used.)

> Also I see this pattern quite some time, maybe it makes sense to move
> it to a function?
> 
> ```
> + /* create and return CachedExpr */
> + CachedExpr *new_node = makeNode(CachedExpr);
> + new_node->subexpr = (CacheableExpr *) current_node;
> +
> + context->root->hasCachedExpr = true;
> +
> + return (Node *) new_node;
> ```

Thanks, I agree with you and I'll change it accordingly.

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company