Обсуждение: TEXT vs VARCHAR join qual push down diffrence, bug or expected?

Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
> It is observed that, when we have one remote (huge) table and one local
> (small) table and a join between them, then
>  1. If the column type is text, then we push the join qual to the remote
>     server, so that we will have less rows to fetch, and thus execution time
>     is very less.
>  2. If the column type is varchar, then we do not push the join qual to the
>     remote server, resulting into large number of data fetch and thus
>     execution time is very high.

Hmm ...

> Also given that RelabelType are just dummy wrapper for binary compatible
> types, can we simply set collation and state from its inner context instead
> on above check block.

I think you're blaming the wrong code; RelabelType is handled basically
the same as most other cases.

It strikes me that this function is really going about things the wrong
way.  Rather than trying to determine the output collation per se, what
we ought to be asking is "does every operator in the proposed expression
have an input collation that can be traced to some foreign Var further
down in the expression"?  That is, given the example in hand,
RelabelType(ForeignVar) = RelabelType(LocalVar)

the logic ought to be like "the ForeignVar has collation X, and that
bubbles up without change through the RelabelType, and then the equals
operator's inputcollation matches that, so accept it --- regardless of
where the other operand's collation came from exactly".  The key point
is that we want to validate operator input collations, not output
collations, as having something to do with what the remote side would do.

This would represent a fairly significant rewrite of foreign_expr_walker's
collation logic; although I think the end result would be no more
complicated, possibly simpler, than it is now.
        regards, tom lane

<div dir="ltr">Hi Tom,<br /><div class="gmail_extra"><br /><div class="gmail_quote">On Tue, Sep 22, 2015 at 12:31 AM,
TomLane <span dir="ltr"><<a href="mailto:tgl@sss.pgh.pa.us" target="_blank">tgl@sss.pgh.pa.us</a>></span>
wrote:<br/><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"><spanclass=""><br /></span>I think you're blaming the wrong code; RelabelType is
handledbasically<br /> the same as most other cases.<br /><br /> It strikes me that this function is really going about
thingsthe wrong<br /> way.  Rather than trying to determine the output collation per se, what<br /> we ought to be
askingis "does every operator in the proposed expression<br /> have an input collation that can be traced to some
foreignVar further<br /> down in the expression"?  That is, given the example in hand,<br /><br />        
RelabelType(ForeignVar)= RelabelType(LocalVar)<br /><br /> the logic ought to be like "the ForeignVar has collation X,
andthat<br /> bubbles up without change through the RelabelType, and then the equals<br /> operator's inputcollation
matchesthat, so accept it --- regardless of<br /> where the other operand's collation came from exactly".  The key
point<br/> is that we want to validate operator input collations, not output<br /> collations, as having something to
dowith what the remote side would do.<br /><br /> This would represent a fairly significant rewrite of
foreign_expr_walker's<br/> collation logic; although I think the end result would be no more<br /> complicated,
possiblysimpler, than it is now.<br /><br />                         regards, tom lane<br /></blockquote></div><br
/><br/>IIUC, you are saying that collation check for output collation is not<br />necessary for all
OpExpr/FuncExpr/ArrayRefetc.<br />Should we remove code blocks like<br /><span
style="font-family:monospace,monospace">               collation = r->resultcollid;<br />                if
(collation== InvalidOid)<br />                    state = FDW_COLLATE_NONE;<br />                else if
(inner_cxt.state== FDW_COLLATE_SAFE &&<br />                         collation == inner_cxt.collation)<br />   
               state = FDW_COLLATE_SAFE;<br />                else<br />                    state =
FDW_COLLATE_UNSAFE;<br/></span><br />and just bubble up the collation and state to the next level?<br /><br />Here I
seethat, in the result collation validation, we missed the case when<br />result collation is default collation. For
foreignvar, we return collation<br />as is in inner context with the state set to SAFE. But in case of local var,<br
/>weare only allowing InvalidOid or Default oid for collation, however while<br />returning back, we set collation to
InvalidOidand state to NONE even for<br />default collation. I think we are missing something here.<br /><br />To
handlethis case, we need to either,<br />1. allow local var to set inner_cxt collation to what var actually has
(which<br/>will be either Invalid or DEFAULT) and set state to NONE if non collable or<br />set state to SAFE if
defaultcollation. Like:<br />  In T_Var, local var case<br /><span style="font-family:monospace,monospace">       
collation= var->varcollid;<br />        state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;<br
/></span><br/>OR<br />2. In above code block, which checks result collation, we need to handle<br />default collation.
Like:<br/><span style="font-family:monospace,monospace">                else if (collation == DEFAULT_COLLATION_OID)<br
/>                   state = inner_cxt.state;<br /></span><br />Let me know if I missed any.<br /><br />Thanks<br /><br
/>--<br /><div class="gmail_signature"><div dir="ltr">Jeevan B Chalke<br />Principal Software Engineer, Product
Development<br/>EnterpriseDB Corporation<br />The Enterprise PostgreSQL Company<br /><br /></div></div></div></div> 

Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
> On Tue, Sep 22, 2015 at 12:31 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It strikes me that this function is really going about things the wrong
>> way.  Rather than trying to determine the output collation per se, what
>> we ought to be asking is "does every operator in the proposed expression
>> have an input collation that can be traced to some foreign Var further
>> down in the expression"?

