Re: CREATE ROUTINE MAPPING

Thank you for the comment.

On Mon, Sep 10, 2018 at 4:16 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@lab.ntt.co.jp> wrote:
> Hello.
>
> At Tue, 4 Sep 2018 09:34:21 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in
<CAD21AoCBfTKRFPwboss4xVEoVwUmi0gKBgwsWQijviJP3hScwQ@mail.gmail.com>
>> On Tue, Sep 4, 2018 at 5:48 AM, David Fetter <david@fetter.org> wrote:
>> > On Fri, Aug 31, 2018 at 05:18:26PM +0900, Masahiko Sawada wrote:
>> >> On Thu, Jan 25, 2018 at 2:13 PM, David Fetter <david@fetter.org> wrote:
>> >> > On Thu, Jan 18, 2018 at 04:09:13PM -0500, Corey Huinker wrote:
>> >> >> >
>> >> >> >
>> >> >> > >
>> >> >> > > But other situations seem un-handle-able to me:
>> >> >> > >
>> >> >> > > SELECT remote_func1(l.x) FROM local_table l WHERE l.active = true;
>> >> >> >
>> >> >> > Do we have any way, or any plan to make a way, to push the set (SELECT
>> >> >> > x FROM local_table WHERE active = true) to the remote side for
>> >> >> > execution there?  Obviously, there are foreign DBs that couldn't
>> >> >> > support this, but I'm guessing they wouldn't have much by way of UDFs
>> >> >> > either.
>> >> >> >
>> >> >>
>> >> >> No. The remote query has to be generated at planning time, so it can't make
>> >> >> predicates out of anything that can't be resolved into constants by the
>> >> >> planner itself. The complexities of doing so would be excessive, far better
>> >> >> to let the application developer split the queries up because they know
>> >> >> better which parts have to resolve first.
>> >> >
>> >> > So Corey and I, with lots of inputs from Andrew Gierth and Matheus
>> >> > Oliveira, have come up with a sketch of how to do this, to wit:
>> >> >
>> >> > - Extend CREATE FUNCTION to take either FOREIGN and SERVER or AS and
>> >> >   LANGUAGE as parameters, but not both. This seems simpler, at least
>> >> >   in a proof of concept, than creating SQL standard compliant grammar
>> >> >   out of whole cloth.  The SQL standard grammar could be layered in
>> >> >   later via the rewriter if this turns out to work.
>> >>
>> >> I'm also interested in this feature. While studying this feature, I
>> >> understood that this feature just pair a local function with a remote
>> >> function, not means that creates a kind of virtual function that can
>> >> be invoked on only foreign servers. For example, if we execute the
>> >> following SQL the local_func() is invoked in local because the col1
>> >> column of local_table is referenced by it.
>
> Do you mean that ISO/IEC 9075-9:2016 (right?) is defining that
> (and we must follow it)?  Or does it comes by referring to
> something like [1]? As far as I see David's mail upthread,
> OPTIONS is not precisely defined.

Yeah, I read [1] and the final committee draft ISO/IEC 9075-9:2006, it
might be old though.

>
> [1] http://cs.unibo.it/~montesi/CBD/Articoli/SQL-MED.pdf
>
> Unfortunately I don't have access to the document nor concrete
> use cases. With a rough idea of "remote mapping", I can guess the
> followng four use cases.  Each example syntax is just a guess
> without any consideration on implementability or other
> restrictions. The patch looks currently covering B.

Thank you for summarizing.

