Jeevan Chalke <[email protected]> writes:
> On Wed, Sep 23, 2015 at 10:15 PM, Tom Lane <[email protected]> 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
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
