Re: Optimization for updating foreign tables in Postgres FDW

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.