Re: TEXT vs VARCHAR join qual push down diffrence, bug or expected? - Mailing list pgsql-hackers

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