> What'd be better, if we could do it, is to ship the clause in
> the form
> WHERE foreigncol = 'one'
> that is, instead of plastering a cast on the Var, try to not put
> any explicit cast on the constant. That fixes your original use
> case better than what you've proposed, and I think it might be
> possible to do it unconditionally instead of needing a hacky
> column property to enable it. The reason this could be okay
> is that it seems reasonable for postgres_fdw to rely on the
> core parser's heuristic that an unknown-type literal is the
> same type as what it's being compared to. So, if we are trying
> to deparse something of the form "foreigncol operator constant",
> and the foreigncol and constant are of the same type locally,
> we could leave off the cast on the constant. (There might need
> to be some restrictions about the operator taking those types
> natively with no cast, not sure; also this doesn't apply to
> constants that are going to be printed as non-string literals.)
>
> Slipping this heuristic into the code structure of deparse.c
> might be rather messy, though. I've not looked at just how
> to implement it.
This doesn't look too bad from here, at least so far. The attached
change adds a new const_showtype field to the deparse_expr_cxt, and
passes that instead of the hardcoded 0 to deparseConst. deparseOpExpr
modifies const_showtype if both sides of a binary operation are text,
and resets it to 0 after the recursion.
I restricted it to text-only after seeing a regression test fail: while
deparsing `percentile_cont(c2/10::numeric)`, c2, an integer column, is a
FuncExpr with a numeric return type. That matches the numeric 10, and
without the explicit cast, integer-division-related havoc ensues. I
don't know why it's a FuncExpr, and I don't know why it's not an int,
but the constant is definitely a non-string, in any case.
In the course of testing, I discovered that the @@ text-search operator
works against textified enums on my stock 13.1 server (a "proper" enum
column yields "operator does not exist"). I'm rather wary of actually
trying to depend on that behavior, although it seems probably-safe in
the same character set and collation.
From f7024ae2e95bd28c47267d97d7350f43a2b6c072 Mon Sep 17 00:00:00 2001
From: Dian M Fay <dian.m....@gmail.com>
Date: Sun, 7 Mar 2021 02:59:14 -0500
Subject: [PATCH] Suppress explicit casts of text constants in postgres_fdw
when the other side of a binary OpExpr is also text.
---
contrib/postgres_fdw/deparse.c | 27 ++++++-
.../postgres_fdw/expected/postgres_fdw.out | 81 ++++++++++++-------
contrib/postgres_fdw/sql/postgres_fdw.sql | 9 +++
3 files changed, 84 insertions(+), 33 deletions(-)
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 6faf499f9a..738c9c1038 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -100,6 +100,7 @@ typedef struct deparse_expr_cxt
* a base
relation. */
StringInfo buf; /* output buffer to append to */
List **params_list; /* exprs that will become remote Params
*/
+ int const_showtype; /* override showtype in
deparseConst */
} deparse_expr_cxt;
#define REL_ALIAS_PREFIX "r"
@@ -1018,6 +1019,7 @@ deparseSelectStmtForRel(StringInfo buf, PlannerInfo
*root, RelOptInfo *rel,
context.foreignrel = rel;
context.scanrel = IS_UPPER_REL(rel) ? fpinfo->outerrel : rel;
context.params_list = params_list;
+ context.const_showtype = 0;
/* Construct SELECT clause */
deparseSelectSql(tlist, is_subquery, retrieved_attrs, &context);
@@ -1597,6 +1599,7 @@ deparseFromExprForRel(StringInfo buf, PlannerInfo *root,
RelOptInfo *foreignrel,
context.scanrel = foreignrel;
context.root = root;
context.params_list = params_list;
+ context.const_showtype = 0;
appendStringInfoChar(buf, '(');
appendConditions(fpinfo->joinclauses, &context);
@@ -1897,6 +1900,7 @@ deparseDirectUpdateSql(StringInfo buf, PlannerInfo *root,
context.scanrel = foreignrel;
context.buf = buf;
context.params_list = params_list;
+ context.const_showtype = 0;
appendStringInfoString(buf, "UPDATE ");
deparseRelation(buf, rel);
@@ -2005,6 +2009,7 @@ deparseDirectDeleteSql(StringInfo buf, PlannerInfo *root,
context.scanrel = foreignrel;
context.buf = buf;
context.params_list = params_list;
+ context.const_showtype = 0;
appendStringInfoString(buf, "DELETE FROM ");
deparseRelation(buf, rel);
@@ -2392,7 +2397,7 @@ deparseExpr(Expr *node, deparse_expr_cxt *context)
deparseVar((Var *) node, context);
break;
case T_Const:
- deparseConst((Const *) node, context, 0);
+ deparseConst((Const *) node, context,
context->const_showtype);
break;
case T_Param:
deparseParam((Param *) node, context);
@@ -2762,6 +2767,9 @@ deparseOpExpr(OpExpr *node, deparse_expr_cxt *context)
StringInfo buf = context->buf;
HeapTuple tuple;
Form_pg_operator form;
+ Expr *left;
+ Expr *right;
+ Oid rightType;
char oprkind;
/* Retrieve information about the operator from system catalog. */
@@ -2778,11 +2786,21 @@ deparseOpExpr(OpExpr *node, deparse_expr_cxt *context)
/* Always parenthesize the expression. */
appendStringInfoChar(buf, '(');
+ right = llast(node->args);
+
/* Deparse left operand, if any. */
if (oprkind == 'b')
{
- deparseExpr(linitial(node->args), context);
+ left = linitial(node->args);
+
+ deparseExpr(left, context);
appendStringInfoChar(buf, ' ');
+ rightType = exprType((Node *) right);
+
+ if (rightType == TEXTOID && rightType == exprType((Node
*)left)) {
+ /* Suppress const casting for text values against text
columns. */
+ context->const_showtype = -1;
+ }
}
/* Deparse operator name. */
@@ -2790,11 +2808,14 @@ deparseOpExpr(OpExpr *node, deparse_expr_cxt *context)
/* Deparse right operand. */
appendStringInfoChar(buf, ' ');
- deparseExpr(llast(node->args), context);
+ deparseExpr(right, context);
appendStringInfoChar(buf, ')');
ReleaseSysCache(tuple);
+
+ /* Release cast suppression in case it was set. */
+ context->const_showtype = 0;
}
/*
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out
b/contrib/postgres_fdw/expected/postgres_fdw.out
index 0649b6b81c..78b7ca2c42 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -341,11 +341,11 @@ SELECT * FROM ft1 WHERE false;
-- with WHERE clause
EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6
= '1' AND t1.c7 >= '1';
- QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------------------------------------------------------------------------
Foreign Scan on public.ft1 t1
Output: c1, c2, c3, c4, c5, c6, c7, c8
- Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE
((c7 >= '1'::bpchar)) AND (("C 1" = 101)) AND ((c6 = '1'::text))
+ Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE
((c7 >= '1'::bpchar)) AND (("C 1" = 101)) AND ((c6 = '1'))
(3 rows)
SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1';
@@ -707,11 +707,11 @@ EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE
c1 = (ARRAY[c1,c2,3])[1]
(3 rows)
EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c6 = E'foo''s\\bar';
-- check special chars
- QUERY PLAN
--------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------------
Foreign Scan on public.ft1 t1
Output: c1, c2, c3, c4, c5, c6, c7, c8
- Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE
((c6 = E'foo''s\\bar'::text))
+ Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE
((c6 = E'foo''s\\bar'))
(3 rows)
EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE c8 = 'foo'; -- can't
be sent to remote
@@ -3396,12 +3396,12 @@ ORDER BY ref_0."C 1";
Index Cond: (ref_0."C 1" < 10)
-> Foreign Scan on public.ft1 ref_1
Output: ref_1.c3, ref_0.c2
- Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 =
'00001'::text))
+ Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'))
-> Materialize
Output: ref_3.c3
-> Foreign Scan on public.ft2 ref_3
Output: ref_3.c3
- Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 =
'00001'::text))
+ Remote SQL: SELECT c3 FROM "S 1"."T 1" WHERE ((c3 = '00001'))
(15 rows)
SELECT ref_0.c2, subq_1.*
@@ -4105,6 +4105,27 @@ ERROR: invalid input syntax for type integer: "foo"
CONTEXT: processing expression at position 2 in select list
ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum;
-- ===================================================================
+-- conversion for const matching local type
+-- ===================================================================
+SELECT * FROM ft1 WHERE c8 = 'foo' LIMIT 1;
+ c1 | c2 | c3 | c4 | c5 |
c6 | c7 | c8
+----+----+-------+------------------------------+--------------------------+----+------------+-----
+ 1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1
| 1 | foo
+(1 row)
+
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE text;
+SELECT * FROM ft1 WHERE c8 = 'foo' LIMIT 1;
+ c1 | c2 | c3 | c4 | c5 |
c6 | c7 | c8
+----+----+-------+------------------------------+--------------------------+----+------------+-----
+ 1 | 1 | 00001 | Fri Jan 02 00:00:00 1970 PST | Fri Jan 02 00:00:00 1970 | 1
| 1 | foo
+(1 row)
+
+SELECT * FROM ft1 WHERE c8 LIKE 'foo' LIMIT 1; -- ERROR
+ERROR: operator does not exist: public.user_enum ~~ unknown
+HINT: No operator matches the given name and argument types. You might need
to add explicit type casts.
+CONTEXT: remote SQL command: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S
1"."T 1" WHERE ((c8 ~~ 'foo')) LIMIT 1::bigint
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum;
+-- ===================================================================
-- subtransaction
-- + local/remote error doesn't break cursor
-- ===================================================================
@@ -4154,35 +4175,35 @@ 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
-------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------
Foreign Scan on public.ft3
Output: f1, f2, f3
- Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f1 = 'foo'::text))
+ Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f1 = 'foo'))
(3 rows)
explain (verbose, costs off) select * from ft3 where f1 COLLATE "C" = 'foo';
- QUERY PLAN
-------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------
Foreign Scan on public.ft3
Output: f1, f2, f3
- Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f1 = 'foo'::text))
+ Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f1 = 'foo'))
(3 rows)
explain (verbose, costs off) select * from ft3 where f2 = 'foo';
- QUERY PLAN
-------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------
Foreign Scan on public.ft3
Output: f1, f2, f3
- Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f2 = 'foo'::text))
+ Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f2 = 'foo'))
(3 rows)
explain (verbose, costs off) select * from ft3 where f3 = 'foo';
- QUERY PLAN
-------------------------------------------------------------------------------
+ QUERY PLAN
+------------------------------------------------------------------------
Foreign Scan on public.ft3
Output: f1, f2, f3
- Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f3 = 'foo'::text))
+ Remote SQL: SELECT f1, f2, f3 FROM public.loct3 WHERE ((f3 = 'foo'))
(3 rows)
explain (verbose, costs off) select * from ft3 f, loct3 l
@@ -4284,22 +4305,22 @@ INSERT INTO ft2 (c1,c2,c3)
INSERT INTO ft2 (c1,c2,c3) VALUES (1104,204,'ddd'), (1105,205,'eee');
EXPLAIN (verbose, costs off)
UPDATE ft2 SET c2 = c2 + 300, c3 = c3 || '_update3' WHERE c1 % 10 = 3;
-- can be pushed down
- QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------
+ QUERY PLAN
+----------------------------------------------------------------------------------------------------------------
Update on public.ft2
-> Foreign Update on public.ft2
- Remote SQL: UPDATE "S 1"."T 1" SET c2 = (c2 + 300), c3 = (c3 ||
'_update3'::text) WHERE ((("C 1" % 10) = 3))
+ Remote SQL: UPDATE "S 1"."T 1" SET c2 = (c2 + 300), c3 = (c3 ||
'_update3') WHERE ((("C 1" % 10) = 3))
(3 rows)
UPDATE ft2 SET c2 = c2 + 300, c3 = c3 || '_update3' WHERE c1 % 10 = 3;
EXPLAIN (verbose, costs off)
UPDATE ft2 SET c2 = c2 + 400, c3 = c3 || '_update7' WHERE c1 % 10 = 7
RETURNING *; -- can be pushed down
-
QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ QUERY
PLAN
+------------------------------------------------------------------------------------------------------------------------------------------------------------
Update on public.ft2
Output: c1, c2, c3, c4, c5, c6, c7, c8
-> Foreign Update on public.ft2
- Remote SQL: UPDATE "S 1"."T 1" SET c2 = (c2 + 400), c3 = (c3 ||
'_update7'::text) WHERE ((("C 1" % 10) = 7)) RETURNING "C 1", c2, c3, c4, c5,
c6, c7, c8
+ Remote SQL: UPDATE "S 1"."T 1" SET c2 = (c2 + 400), c3 = (c3 ||
'_update7') WHERE ((("C 1" % 10) = 7)) RETURNING "C 1", c2, c3, c4, c5, c6, c7,
c8
(4 rows)
UPDATE ft2 SET c2 = c2 + 400, c3 = c3 || '_update7' WHERE c1 % 10 = 7
RETURNING *;
@@ -4412,11 +4433,11 @@ UPDATE ft2 SET c2 = c2 + 400, c3 = c3 || '_update7'
WHERE c1 % 10 = 7 RETURNING
EXPLAIN (verbose, costs off)
UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
FROM ft1 WHERE ft1.c1 = ft2.c2 AND ft1.c1 % 10 = 9;
-- can be pushed down
-
QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+
QUERY PLAN
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Update on public.ft2
-> Foreign Update
- Remote SQL: UPDATE "S 1"."T 1" r1 SET c2 = (r1.c2 + 500), c3 = (r1.c3
|| '_update9'::text), c7 = 'ft2 '::character(10) FROM "S 1"."T 1" r2
WHERE ((r1.c2 = r2."C 1")) AND (((r2."C 1" % 10) = 9))
+ Remote SQL: UPDATE "S 1"."T 1" r1 SET c2 = (r1.c2 + 500), c3 = (r1.c3
|| '_update9'), c7 = 'ft2 '::character(10) FROM "S 1"."T 1" r2 WHERE
((r1.c2 = r2."C 1")) AND (((r2."C 1" % 10) = 9))
(3 rows)
UPDATE ft2 SET c2 = ft2.c2 + 500, c3 = ft2.c3 || '_update9', c7 = DEFAULT
@@ -7834,7 +7855,7 @@ select tableoid::regclass, * FROM locp;
update utrtest set a = 2 where b = 'foo' returning *;
ERROR: new row for relation "loct" violates check constraint "loct_a_check"
DETAIL: Failing row contains (2, foo).
-CONTEXT: remote SQL command: UPDATE public.loct SET a = 2 WHERE ((b =
'foo'::text)) RETURNING a, b
+CONTEXT: remote SQL command: UPDATE public.loct SET a = 2 WHERE ((b = 'foo'))
RETURNING a, b
-- But the reverse is allowed
update utrtest set a = 1 where b = 'qux' returning *;
a | b
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql
b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 2b525ea44a..f25941a022 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -1133,6 +1133,15 @@ SELECT ft1.c1, ft2.c2, ft1 FROM ft1, ft2 WHERE ft1.c1
= ft2.c1 AND ft1.c1 = 1;
SELECT sum(c2), array_agg(c8) FROM ft1 GROUP BY c8; -- ERROR
ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum;
+-- ===================================================================
+-- conversion for const matching local type
+-- ===================================================================
+SELECT * FROM ft1 WHERE c8 = 'foo' LIMIT 1;
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE text;
+SELECT * FROM ft1 WHERE c8 = 'foo' LIMIT 1;
+SELECT * FROM ft1 WHERE c8 LIKE 'foo' LIMIT 1; -- ERROR
+ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum;
+
-- ===================================================================
-- subtransaction
-- + local/remote error doesn't break cursor
--
2.30.1