> IIUC, you are saying that collation check for output collation is not
> necessary for all OpExpr/FuncExpr/ArrayRef etc.
> Should we remove code blocks like
>                 collation = r->resultcollid;
>                 if (collation == InvalidOid)
>                     state = FDW_COLLATE_NONE;
>                 else if (inner_cxt.state == FDW_COLLATE_SAFE &&
>                          collation == inner_cxt.collation)
>                     state = FDW_COLLATE_SAFE;
>                 else
>                     state = FDW_COLLATE_UNSAFE;

> and just bubble up the collation and state to the next level?

Removing that entirely would be quite incorrect, because then you'd be
lying to the parent node about what collation your node outputs.

After thinking a bit more about the existing special case for non-foreign
Vars, I wonder if what we should do is change these code blocks to look
like
               collation = r->resultcollid;               if (collation == InvalidOid)                   state =
FDW_COLLATE_NONE;              else if (inner_cxt.state == FDW_COLLATE_SAFE &&                        collation ==
inner_cxt.collation)                  state = FDW_COLLATE_SAFE;
 
+        else if (collation == DEFAULT_COLLATION_OID)
+            state = FDW_COLLATE_NONE;               else                   state = FDW_COLLATE_UNSAFE;

That is, only explicit introduction of a non-default collation causes
a subexpression to get labeled FDW_COLLATE_UNSAFE.  Default collations
would lose out when getting merged with a nondefault collation from a
foreign Var, so they should work all right.

The core point here is that we're going to send the expression to the
remote without any COLLATE clauses, so the remote's parser has to
come to the same conclusions we did about which collation to apply.
We assume that default-collation-throughout will work all right.
Nondefault collations will work as long as they originate from foreign
Vars, because then the remote parser should see the equivalent far-end
collations originating from those Vars --- and those collations win when
combined with default collations.
        regards, tom lane

I wrote:
> After thinking a bit more about the existing special case for non-foreign
> Vars, I wonder if what we should do is change these code blocks to look
> like

>                 collation = r->resultcollid;
>                 if (collation == InvalidOid)
>                     state = FDW_COLLATE_NONE;
>                 else if (inner_cxt.state == FDW_COLLATE_SAFE &&
>                          collation == inner_cxt.collation)
>                     state = FDW_COLLATE_SAFE;
> +        else if (collation == DEFAULT_COLLATION_OID)
> +            state = FDW_COLLATE_NONE;
>                 else
>                     state = FDW_COLLATE_UNSAFE;

On further thought, maybe it's the special case for non-foreign Vars
that is busted.  That is, non-foreign Vars should do
                   /* Var belongs to some other table */                   if (var->varcollid != InvalidOid &&
            var->varcollid != DEFAULT_COLLATION_OID)                       return false;
 

-                   /* We can consider that it doesn't set collation */
-                   collation = InvalidOid;
-                   state = FDW_COLLATE_NONE;
+                   collation = var->varcollid;
+                   state = OidIsValid(collation) ? FDW_COLLATE_SAFE : FDW_COLLATE_NONE;

and likewise for Consts, Params, etc.

This would basically mean clarifying the meaning of the state values as:

FDW_COLLATE_NONE: the expression is of a noncollatable type, period.

FDW_COLLATE_SAFE: the expression has default collation, or a nondefault
collation that is traceable to a foreign Var.

FDW_COLLATE_UNSAFE: the expression has a nondefault collation that is not
traceable to a foreign Var.

Hm ... actually, we probably need *both* types of changes if that's
what we believe the state values mean.

An alternative definition would be that FDW_COLLATE_NONE subsumes the
"collation doesn't trace to a foreign Var, but it's default so we don't
really care" case.  I think the problem we've got is that the
non-foreign-Var code thinks that's what the definition is, but the rest
of the code isn't quite consistent with it.

In any case I think we want to end up with a clearer specification of
what the states mean.
        regards, tom lane

I wrote:
> Hm ... actually, we probably need *both* types of changes if that's
> what we believe the state values mean.

