From cf0866241f051086cf1b19647102679149214565 Mon Sep 17 00:00:00 2001 From: Dian M Fay Date: Mon, 8 Nov 2021 22:52:07 -0500 Subject: [PATCH v7 1/2] Suppress explicit casts of safe Consts in postgres_fdw Comparisons between Consts of UNKNOWN type (string literals or NULLs) and Vars can rely on the remote server's operator resolution heuristic to match the Const to the remote column type instead of forcing the type from the local schema definition. This allows the local schema to diverge usefully from the remote, for example by declaring a remote enum column as text and eliminating the need to keep the enum synchronized across databases. --- contrib/postgres_fdw/deparse.c | 75 ++++++++++-- .../postgres_fdw/expected/postgres_fdw.out | 113 +++++++++++++----- contrib/postgres_fdw/sql/postgres_fdw.sql | 18 +++ 3 files changed, 165 insertions(+), 41 deletions(-) diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index d98bd66681..83c1749882 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -2695,9 +2695,14 @@ deparseVar(Var *node, deparse_expr_cxt *context) * Deparse given constant value into context->buf. * * This function has to be kept in sync with ruleutils.c's get_const_expr. - * As for that function, showtype can be -1 to never show "::typename" decoration, - * or +1 to always show it, or 0 to show it only if the constant wouldn't be assumed - * to be the right type by default. + * + * As in that function, showtype can be -1 to never show "::typename" + * decoration, +1 to always show it, or 0 to show it only if the constant + * wouldn't be assumed to be the right type by default. + * + * In addition, this code allows showtype to be -2 to indicate that we should + * not show "::typename" decoration if the constant is printed as an untyped + * literal or NULL (while in other cases, behaving as for showtype == 0). */ static void deparseConst(Const *node, deparse_expr_cxt *context, int showtype) @@ -2707,6 +2712,7 @@ deparseConst(Const *node, deparse_expr_cxt *context, int showtype) bool typIsVarlena; char *extval; bool isfloat = false; + bool isstring = false; bool needlabel; if (node->constisnull) @@ -2762,13 +2768,14 @@ deparseConst(Const *node, deparse_expr_cxt *context, int showtype) break; default: deparseStringLiteral(buf, extval); + isstring = true; break; } pfree(extval); - if (showtype < 0) - return; + if (showtype == -1) + return; /* never print type label */ /* * For showtype == 0, append ::typename unless the constant will be @@ -2788,7 +2795,13 @@ deparseConst(Const *node, deparse_expr_cxt *context, int showtype) needlabel = !isfloat || (node->consttypmod >= 0); break; default: - needlabel = true; + if (showtype == -2) + { + /* label unless we printed it as an untyped string */ + needlabel = !isstring; + } + else + needlabel = true; break; } if (needlabel || showtype > 0) @@ -2953,6 +2966,8 @@ deparseOpExpr(OpExpr *node, deparse_expr_cxt *context) StringInfo buf = context->buf; HeapTuple tuple; Form_pg_operator form; + Expr *right; + bool canSuppressRightConstCast = false; char oprkind; /* Retrieve information about the operator from system catalog. */ @@ -2966,13 +2981,53 @@ deparseOpExpr(OpExpr *node, deparse_expr_cxt *context) Assert((oprkind == 'l' && list_length(node->args) == 1) || (oprkind == 'b' && list_length(node->args) == 2)); + right = llast(node->args); + /* Always parenthesize the expression. */ appendStringInfoChar(buf, '('); /* Deparse left operand, if any. */ if (oprkind == 'b') { - deparseExpr(linitial(node->args), context); + Expr *left = linitial(node->args); + Oid leftType = exprType((Node *) left); + Oid rightType = exprType((Node *) right); + bool canSuppressLeftConstCast = false; + + /* + * When considering a binary operator, if one operand is a Const that + * can be printed as a bare string literal or NULL (i.e., it will look + * like type UNKNOWN to the remote parser), it normally receives an + * explicit cast to the operator-defined type. However, in Const-to- + * Var comparisons where both operands are of the same type, we prefer + * to suppress the explicit cast, leaving the Const's type resolution + * up to the remote parser. The remote's resolution heuristic will + * assume that an unknown input type being compared to a known input + * type is of that known type as well. + * + * Doing things this way allows some cases to succeed where a remote + * column is declared as a different type in the local (foreign) table. + * By emitting "foreigncol = 'foo'" not "foreigncol = 'foo'::text", we + * allow the remote parser to pick an "=" operator that's compatible + * with whatever the column really is remotely, such as an enum. + */ + if (leftType == rightType) + { + if (IsA(left, Const)) + { + canSuppressLeftConstCast = IsA(right, Var); + } + else if (IsA(right, Const)) + { + canSuppressRightConstCast = IsA(left, Var); + } + } + + if (canSuppressLeftConstCast) + deparseConst((Const *) left, context, -2); + else + deparseExpr(left, context); + appendStringInfoChar(buf, ' '); } @@ -2981,7 +3036,11 @@ deparseOpExpr(OpExpr *node, deparse_expr_cxt *context) /* Deparse right operand. */ appendStringInfoChar(buf, ' '); - deparseExpr(llast(node->args), context); + + if (canSuppressRightConstCast) + deparseConst((Const *) right, context, -2); + else + deparseExpr(right, context); appendStringInfoChar(buf, ')'); diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index fd141a0fa5..a07035c51e 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')) AND (("C 1" = 101)) AND ((c6 = '1'::text)) (3 rows) SELECT * FROM ft1 t1 WHERE t1.c1 = 101 AND t1.c6 = '1' AND t1.c7 >= '1'; @@ -1130,20 +1130,20 @@ SELECT * FROM ft1 WHERE c1 > (CASE random()::integer WHEN 0 THEN 1 WHEN 2 THEN 5 -- these are shippable EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 WHERE CASE c6 WHEN 'foo' THEN true ELSE c3 < 'bar' END; - QUERY PLAN --------------------------------------------------------------------------------------------------------------------------------------------------- + QUERY PLAN +-------------------------------------------------------------------------------------------------------------------------------------------- Foreign Scan on public.ft1 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 ((CASE c6 WHEN 'foo'::text THEN true ELSE (c3 < 'bar'::text) END)) + Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((CASE c6 WHEN 'foo'::text THEN true ELSE (c3 < 'bar') END)) (3 rows) EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 WHERE CASE c3 WHEN c6 THEN true ELSE c3 < 'bar' END; - QUERY PLAN ------------------------------------------------------------------------------------------------------------------------------------------ + QUERY PLAN +----------------------------------------------------------------------------------------------------------------------------------- Foreign Scan on public.ft1 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 ((CASE c3 WHEN c6 THEN true ELSE (c3 < 'bar'::text) END)) + Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((CASE c3 WHEN c6 THEN true ELSE (c3 < 'bar') END)) (3 rows) -- but this is not because of collation @@ -3491,12 +3491,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.* @@ -4114,11 +4114,11 @@ SELECT tableoid::regclass, * FROM ft1 t1 LIMIT 1; EXPLAIN (VERBOSE, COSTS OFF) SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)'; - 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 ((ctid = '(0,2)'::tid)) + Remote SQL: SELECT "C 1", c2, c3, c4, c5, c6, c7, c8 FROM "S 1"."T 1" WHERE ((ctid = '(0,2)')) (3 rows) SELECT * FROM ft1 t1 WHERE t1.ctid = '(0,2)'; @@ -4205,6 +4205,53 @@ ERROR: invalid input syntax for type integer: "foo" CONTEXT: column "c8" of foreign table "ft1" ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum; -- =================================================================== +-- local type can be different from remote type in some cases, +-- in particular if similarly-named operators do equivalent things +-- =================================================================== +ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE text; +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ft1 WHERE c8 = 'foo' LIMIT 1; + QUERY PLAN +-------------------------------------------------------------------------------------------------------------- + Foreign Scan on public.ft1 + 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 ((c8 = 'foo')) LIMIT 1::bigint +(3 rows) + +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) + +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ft1 WHERE 'foo' = c8 LIMIT 1; + QUERY PLAN +-------------------------------------------------------------------------------------------------------------- + Foreign Scan on public.ft1 + 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 (('foo' = c8)) LIMIT 1::bigint +(3 rows) + +SELECT * FROM ft1 WHERE 'foo' = c8 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) + +-- we declared c8 to be text locally, but it's still the same type on +-- the remote which will balk if we try to do anything incompatible +-- with that remote type +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 +SELECT * FROM ft1 WHERE c8::text LIKE 'foo' LIMIT 1; -- ERROR; cast not pushed down +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 -- =================================================================== @@ -4254,27 +4301,27 @@ 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'; @@ -4384,22 +4431,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 *; @@ -4512,11 +4559,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 @@ -8129,7 +8176,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 *; ERROR: cannot route tuples into foreign table to be updated "remp" diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 43c30d492d..e40112e41d 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -1167,6 +1167,24 @@ SELECT sum(c2), array_agg(c8) FROM ft1 GROUP BY c8; -- ERROR ANALYZE ft1; -- ERROR ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum; +-- =================================================================== +-- local type can be different from remote type in some cases, +-- in particular if similarly-named operators do equivalent things +-- =================================================================== +ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE text; +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ft1 WHERE c8 = 'foo' LIMIT 1; +SELECT * FROM ft1 WHERE c8 = 'foo' LIMIT 1; +EXPLAIN (VERBOSE, COSTS OFF) +SELECT * FROM ft1 WHERE 'foo' = c8 LIMIT 1; +SELECT * FROM ft1 WHERE 'foo' = c8 LIMIT 1; +-- we declared c8 to be text locally, but it's still the same type on +-- the remote which will balk if we try to do anything incompatible +-- with that remote type +SELECT * FROM ft1 WHERE c8 LIKE 'foo' LIMIT 1; -- ERROR +SELECT * FROM ft1 WHERE c8::text LIKE 'foo' LIMIT 1; -- ERROR; cast not pushed down +ALTER FOREIGN TABLE ft1 ALTER COLUMN c8 TYPE user_enum; + -- =================================================================== -- subtransaction -- + local/remote error doesn't break cursor -- 2.33.1