On 2016/01/06 18:58, Rushabh Lathia wrote:
I started looking at updated patch and its definitely iked the new
approach.
Thanks for the review!
With the initial look and test overall things looking great, I am still
reviewing the code changes but here are few early doubts/questions:
.) 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.
.) 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.
I think this depends on the capabilities of the FDW.
.) Documentation for the new API is missing (fdw-callbacks).
Will add the docs.
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