After a bit more thinking and experimentation, I propose the attached
patch.

            regards, tom lane

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 81cb2b4..f64482c 100644
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 17,27 ****
   * We do not consider that it is ever safe to send COLLATE expressions to
   * the remote server: it might not have the same collation names we do.
   * (Later we might consider it safe to send COLLATE "C", but even that would
!  * fail on old remote servers.)  An expression is considered safe to send only
!  * if all collations used in it are traceable to Var(s) of the foreign table.
!  * That implies that if the remote server gets a different answer than we do,
!  * the foreign table's columns are not marked with collations that match the
!  * remote table's columns, which we can consider to be user error.
   *
   * Portions Copyright (c) 2012-2015, PostgreSQL Global Development Group
   *
--- 17,28 ----
   * We do not consider that it is ever safe to send COLLATE expressions to
   * the remote server: it might not have the same collation names we do.
   * (Later we might consider it safe to send COLLATE "C", but even that would
!  * fail on old remote servers.)  An expression is considered safe to send
!  * only if all operator/function input collations used in it are traceable to
!  * Var(s) of the foreign table.  That implies that if the remote server gets
!  * a different answer than we do, the foreign table's columns are not marked
!  * with collations that match the remote table's columns, which we can
!  * consider to be user error.
   *
   * Portions Copyright (c) 2012-2015, PostgreSQL Global Development Group
   *
*************** typedef struct foreign_glob_cxt
*** 69,77 ****
   */
  typedef enum
  {
!     FDW_COLLATE_NONE,            /* expression is of a noncollatable type */
      FDW_COLLATE_SAFE,            /* collation derives from a foreign Var */
!     FDW_COLLATE_UNSAFE            /* collation derives from something else */
  } FDWCollateState;

  typedef struct foreign_loc_cxt
--- 70,81 ----
   */
  typedef enum
  {
!     FDW_COLLATE_NONE,            /* expression is of a noncollatable type, or
!                                  * it has default collation that is not
!                                  * traceable to a foreign Var */
      FDW_COLLATE_SAFE,            /* collation derives from a foreign Var */
!     FDW_COLLATE_UNSAFE            /* collation is non-default and derives from
!                                  * something other than a foreign Var */
  } FDWCollateState;

  typedef struct foreign_loc_cxt
*************** foreign_expr_walker(Node *node,
*** 272,283 ****
                  else
                  {
                      /* Var belongs to some other table */
!                     if (var->varcollid != InvalidOid &&
!                         var->varcollid != DEFAULT_COLLATION_OID)
                          return false;
!
!                     /* We can consider that it doesn't set collation */
!                     collation = InvalidOid;
                      state = FDW_COLLATE_NONE;
                  }
              }
--- 276,286 ----
                  else
                  {
                      /* Var belongs to some other table */
!                     collation = var->varcollid;
!                     if (collation != InvalidOid &&
!                         collation != DEFAULT_COLLATION_OID)
                          return false;
!                     /* For either allowed collation, the state is NONE */
                      state = FDW_COLLATE_NONE;
                  }
              }
*************** foreign_expr_walker(Node *node,
*** 291,302 ****
                   * non-builtin type, or it reflects folding of a CollateExpr;
                   * either way, it's unsafe to send to the remote.
                   */
!                 if (c->constcollid != InvalidOid &&
!                     c->constcollid != DEFAULT_COLLATION_OID)
                      return false;
!
!                 /* Otherwise, we can consider that it doesn't set collation */
!                 collation = InvalidOid;
                  state = FDW_COLLATE_NONE;
              }
              break;
--- 294,304 ----
                   * non-builtin type, or it reflects folding of a CollateExpr;
                   * either way, it's unsafe to send to the remote.
                   */
!                 collation = c->constcollid;
!                 if (collation != InvalidOid &&
!                     collation != DEFAULT_COLLATION_OID)
                      return false;
!                 /* For either allowed collation, the state is NONE */
                  state = FDW_COLLATE_NONE;
              }
              break;
*************** foreign_expr_walker(Node *node,
*** 307,317 ****
                  /*
                   * Collation handling is same as for Consts.
                   */
!                 if (p->paramcollid != InvalidOid &&
!                     p->paramcollid != DEFAULT_COLLATION_OID)
                      return false;
-
-                 collation = InvalidOid;
                  state = FDW_COLLATE_NONE;
              }
              break;
--- 309,318 ----
                  /*
                   * Collation handling is same as for Consts.
                   */
!                 collation = p->paramcollid;
!                 if (collation != InvalidOid &&
!                     collation != DEFAULT_COLLATION_OID)
                      return false;
                  state = FDW_COLLATE_NONE;
              }
              break;
