On 2016/01/12 20:31, Rushabh Lathia wrote:
On Thu, Jan 7, 2016 at 6:15 PM, Etsuro Fujita
<fujita.ets...@lab.ntt.co.jp <mailto:fujita.ets...@lab.ntt.co.jp>> wrote:
On 2016/01/06 18:58, Rushabh Lathia wrote:
.) What the need of following change ?
@@ -833,9 +833,6 @@ appendWhereClause(StringInfo buf,
int nestlevel;
ListCell *lc;
- if (params)
- *params = NIL; /* initialize result list to
empty */
-
/* Set up context struct for recursion */
context.root = root;
context.foreignrel = baserel;
@@ -971,6 +968,63 @@ deparseUpdateSql(StringInfo buf,
PlannerInfo *root,
}
It is needed for deparsePushedDownUpdateSql to store params in both
WHERE clauses and expressions to assign to the target columns
into one params_list list.
Hmm sorry but I am still not getting the point, can you provide some
example to explain this ?
Sorry, maybe my explanation was not enough. Consider:
postgres=# create foreign table ft1 (a int, b int) server myserver
options (table_name 't1');
postgres=# insert into ft1 values (0, 0);
postgres=# prepare mt(int, int) as update ft1 set a = $1 where b = $2;
postgres=# explain verbose execute mt(1, 0);
postgres=# explain verbose execute mt(1, 0);
postgres=# explain verbose execute mt(1, 0);
postgres=# explain verbose execute mt(1, 0);
postgres=# explain verbose execute mt(1, 0);
After the 5 executions of mt we have
postgres=# explain verbose execute mt(1, 0);
QUERY PLAN
------------------------------------------------------------------------------------
Update on public.ft1 (cost=100.00..140.35 rows=12 width=10)
-> Foreign Update on public.ft1 (cost=100.00..140.35 rows=12 width=10)
Remote SQL: UPDATE public.t1 SET a = $1::integer WHERE ((b =
$2::integer))
(3 rows)
If we do that initialization in appendWhereClause, we would get a wrong
params_list list and a wrong remote pushed-down query for the last mt()
in deparsePushedDownUpdateSql.
.) When Tom Lane and Stephen Frost suggested getting the core
code involved,
I thought that we can do the mandatory checks into core it self
and making
completely out of dml_is_pushdown_safe(). Please correct me
The reason why I put that function in postgres_fdw.c is Check point 4:
+ * 4. We can't push an UPDATE down, if any expressions to assign
to the target
+ * columns are unsafe to evaluate on the remote server.
Here I was talking about checks related to triggers, or to LIMIT. I think
earlier thread talked about those mandatory check to the core. So may
be we can move those checks into make_modifytable() before calling
the PlanDMLPushdown.
Noticed that. Will do.
BTW, I keep a ForeignScan node pushing down an update to the remote
server, in the updated patches. I have to admit that that seems like
rather a misnomer. So, it might be worth adding a new ForeignUpdate
node, but my concern about that is that if doing so, we would have a lot
of duplicate code in ForeignUpdate and ForeignScan. What do you think
about that?
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
