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.

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index bc85a34739c58563d9d3b71bfe21909fe135e41a..6a58b7e1ff45a5648e4ca1297197da8880cf4721 100644 (file)
--- 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 9fb750d2d36a53f50df668d7c5b6b1410ab24f00..d8fe5e4c8a1867916463e58b0de414f96e8a6a38 100644 (file)
--- 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 1fd7b11fd439aea0e9f3de44dd1108b25878dd4d..5ec128dd254d678d623e3686586257c493af1bec 100644 (file)
--- 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;