*************** foreign_expr_walker(Node *node,
*** 348,353 ****
--- 349,356 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
*************** foreign_expr_walker(Node *node,
*** 393,398 ****
--- 396,403 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
*************** foreign_expr_walker(Node *node,
*** 434,439 ****
--- 439,446 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
*************** foreign_expr_walker(Node *node,
*** 483,489 ****

                  /*
                   * RelabelType must not introduce a collation not derived from
!                  * an input foreign Var.
                   */
                  collation = r->resultcollid;
                  if (collation == InvalidOid)
--- 490,496 ----

                  /*
                   * RelabelType must not introduce a collation not derived from
!                  * an input foreign Var (same logic as for a real function).
                   */
                  collation = r->resultcollid;
                  if (collation == InvalidOid)
*************** foreign_expr_walker(Node *node,
*** 491,496 ****
--- 498,505 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
*************** foreign_expr_walker(Node *node,
*** 540,546 ****

                  /*
                   * ArrayExpr must not introduce a collation not derived from
!                  * an input foreign Var.
                   */
                  collation = a->array_collid;
                  if (collation == InvalidOid)
--- 549,555 ----

                  /*
                   * ArrayExpr must not introduce a collation not derived from
!                  * an input foreign Var (same logic as for a function).
                   */
                  collation = a->array_collid;
                  if (collation == InvalidOid)
*************** foreign_expr_walker(Node *node,
*** 548,553 ****
--- 557,564 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 1f417b3..65ea6e8 100644
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
*************** COMMIT;
*** 1005,1075 ****
  -- ===================================================================
  -- test handling of collations
  -- ===================================================================
! create table loct3 (f1 text collate "C", f2 text);
! create foreign table ft3 (f1 text collate "C", f2 text)
!   server loopback options (table_name 'loct3');
  -- can be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 = 'foo';
!                                 QUERY PLAN
! --------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
!    Remote SQL: SELECT f1, f2 FROM public.loct3 WHERE ((f1 = 'foo'::text))
  (3 rows)

  explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
!                                 QUERY PLAN
! --------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
!    Remote SQL: SELECT f1, f2 FROM public.loct3 WHERE ((f1 = 'foo'::text))
  (3 rows)

  explain (verbose, costs off) select * from ft3 where f2 = 'foo';
!                                 QUERY PLAN
! --------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
!    Remote SQL: SELECT f1, f2 FROM public.loct3 WHERE ((f2 = 'foo'::text))
  (3 rows)

  -- can't be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "POSIX" = 'foo';
!                   QUERY PLAN
! -----------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
     Filter: ((ft3.f1)::text = 'foo'::text)
!    Remote SQL: SELECT f1, f2 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f1 = 'foo' COLLATE "C";
!                   QUERY PLAN
! -----------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
     Filter: (ft3.f1 = 'foo'::text COLLATE "C")
!    Remote SQL: SELECT f1, f2 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f2 COLLATE "C" = 'foo';
!                   QUERY PLAN
! -----------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
     Filter: ((ft3.f2)::text = 'foo'::text)
!    Remote SQL: SELECT f1, f2 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";
!                   QUERY PLAN
! -----------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
     Filter: (ft3.f2 = 'foo'::text COLLATE "C")
!    Remote SQL: SELECT f1, f2 FROM public.loct3
  (4 rows)

  -- ===================================================================
  -- test writable foreign table stuff
  -- ===================================================================
--- 1005,1114 ----
  -- ===================================================================
  -- test handling of collations
  -- ===================================================================
! create table loct3 (f1 text collate "C" unique, f2 text, f3 varchar(10) unique);
! create foreign table ft3 (f1 text collate "C", f2 text, f3 varchar(10))
!   server loopback options (table_name 'loct3', use_remote_estimate 'true');
  -- can be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 = 'foo';
!                                   QUERY PLAN
! ------------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f1 = 'foo'::text))
  (3 rows)

  explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
!                                   QUERY PLAN
! ------------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f1 = 'foo'::text))
  (3 rows)

  explain (verbose, costs off) select * from ft3 where f2 = 'foo';
!                                   QUERY PLAN
! ------------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f2 = 'foo'::text))
  (3 rows)

+ explain (verbose, costs off) select * from ft3 where f3 = 'foo';
+                                   QUERY PLAN
+ ------------------------------------------------------------------------------
+  Foreign Scan on public.ft3
+    Output: f1, f2, f3
+    Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f3 = 'foo'::text))
+ (3 rows)
+
+ explain (verbose, costs off) select * from ft3 f, loct3 l
+   where f.f3 = l.f3 and l.f1 = 'foo';
+                                             QUERY PLAN
+ --------------------------------------------------------------------------------------------------
+  Nested Loop
+    Output: f.f1, f.f2, f.f3, l.f1, l.f2, l.f3
+    ->  Index Scan using loct3_f1_key on public.loct3 l
+          Output: l.f1, l.f2, l.f3
+          Index Cond: (l.f1 = 'foo'::text)
+    ->  Foreign Scan on public.ft3 f
+          Output: f.f1, f.f2, f.f3
+          Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE (($1::character varying(10) = f3))
+ (8 rows)
+
  -- can't be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "POSIX" = 'foo';
