From ff2d5372235dae339cbfa04aa55a8458cd6bfab8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 12 Jul 2017 18:00:04 -0400 Subject: [PATCH] Fix ruleutils.c for domain-over-array cases, too. Further investigation shows that ruleutils isn't quite up to speed either for cases where we have a domain-over-array: it needs to be prepared to look past a CoerceToDomain at the top level of field and element assignments, else it decompiles them incorrectly. Potentially this would result in failure to dump/reload a rule, if it looked like the one in the new test case. (I also added a test for EXPLAIN; that output isn't broken, but clearly we need more test coverage here.) Like commit b1cb32fb6, this bug is reachable in cases we already support, so back-patch all the way. --- src/backend/utils/adt/ruleutils.c | 41 +++++++++++++++++++++++++--- src/test/regress/expected/domain.out | 41 +++++++++++++++++++++++----- src/test/regress/sql/domain.sql | 11 +++++++- 3 files changed, 81 insertions(+), 12 deletions(-) diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index bc85a34739c..6a58b7e1ff4 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -5832,8 +5832,11 @@ get_update_query_targetlist_def(Query *query, List *targetList, /* * We must dig down into the expr to see if it's a PARAM_MULTIEXPR * Param. That could be buried under FieldStores and ArrayRefs - * (cf processIndirection()), and underneath those there could be - * an implicit type coercion. + * and CoerceToDomains (cf processIndirection()), and underneath + * those there could be an implicit type coercion. Because we + * would ignore implicit type coercions anyway, we don't need to + * be as careful as processIndirection() is about descending past + * implicit CoerceToDomains. */ expr = (Node *) tle->expr; while (expr) @@ -5852,6 +5855,14 @@ get_update_query_targetlist_def(Query *query, List *targetList, break; expr = (Node *) aref->refassgnexpr; } + else if (IsA(expr, CoerceToDomain)) + { + CoerceToDomain *cdomain = (CoerceToDomain *) expr; + + if (cdomain->coercionformat != COERCE_IMPLICIT_CAST) + break; + expr = (Node *) cdomain->arg; + } else break; } @@ -9553,13 +9564,17 @@ get_opclass_name(Oid opclass, Oid actual_datatype, * * We strip any top-level FieldStore or assignment ArrayRef nodes that * appear in the input, printing them as decoration for the base column - * name (which we assume the caller just printed). Return the subexpression - * that's to be assigned. + * name (which we assume the caller just printed). We might also need to + * strip CoerceToDomain nodes, but only ones that appear above assignment + * nodes. + * + * Returns the subexpression that's to be assigned. */ static Node * processIndirection(Node *node, deparse_context *context) { StringInfo buf = context->buf; + CoerceToDomain *cdomain = NULL; for (;;) { @@ -9607,10 +9622,28 @@ processIndirection(Node *node, deparse_context *context) */ node = (Node *) aref->refassgnexpr; } + else if (IsA(node, CoerceToDomain)) + { + cdomain = (CoerceToDomain *) node; + /* If it's an explicit domain coercion, we're done */ + if (cdomain->coercionformat != COERCE_IMPLICIT_CAST) + break; + /* Tentatively descend past the CoerceToDomain */ + node = (Node *) cdomain->arg; + } else break; } + /* + * If we descended past a CoerceToDomain whose argument turned out not to + * be a FieldStore or array assignment, back up to the CoerceToDomain. + * (This is not enough to be fully correct if there are nested implicit + * CoerceToDomains, but such cases shouldn't ever occur.) + */ + if (cdomain && node == (Node *) cdomain->arg) + node = (Node *) cdomain; + return node; } diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index 9fb750d2d36..d8fe5e4c8a1 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -266,20 +266,47 @@ insert into dcomptable (d1[1].r, d1[1].i) values(100, 99); -- fail ERROR: value for domain dcomptypea violates check constraint "c1" update dcomptable set d1[1].r = d1[1].r + 1 where d1[1].i > 0; -- fail ERROR: value for domain dcomptypea violates check constraint "c1" -update dcomptable set d1[1].r = d1[1].r - 1 where d1[1].i > 0; +update dcomptable set d1[1].r = d1[1].r - 1, d1[1].i = d1[1].i + 1 + where d1[1].i > 0; select * from dcomptable; d1 -------------------- {"(11,)","(,)"} {"(99,)"} - {"(1,2)","(,)"} - {"(3,4)","(6,5)"} - {"(7,8)","(10,9)"} - {"(9,10)","(,)"} - {"(0,2)"} - {"(98,100)"} + {"(1,3)","(,)"} + {"(3,5)","(6,5)"} + {"(7,9)","(10,9)"} + {"(9,11)","(,)"} + {"(0,3)"} + {"(98,101)"} (8 rows) +explain (verbose, costs off) + update dcomptable set d1[1].r = d1[1].r - 1, d1[1].i = d1[1].i + 1 + where d1[1].i > 0; + QUERY PLAN +---------------------------------------------------------------------------------------------------------------- + Update on public.dcomptable + -> Seq Scan on public.dcomptable + Output: (d1[1].r := (d1[1].r - '1'::double precision))[1].i := (d1[1].i + '1'::double precision), ctid + Filter: (dcomptable.d1[1].i > '0'::double precision) +(4 rows) + +create rule silly as on delete to dcomptable do instead + update dcomptable set d1[1].r = d1[1].r - 1, d1[1].i = d1[1].i + 1 + where d1[1].i > 0; +\d+ dcomptable + Table "public.dcomptable" + Column | Type | Modifiers | Storage | Stats target | Description +--------+------------+-----------+----------+--------------+------------- + d1 | dcomptypea | | extended | | +Indexes: + "dcomptable_d1_key" UNIQUE CONSTRAINT, btree (d1) +Rules: + silly AS + ON DELETE TO dcomptable DO INSTEAD UPDATE dcomptable SET d1[1].r = dcomptable.d1[1].r - 1::double precision, d1[1].i = dcomptable.d1[1].i + 1::double precision + WHERE dcomptable.d1[1].i > 0::double precision + drop table dcomptable; drop type comptype cascade; NOTICE: drop cascades to type dcomptypea diff --git a/src/test/regress/sql/domain.sql b/src/test/regress/sql/domain.sql index 1fd7b11fd43..5ec128dd254 100644 --- a/src/test/regress/sql/domain.sql +++ b/src/test/regress/sql/domain.sql @@ -150,9 +150,18 @@ insert into dcomptable (d1[1].r) values(99); insert into dcomptable (d1[1].r, d1[1].i) values(99, 100); insert into dcomptable (d1[1].r, d1[1].i) values(100, 99); -- fail update dcomptable set d1[1].r = d1[1].r + 1 where d1[1].i > 0; -- fail -update dcomptable set d1[1].r = d1[1].r - 1 where d1[1].i > 0; +update dcomptable set d1[1].r = d1[1].r - 1, d1[1].i = d1[1].i + 1 + where d1[1].i > 0; select * from dcomptable; +explain (verbose, costs off) + update dcomptable set d1[1].r = d1[1].r - 1, d1[1].i = d1[1].i + 1 + where d1[1].i > 0; +create rule silly as on delete to dcomptable do instead + update dcomptable set d1[1].r = d1[1].r - 1, d1[1].i = d1[1].i + 1 + where d1[1].i > 0; +\d+ dcomptable + drop table dcomptable; drop type comptype cascade;