On 2016/11/23 20:28, Rushabh Lathia wrote:
On Tue, Nov 22, 2016 at 6:24 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp <mailto:fujita.ets...@lab.ntt.co.jp>> wrote:


        1)
        -static void deparseExplicitTargetList(List *tlist, List
        **retrieved_attrs,
        +static void deparseExplicitTargetList(bool is_returning,
        +                          List *tlist,
        +                          List **retrieved_attrs,
                                   deparse_expr_cxt *context);

        Any particular reason of inserting new argument as 1st
        argunment? In general
        we add new flags at the end or before the out param for the existing
        function.

    No, I don't.  I think the *context should be the last argument as in
    other functions in deparse.c.  How about inserting that before the
    out param **retrieved_attrs, like this?

    static void
    deparseExplicitTargetList(List *tlist,
                              bool is_returning,
                              List **retrieved_attrs,
                              deparse_expr_cxt *context);

Yes, adding it before retrieved_attrs would be good.

OK, will do.

        5) make_explicit_returning_list() pull the var list from the
        returningList and
        build the targetentry for the returning list and at the end
        rewrites the
        fdw_scan_tlist.

        AFAIK, in case of DML - which is going to pushdown to the remote
        server
        ideally fdw_scan_tlist should be same as returning list, as
        final output
        for the query is query will be RETUNING list only. isn't that true?

    That would be true.  But the fdw_scan_tlist passed from the core
    would contain junk columns inserted by the rewriter and planner
    work, such as CTID for the target table and whole-row Vars for other
    tables specified in the FROM clause of an UPDATE or the USING clause
    of a DELETE.  So, I created the patch so that the pushed-down
    UPDATE/DELETE retrieves only the data needed for the RETURNING
    calculation from the remote server, not the whole fdw_scan_tlist data.

Yes, I noticed that fdw_scan_tlist have CTID for the target table and
whole-raw vars for
other tables specified in the FROM clause of the DML. But I was thinking
isn't it possible
to create new fdw_scan_tlist once we found that DML is direct pushable
to foreign server?
I tried quickly doing that - but later its was throwing "variable not
found in subplan target list"
from set_foreignscan_references().

        If yes, then why can't we directly replace the fdw_scan_tlist
        with the
        returning
        list, rather then make_explicit_returning_list()?

    I think we could do that if we modified the core (maybe the executor
    part).  I'm not sure that's a good idea, though.

Yes modifying core is not good idea. But just want to understand why you
think that this need need to modify core?

Sorry, I don't remember that.  Will check.

I'd like to move this to the next CF.

Thank you for the comments!

Best regards,
Etsuro Fujita




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to