!                     QUERY PLAN
! ---------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
     Filter: ((ft3.f1)::text = 'foo'::text)
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f1 = 'foo' COLLATE "C";
!                     QUERY PLAN
! ---------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
     Filter: (ft3.f1 = 'foo'::text COLLATE "C")
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f2 COLLATE "C" = 'foo';
!                     QUERY PLAN
! ---------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
     Filter: ((ft3.f2)::text = 'foo'::text)
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";
!                     QUERY PLAN
! ---------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
     Filter: (ft3.f2 = 'foo'::text COLLATE "C")
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3
  (4 rows)

+ explain (verbose, costs off) select * from ft3 f, loct3 l
+   where f.f3 = l.f3 COLLATE "POSIX" and l.f1 = 'foo';
+                          QUERY PLAN
+ -------------------------------------------------------------
+  Hash Join
+    Output: f.f1, f.f2, f.f3, l.f1, l.f2, l.f3
+    Hash Cond: ((f.f3)::text = (l.f3)::text)
+    ->  Foreign Scan on public.ft3 f
+          Output: f.f1, f.f2, f.f3
+          Remote SQL: SELECT f1, f2, f3 FROM public.loct3
+    ->  Hash
+          Output: l.f1, l.f2, l.f3
+          ->  Index Scan using loct3_f1_key on public.loct3 l
+                Output: l.f1, l.f2, l.f3
+                Index Cond: (l.f1 = 'foo'::text)
+ (11 rows)
+
  -- ===================================================================
  -- test writable foreign table stuff
  -- ===================================================================
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index fcdd92e..11160f8 100644
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
*************** COMMIT;
*** 316,334 ****
  -- ===================================================================
  -- test handling of collations
  -- ===================================================================
! create table loct3 (f1 text collate "C", f2 text);
! create foreign table ft3 (f1 text collate "C", f2 text)
!   server loopback options (table_name 'loct3');

  -- can be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 = 'foo';
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
  explain (verbose, costs off) select * from ft3 where f2 = 'foo';
  -- can't be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "POSIX" = 'foo';
  explain (verbose, costs off) select * from ft3 where f1 = 'foo' COLLATE "C";
  explain (verbose, costs off) select * from ft3 where f2 COLLATE "C" = 'foo';
  explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";

  -- ===================================================================
  -- test writable foreign table stuff
--- 316,339 ----
  -- ===================================================================
  -- test handling of collations
  -- ===================================================================
! create table loct3 (f1 text collate "C" unique, f2 text, f3 varchar(10) unique);
! create foreign table ft3 (f1 text collate "C", f2 text, f3 varchar(10))
!   server loopback options (table_name 'loct3', use_remote_estimate 'true');

  -- can be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 = 'foo';
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
  explain (verbose, costs off) select * from ft3 where f2 = 'foo';
+ explain (verbose, costs off) select * from ft3 where f3 = 'foo';
+ explain (verbose, costs off) select * from ft3 f, loct3 l
+   where f.f3 = l.f3 and l.f1 = 'foo';
  -- can't be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "POSIX" = 'foo';
  explain (verbose, costs off) select * from ft3 where f1 = 'foo' COLLATE "C";
  explain (verbose, costs off) select * from ft3 where f2 COLLATE "C" = 'foo';
  explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";
+ explain (verbose, costs off) select * from ft3 f, loct3 l
+   where f.f3 = l.f3 COLLATE "POSIX" and l.f1 = 'foo';

  -- ===================================================================
  -- test writable foreign table stuff

Jeevan Chalke <jeevan.chalke@enterprisedb.com> writes:
> On Wed, Sep 23, 2015 at 10:15 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> After a bit more thinking and experimentation, I propose the attached
>> patch.

> I had a look over the patch and reviewed it. It is in excellent state to
> check-in.

After further thought I decided that the base case for
Const/Param/non-foreign-Vars wasn't quite right either.  If we don't like
the collation we should just set the state to UNSAFE not fail immediately,
because it might appear in a context where collation doesn't matter.
An example is "var IS NOT NULL".

So I've committed the attached modification of that patch.

            regards, tom lane

diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 81cb2b4..697de60 100644
*** a/contrib/postgres_fdw/deparse.c
--- b/contrib/postgres_fdw/deparse.c
***************
*** 17,27 ****
   * We do not consider that it is ever safe to send COLLATE expressions to
   * the remote server: it might not have the same collation names we do.
   * (Later we might consider it safe to send COLLATE "C", but even that would
!  * fail on old remote servers.)  An expression is considered safe to send only
!  * if all collations used in it are traceable to Var(s) of the foreign table.
!  * That implies that if the remote server gets a different answer than we do,
!  * the foreign table's columns are not marked with collations that match the
!  * remote table's columns, which we can consider to be user error.
   *
   * Portions Copyright (c) 2012-2015, PostgreSQL Global Development Group
   *
