Re: Push down more UPDATEs/DELETEs in postgres_fdw

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Push down more UPDATEs/DELETEs in postgres_fdw
Date: 2016-11-30 08:29:47
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2016/11/23 20:28, Rushabh Lathia wrote:
> On Tue, Nov 22, 2016 at 6:24 PM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp <mailto:fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2016-11-30 08:47:20 Re: PSQL commands: \quit_if, \quit_unless
Previous Message Etsuro Fujita 2016-11-30 08:25:04 Re: postgres_fdw : altering foreign table not invalidating prepare statement execution plan.