>
> A. Just notify a function can be just pushed down.
>
>   ex. SELECT foo(1, 'bar');  Remote: SELECT foo(1, 'bar');
>
>    CREATE ROUTINE MAPPING foomap FOR foo(int, text) SERVER rem;
>    (or same as B)
>
> B. Replace function name with the remote equivalent.
>
>   ex. SELECT foo(1, 'bar');  Remote: SELECT hoge(1, 'bar');
>
>    CREATE ROUTINE MAPPING foomap FOR foo(int, text) SERVER rem
>    OPTIONS (remote_func_name 'hoge'));
>
> C. Adjust function specification with remote.
>
>   ex. SELECT foo(1, 'bar');  Remote: SELECT hoge('bar', 1, true);
>
>    CREATE ROUTINE MAPPING foomap FOR foo(int, text) SERVER rem
>    OPTIONS (remote_expression 'hoge($2,$1,true)');
>
> D. Replace with an equivalent remote expression.
>
>   ex. SELECT foo(1, 'bar');  Remote: SELECT ('bar' || to_char(1 % 10));
>
>    CREATE ROUTINE MAPPING foomap FOR foo(int, text) SERVER rem
>       OPTIONS (remote_expression '$2 || to_char($1 % 10)');
>
> I haven't looked the patch in depth, but the core side looks
> generic and the FDW side is extensible to A, C and D. I think B
> is enough as a starter.
> I don't mean that we should implement all of them. They are just possibilities.

I agree that this feature covers A and B as the first step. But I'm
concerned that for D (and maybe for C?) the volatility of mapped
function could be changed. That is, currently we allow to push down
only immutable functions but they might break it. Also, can the
replacing a function with any expression be a risk of sql injections?

Also, according to the standard the routine mapping seems to work when
columns of referenced foreign table are passed to the function that is
mapped to the remote function. The functions in WHERE clause will
obviously be mapped but I'm not sure for function in target lists.

>
>
> I have some comments on the patch.
>
> It doesn't seem working. Am I missing something?
> ====
> create server sv1 foreign data wrapper postgres_fdw options (host '/tmp', port '5432', dbname 'postgres');
> create table lt (a int);
> create foreign table ft (a int) server sv1 options (table_name 'lt');
> create function lhoge(int) returns int as 'begin return $1 * 2; end;' language plpgsql;
> create routine mapping rm1 for function lhoge(int) server sv1 options (remote_func_name 'rhoge');
> explain verbose select * from ft where a = lhoge(3);
>                             QUERY PLAN
> ------------------------------------------------------------------
>  Foreign Scan on public.ft  (cost=100.00..936.31 rows=15 width=4)
>    Output: a
>    Filter: (ft.a = lhoge(3))
>    Remote SQL: SELECT a FROM public.lt
> (4 rows)
> ====
>
> Perhaps it cannot push down simple SQL local functions. (I'm not
> sure we should do that.)

Yeah, the current patch maps only when the function can be pushed down
to the foreign server. So maybe you can use either the built-in
functions or functions that are provided an extension that is listed
in 'extension' option.

>
> Can't we specify remote schema in remote_func_name just as
> (remote_func_name "fooschema.funcname")?

That's a possible option.

>
> Can't we provide the syntax without making MAPPING reserved?

I think it's possible but I haven't tried it hard yet. The problem I
faced is that, for example, without that the "DROP ROUTINE MAPPING
name" where the name can be the ColId conflicts with the "DROP ROUTINE
function_with_argtype_list [CASCADE | RESTRICT]" where
function_with_argtype_list can be the ColId as well.

>
>  Chainging the syntax for ALTER/DROP ROUTINE MAPPING like USER
>  MAPPING, specifically to ALTER/DROP ROUTINE MAPPING FOR FUNCTION
>  fname SERVER sname would evade the conflict.

Hmm, the syntax of ALTER/DROP ROUTINE MAPPING follow the standard.
Maybe we can discuss later.

>
> Can't we just push down the function itself with no option defined?
>
>   CREATE ROUTINE MAPPING rm1 FOR FUNCTION pg_backend_pid() SERVER sv1;

Yeah, it would be an good idea.

>
> funcid is not used in objectaddress.c.
>
> (The patch is missing a LF in a comment in lsyscache.c)
>

Thanks, I'll fix them in the next patch.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center