--- 17,28 ----
   * We do not consider that it is ever safe to send COLLATE expressions to
   * the remote server: it might not have the same collation names we do.
   * (Later we might consider it safe to send COLLATE "C", but even that would
!  * fail on old remote servers.)  An expression is considered safe to send
!  * only if all operator/function input collations used in it are traceable to
!  * Var(s) of the foreign table.  That implies that if the remote server gets
!  * a different answer than we do, the foreign table's columns are not marked
!  * with collations that match the remote table's columns, which we can
!  * consider to be user error.
   *
   * Portions Copyright (c) 2012-2015, PostgreSQL Global Development Group
   *
*************** typedef struct foreign_glob_cxt
*** 69,77 ****
   */
  typedef enum
  {
!     FDW_COLLATE_NONE,            /* expression is of a noncollatable type */
      FDW_COLLATE_SAFE,            /* collation derives from a foreign Var */
!     FDW_COLLATE_UNSAFE            /* collation derives from something else */
  } FDWCollateState;

  typedef struct foreign_loc_cxt
--- 70,81 ----
   */
  typedef enum
  {
!     FDW_COLLATE_NONE,            /* expression is of a noncollatable type, or
!                                  * it has default collation that is not
!                                  * traceable to a foreign Var */
      FDW_COLLATE_SAFE,            /* collation derives from a foreign Var */
!     FDW_COLLATE_UNSAFE            /* collation is non-default and derives from
!                                  * something other than a foreign Var */
  } FDWCollateState;

  typedef struct foreign_loc_cxt
*************** foreign_expr_walker(Node *node,
*** 272,284 ****
                  else
                  {
                      /* Var belongs to some other table */
!                     if (var->varcollid != InvalidOid &&
!                         var->varcollid != DEFAULT_COLLATION_OID)
!                         return false;
!
!                     /* We can consider that it doesn't set collation */
!                     collation = InvalidOid;
!                     state = FDW_COLLATE_NONE;
                  }
              }
              break;
--- 276,299 ----
                  else
                  {
                      /* Var belongs to some other table */
!                     collation = var->varcollid;
!                     if (collation == InvalidOid ||
!                         collation == DEFAULT_COLLATION_OID)
!                     {
!                         /*
!                          * It's noncollatable, or it's safe to combine with a
!                          * collatable foreign Var, so set state to NONE.
!                          */
!                         state = FDW_COLLATE_NONE;
!                     }
!                     else
!                     {
!                         /*
!                          * Do not fail right away, since the Var might appear
!                          * in a collation-insensitive context.
!                          */
!                         state = FDW_COLLATE_UNSAFE;
!                     }
                  }
              }
              break;
*************** foreign_expr_walker(Node *node,
*** 288,303 ****

                  /*
                   * If the constant has nondefault collation, either it's of a
!                  * non-builtin type, or it reflects folding of a CollateExpr;
!                  * either way, it's unsafe to send to the remote.
                   */
!                 if (c->constcollid != InvalidOid &&
!                     c->constcollid != DEFAULT_COLLATION_OID)
!                     return false;
!
!                 /* Otherwise, we can consider that it doesn't set collation */
!                 collation = InvalidOid;
!                 state = FDW_COLLATE_NONE;
              }
              break;
          case T_Param:
--- 303,318 ----

                  /*
                   * If the constant has nondefault collation, either it's of a
!                  * non-builtin type, or it reflects folding of a CollateExpr.
!                  * It's unsafe to send to the remote unless it's used in a
!                  * non-collation-sensitive context.
                   */
!                 collation = c->constcollid;
!                 if (collation == InvalidOid ||
!                     collation == DEFAULT_COLLATION_OID)
!                     state = FDW_COLLATE_NONE;
!                 else
!                     state = FDW_COLLATE_UNSAFE;
              }
              break;
          case T_Param:
*************** foreign_expr_walker(Node *node,
*** 305,318 ****
                  Param       *p = (Param *) node;

                  /*
!                  * Collation handling is same as for Consts.
                   */
!                 if (p->paramcollid != InvalidOid &&
!                     p->paramcollid != DEFAULT_COLLATION_OID)
!                     return false;
!
!                 collation = InvalidOid;
!                 state = FDW_COLLATE_NONE;
              }
              break;
          case T_ArrayRef:
--- 320,333 ----
                  Param       *p = (Param *) node;

                  /*
!                  * Collation rule is same as for Consts and non-foreign Vars.
                   */
!                 collation = p->paramcollid;
!                 if (collation == InvalidOid ||
!                     collation == DEFAULT_COLLATION_OID)
!                     state = FDW_COLLATE_NONE;
!                 else
!                     state = FDW_COLLATE_UNSAFE;
              }
              break;
          case T_ArrayRef:
*************** foreign_expr_walker(Node *node,
*** 348,353 ****
--- 363,370 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
*************** foreign_expr_walker(Node *node,
*** 393,398 ****
--- 410,417 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
*************** foreign_expr_walker(Node *node,
*** 434,439 ****
--- 453,460 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
*************** foreign_expr_walker(Node *node,
*** 483,489 ****

                  /*
                   * RelabelType must not introduce a collation not derived from
!                  * an input foreign Var.
                   */
                  collation = r->resultcollid;
                  if (collation == InvalidOid)
--- 504,510 ----

                  /*
                   * RelabelType must not introduce a collation not derived from
!                  * an input foreign Var (same logic as for a real function).
                   */
                  collation = r->resultcollid;
                  if (collation == InvalidOid)
*************** foreign_expr_walker(Node *node,
*** 491,496 ****
--- 512,519 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
*************** foreign_expr_walker(Node *node,
*** 540,546 ****

                  /*
                   * ArrayExpr must not introduce a collation not derived from
!                  * an input foreign Var.
                   */
                  collation = a->array_collid;
                  if (collation == InvalidOid)
--- 563,569 ----

                  /*
                   * ArrayExpr must not introduce a collation not derived from
!                  * an input foreign Var (same logic as for a function).
                   */
                  collation = a->array_collid;
                  if (collation == InvalidOid)
*************** foreign_expr_walker(Node *node,
*** 548,553 ****
--- 571,578 ----
                  else if (inner_cxt.state == FDW_COLLATE_SAFE &&
                           collation == inner_cxt.collation)
                      state = FDW_COLLATE_SAFE;
+                 else if (collation == DEFAULT_COLLATION_OID)
+                     state = FDW_COLLATE_NONE;
                  else
                      state = FDW_COLLATE_UNSAFE;
              }
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 1f417b3..65ea6e8 100644
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
*************** COMMIT;
*** 1005,1075 ****
  -- ===================================================================
  -- test handling of collations
  -- ===================================================================
! create table loct3 (f1 text collate "C", f2 text);
! create foreign table ft3 (f1 text collate "C", f2 text)
!   server loopback options (table_name 'loct3');
  -- can be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 = 'foo';
!                                 QUERY PLAN
! --------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
!    Remote SQL: SELECT f1, f2 FROM public.loct3 WHERE ((f1 = 'foo'::text))
  (3 rows)

  explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
!                                 QUERY PLAN
! --------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
!    Remote SQL: SELECT f1, f2 FROM public.loct3 WHERE ((f1 = 'foo'::text))
  (3 rows)

  explain (verbose, costs off) select * from ft3 where f2 = 'foo';
!                                 QUERY PLAN
! --------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
!    Remote SQL: SELECT f1, f2 FROM public.loct3 WHERE ((f2 = 'foo'::text))
  (3 rows)

  -- can't be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "POSIX" = 'foo';
!                   QUERY PLAN
! -----------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
     Filter: ((ft3.f1)::text = 'foo'::text)
!    Remote SQL: SELECT f1, f2 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f1 = 'foo' COLLATE "C";
!                   QUERY PLAN
! -----------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
     Filter: (ft3.f1 = 'foo'::text COLLATE "C")
!    Remote SQL: SELECT f1, f2 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f2 COLLATE "C" = 'foo';
!                   QUERY PLAN
! -----------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
     Filter: ((ft3.f2)::text = 'foo'::text)
!    Remote SQL: SELECT f1, f2 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";
!                   QUERY PLAN
! -----------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2
     Filter: (ft3.f2 = 'foo'::text COLLATE "C")
!    Remote SQL: SELECT f1, f2 FROM public.loct3
  (4 rows)

  -- ===================================================================
  -- test writable foreign table stuff
  -- ===================================================================
--- 1005,1114 ----
  -- ===================================================================
  -- test handling of collations
  -- ===================================================================
! create table loct3 (f1 text collate "C" unique, f2 text, f3 varchar(10) unique);
! create foreign table ft3 (f1 text collate "C", f2 text, f3 varchar(10))
!   server loopback options (table_name 'loct3', use_remote_estimate 'true');
  -- can be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 = 'foo';
!                                   QUERY PLAN
! ------------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f1 = 'foo'::text))
  (3 rows)

  explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
!                                   QUERY PLAN
! ------------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f1 = 'foo'::text))
  (3 rows)

  explain (verbose, costs off) select * from ft3 where f2 = 'foo';
!                                   QUERY PLAN
! ------------------------------------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f2 = 'foo'::text))
  (3 rows)

