Re: Push down time-related SQLValue functions to foreign server

Hi.

Ashutosh Bapat писал 2021-08-19 17:01:
> I spent some time looking at this patch.
> 
> Generally it looks like a good idea. These stable functions will be
> evaluated at the execution time and replaced with constants. I am not
> sure whether the nodes saved in the param_list may not get the same
> treatment. Have you verified that?

I'm not sure I understand you. All parameters are treated in the same 
way.
They are evaluated in process_query_params(), real params and 
parameters, corresponding to our SQLValue functions.
If we look at execution of something like

  explain verbose select * from test1 t1 where i in (select i from test1 
t2 where t2.t< now() and t1.i=t2.i) ;
                                                     QUERY PLAN
-------------------------------------------------------------------------------------------------------------------
  Foreign Scan on public.test1 t1  (cost=100.00..243310.11 rows=930 
width=20)
    Output: t1.i, t1.t, t1.l
    Filter: (SubPlan 1)
    Remote SQL: SELECT i, t, l FROM data.test1
    SubPlan 1
      ->  Foreign Scan on public.test1 t2  (cost=100.00..161.29 rows=5 
width=4)
            Output: t2.i
            Remote SQL: SELECT i FROM data.test1 WHERE (($1::integer = 
i)) AND ((t < $2::timestamp with time zone)

we can see two parameters evaluated in process_query_params(), one - of 
T_Param type (with value of current t1.i) and one of T_SQLValueFunction 
type (with value of current_timestamp).

> 
> Also the new node types being added to the param list is something
> other than Param. So it conflicts with the comment below in
> prepare_query_params()?
>    /*
>      * Prepare remote-parameter expressions for evaluation.  (Note: in
>      * practice, we expect that all these expressions will be just 
> Params, so
>      * we could possibly do something more efficient than using the 
> full
>      * expression-eval machinery for this.  But probably there would be 
> little
>      * benefit, and it'd require postgres_fdw to know more than is 
> desirable
>      * about Param evaluation.)
>      */
> If we are already adding non-params to this list, then the comment is 
> outdated?

Fixed comment in the new version of the patches.

-- 
Best regards,
Alexander Pyhalov,
Postgres Professional