+ explain (verbose, costs off) select * from ft3 where f3 = 'foo';
+                                   QUERY PLAN
+ ------------------------------------------------------------------------------
+  Foreign Scan on public.ft3
+    Output: f1, f2, f3
+    Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f3 = 'foo'::text))
+ (3 rows)
+
+ explain (verbose, costs off) select * from ft3 f, loct3 l
+   where f.f3 = l.f3 and l.f1 = 'foo';
+                                             QUERY PLAN
+ --------------------------------------------------------------------------------------------------
+  Nested Loop
+    Output: f.f1, f.f2, f.f3, l.f1, l.f2, l.f3
+    ->  Index Scan using loct3_f1_key on public.loct3 l
+          Output: l.f1, l.f2, l.f3
+          Index Cond: (l.f1 = 'foo'::text)
+    ->  Foreign Scan on public.ft3 f
+          Output: f.f1, f.f2, f.f3
+          Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE (($1::character varying(10) = f3))
+ (8 rows)
+
  -- can't be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "POSIX" = 'foo';
!                     QUERY PLAN
! ---------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
     Filter: ((ft3.f1)::text = 'foo'::text)
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f1 = 'foo' COLLATE "C";
!                     QUERY PLAN
! ---------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
     Filter: (ft3.f1 = 'foo'::text COLLATE "C")
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f2 COLLATE "C" = 'foo';
!                     QUERY PLAN
! ---------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
     Filter: ((ft3.f2)::text = 'foo'::text)
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3
  (4 rows)

  explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";
!                     QUERY PLAN
! ---------------------------------------------------
   Foreign Scan on public.ft3
!    Output: f1, f2, f3
     Filter: (ft3.f2 = 'foo'::text COLLATE "C")
!    Remote SQL: SELECT f1, f2, f3 FROM public.loct3
  (4 rows)

+ explain (verbose, costs off) select * from ft3 f, loct3 l
+   where f.f3 = l.f3 COLLATE "POSIX" and l.f1 = 'foo';
+                          QUERY PLAN
+ -------------------------------------------------------------
+  Hash Join
+    Output: f.f1, f.f2, f.f3, l.f1, l.f2, l.f3
+    Hash Cond: ((f.f3)::text = (l.f3)::text)
+    ->  Foreign Scan on public.ft3 f
+          Output: f.f1, f.f2, f.f3
+          Remote SQL: SELECT f1, f2, f3 FROM public.loct3
+    ->  Hash
+          Output: l.f1, l.f2, l.f3
+          ->  Index Scan using loct3_f1_key on public.loct3 l
+                Output: l.f1, l.f2, l.f3
+                Index Cond: (l.f1 = 'foo'::text)
+ (11 rows)
+
  -- ===================================================================
  -- test writable foreign table stuff
  -- ===================================================================
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index fcdd92e..11160f8 100644
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
*************** COMMIT;
*** 316,334 ****
  -- ===================================================================
  -- test handling of collations
  -- ===================================================================
! create table loct3 (f1 text collate "C", f2 text);
! create foreign table ft3 (f1 text collate "C", f2 text)
!   server loopback options (table_name 'loct3');

  -- can be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 = 'foo';
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
  explain (verbose, costs off) select * from ft3 where f2 = 'foo';
  -- can't be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "POSIX" = 'foo';
  explain (verbose, costs off) select * from ft3 where f1 = 'foo' COLLATE "C";
  explain (verbose, costs off) select * from ft3 where f2 COLLATE "C" = 'foo';
  explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";

  -- ===================================================================
  -- test writable foreign table stuff
--- 316,339 ----
  -- ===================================================================
  -- test handling of collations
  -- ===================================================================
! create table loct3 (f1 text collate "C" unique, f2 text, f3 varchar(10) unique);
! create foreign table ft3 (f1 text collate "C", f2 text, f3 varchar(10))
!   server loopback options (table_name 'loct3', use_remote_estimate 'true');

  -- can be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 = 'foo';
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
  explain (verbose, costs off) select * from ft3 where f2 = 'foo';
+ explain (verbose, costs off) select * from ft3 where f3 = 'foo';
+ explain (verbose, costs off) select * from ft3 f, loct3 l
+   where f.f3 = l.f3 and l.f1 = 'foo';
  -- can't be sent to remote
  explain (verbose, costs off) select * from ft3 where f1 COLLATE "POSIX" = 'foo';
  explain (verbose, costs off) select * from ft3 where f1 = 'foo' COLLATE "C";
  explain (verbose, costs off) select * from ft3 where f2 COLLATE "C" = 'foo';
  explain (verbose, costs off) select * from ft3 where f2 = 'foo' COLLATE "C";
+ explain (verbose, costs off) select * from ft3 f, loct3 l
+   where f.f3 = l.f3 COLLATE "POSIX" and l.f1 = 'foo';

  -- ===================================================================
  -- test writable foreign table stuff