]> granicus.if.org Git - postgresql/commitdiff
Teach map_partition_varattnos to handle whole-row expressions.
authorRobert Haas <rhaas@postgresql.org>
Thu, 3 Aug 2017 15:21:29 +0000 (11:21 -0400)
committerRobert Haas <rhaas@postgresql.org>
Thu, 3 Aug 2017 15:21:29 +0000 (11:21 -0400)
Otherwise, partitioned tables with RETURNING expressions or subject
to a WITH CHECK OPTION do not work properly.

Amit Langote, reviewed by Amit Khandekar and Etsuro Fujita.  A few
comment changes by me.

Discussion: http://postgr.es/m/9a39df80-871e-6212-0684-f93c83be4097@lab.ntt.co.jp

src/backend/catalog/partition.c
src/backend/commands/tablecmds.c
src/backend/executor/nodeModifyTable.c
src/backend/parser/parse_utilcmd.c
src/backend/rewrite/rewriteManip.c
src/include/catalog/partition.h
src/include/rewrite/rewriteManip.h
src/test/regress/expected/insert.out
src/test/regress/expected/updatable_views.out
src/test/regress/sql/insert.sql
src/test/regress/sql/updatable_views.sql

index 9d50efb6a04751afbc13e549c79a2e3db51e8800..dcc7f8af27c0c7fad63cc1f800599c42b852375b 100644 (file)
@@ -898,16 +898,20 @@ get_qual_from_partbound(Relation rel, Relation parent,
  * We must allow for cases where physical attnos of a partition can be
  * different from the parent's.
  *
+ * If found_whole_row is not NULL, *found_whole_row returns whether a
+ * whole-row variable was found in the input expression.
+ *
  * Note: this will work on any node tree, so really the argument and result
  * should be declared "Node *".  But a substantial majority of the callers
  * are working on Lists, so it's less messy to do the casts internally.
  */
 List *
 map_partition_varattnos(List *expr, int target_varno,
-                                               Relation partrel, Relation parent)
+                                               Relation partrel, Relation parent,
+                                               bool *found_whole_row)
 {
        AttrNumber *part_attnos;
-       bool            found_whole_row;
+       bool            my_found_whole_row;
 
        if (expr == NIL)
                return NIL;
@@ -919,10 +923,10 @@ map_partition_varattnos(List *expr, int target_varno,
                                                                                target_varno, 0,
                                                                                part_attnos,
                                                                                RelationGetDescr(parent)->natts,
-                                                                               &found_whole_row);
-       /* There can never be a whole-row reference here */
+                                                                               RelationGetForm(partrel)->reltype,
+                                                                               &my_found_whole_row);
        if (found_whole_row)
-               elog(ERROR, "unexpected whole-row reference found in partition key");
+               *found_whole_row = my_found_whole_row;
 
        return expr;
 }
@@ -1783,6 +1787,7 @@ generate_partition_qual(Relation rel)
        List       *my_qual = NIL,
                           *result = NIL;
        Relation        parent;
+       bool            found_whole_row;
 
        /* Guard against stack overflow due to overly deep partition tree */
        check_stack_depth();
@@ -1825,7 +1830,11 @@ generate_partition_qual(Relation rel)
         * in it to bear this relation's attnos. It's safe to assume varno = 1
         * here.
         */
-       result = map_partition_varattnos(result, 1, rel, parent);
+       result = map_partition_varattnos(result, 1, rel, parent,
+                                                                        &found_whole_row);
+       /* There can never be a whole-row reference here */
+       if (found_whole_row)
+               elog(ERROR, "unexpected whole-row reference found in partition key");
 
        /* Save a copy in the relcache */
        oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
index bb00858ad13567859d8f1df08ac5220a29325706..b58c92d8466750d984008973599023e49c12d4e3 100644 (file)
@@ -1989,7 +1989,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
                                expr = map_variable_attnos(stringToNode(check[i].ccbin),
                                                                                   1, 0,
                                                                                   newattno, tupleDesc->natts,
-                                                                                  &found_whole_row);
+                                                                                  InvalidOid, &found_whole_row);
 
                                /*
                                 * For the moment we have to reject whole-row variables. We
@@ -8874,7 +8874,7 @@ ATPrepAlterColumnType(List **wqueue,
                                        map_variable_attnos(def->cooked_default,
                                                                                1, 0,
                                                                                attmap, RelationGetDescr(rel)->natts,
-                                                                               &found_whole_row);
+                                                                               InvalidOid, &found_whole_row);
                                if (found_whole_row)
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -13713,6 +13713,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
                        Oid                     part_relid = lfirst_oid(lc);
                        Relation        part_rel;
                        Expr       *constr;
+                       bool            found_whole_row;
 
                        /* Lock already taken */
                        if (part_relid != RelationGetRelid(attachRel))
@@ -13738,7 +13739,12 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
                        constr = linitial(partConstraint);
                        tab->partition_constraint = (Expr *)
                                map_partition_varattnos((List *) constr, 1,
-                                                                               part_rel, rel);
+                                                                               part_rel, rel,
+                                                                               &found_whole_row);
+                       /* There can never be a whole-row reference here */
+                       if (found_whole_row)
+                               elog(ERROR, "unexpected whole-row reference found in partition key");
+
                        /* keep our lock until commit */
                        if (part_rel != attachRel)
                                heap_close(part_rel, NoLock);
index 0dde0ed6eb2bdb91cd34ef28e283c589c9d16336..435aed3b8bc40ec83d9add17edcd02da0a8a6e79 100644 (file)
@@ -1996,7 +1996,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
                        /* varno = node->nominalRelation */
                        mapped_wcoList = map_partition_varattnos(wcoList,
                                                                                                         node->nominalRelation,
-                                                                                                        partrel, rel);
+                                                                                                        partrel, rel, NULL);
                        foreach(ll, mapped_wcoList)
                        {
                                WithCheckOption *wco = castNode(WithCheckOption, lfirst(ll));
@@ -2069,7 +2069,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
                        /* varno = node->nominalRelation */
                        rlist = map_partition_varattnos(returningList,
                                                                                        node->nominalRelation,
-                                                                                       partrel, rel);
+                                                                                       partrel, rel, NULL);
                        resultRelInfo->ri_projectReturning =
                                ExecBuildProjectionInfo(rlist, econtext, slot, &mtstate->ps,
                                                                                resultRelInfo->ri_RelationDesc->rd_att);
index 9f37f1b9206f4fe6f9ba6f25dc110ba923ee9369..a86c2341f5079af6f6415d30365d4109fdf145a5 100644 (file)
@@ -1107,7 +1107,7 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
                        ccbin_node = map_variable_attnos(stringToNode(ccbin),
                                                                                         1, 0,
                                                                                         attmap, tupleDesc->natts,
-                                                                                        &found_whole_row);
+                                                                                        InvalidOid, &found_whole_row);
 
                        /*
                         * We reject whole-row variables because the whole point of LIKE
@@ -1463,7 +1463,7 @@ generateClonedIndexStmt(CreateStmtContext *cxt, Relation source_idx,
                        indexkey = map_variable_attnos(indexkey,
                                                                                   1, 0,
                                                                                   attmap, attmap_length,
-                                                                                  &found_whole_row);
+                                                                                  InvalidOid, &found_whole_row);
 
                        /* As in transformTableLikeClause, reject whole-row variables */
                        if (found_whole_row)
@@ -1539,7 +1539,7 @@ generateClonedIndexStmt(CreateStmtContext *cxt, Relation source_idx,
                pred_tree = map_variable_attnos(pred_tree,
                                                                                1, 0,
                                                                                attmap, attmap_length,
-                                                                               &found_whole_row);
+                                                                               InvalidOid, &found_whole_row);
 
                /* As in transformTableLikeClause, reject whole-row variables */
                if (found_whole_row)
index b89b435da020c22a9233f502ee727c4a10249d74..ba706b25b409a99e8eb91bc96db5686fdbfe3c8f 100644 (file)
@@ -1203,14 +1203,12 @@ replace_rte_variables_mutator(Node *node,
  * appear in the expression.
  *
  * If the expression tree contains a whole-row Var for the target RTE,
- * the Var is not changed but *found_whole_row is returned as TRUE.
- * For most callers this is an error condition, but we leave it to the caller
- * to report the error so that useful context can be provided.  (In some
- * usages it would be appropriate to modify the Var's vartype and insert a
- * ConvertRowtypeExpr node to map back to the original vartype.  We might
- * someday extend this function's API to support that.  For now, the only
- * concession to that future need is that this function is a tree mutator
- * not just a walker.)
+ * *found_whole_row is returned as TRUE.  In addition, if to_rowtype is
+ * not InvalidOid, we modify the Var's vartype and insert a ConvertRowTypeExpr
+ * to map back to the orignal rowtype.  Callers that don't provide to_rowtype
+ * should report an error if *found_row_type is true; we don't do that here
+ * because we don't know exactly what wording for the error message would
+ * be most appropriate.  The caller will be aware of the context.
  *
  * This could be built using replace_rte_variables and a callback function,
  * but since we don't ever need to insert sublinks, replace_rte_variables is
@@ -1223,6 +1221,8 @@ typedef struct
        int                     sublevels_up;   /* (current) nesting depth */
        const AttrNumber *attno_map;    /* map array for user attnos */
        int                     map_length;             /* number of entries in attno_map[] */
+       /* Target type when converting whole-row vars */
+       Oid                     to_rowtype;
        bool       *found_whole_row;    /* output flag */
 } map_variable_attnos_context;
 
@@ -1257,6 +1257,34 @@ map_variable_attnos_mutator(Node *node,
                        {
                                /* whole-row variable, warn caller */
                                *(context->found_whole_row) = true;
+
+                               /* If the callers expects us to convert the same, do so. */
+                               if (OidIsValid(context->to_rowtype))
+                               {
+                                       /* No support for RECORDOID. */
+                                       Assert(var->vartype != RECORDOID);
+
+                                       /* Don't convert unless necessary. */
+                                       if (context->to_rowtype != var->vartype)
+                                       {
+                                               ConvertRowtypeExpr *r;
+
+                                               /* Var itself is converted to the requested type. */
+                                               newvar->vartype = context->to_rowtype;
+
+                                               /*
+                                                * And a conversion node on top to convert back to the
+                                                * original type.
+                                                */
+                                               r = makeNode(ConvertRowtypeExpr);
+                                               r->arg = (Expr *) newvar;
+                                               r->resulttype = var->vartype;
+                                               r->convertformat = COERCE_IMPLICIT_CAST;
+                                               r->location = -1;
+
+                                               return (Node *) r;
+                                       }
+                               }
                        }
                        return (Node *) newvar;
                }
@@ -1283,7 +1311,7 @@ Node *
 map_variable_attnos(Node *node,
                                        int target_varno, int sublevels_up,
                                        const AttrNumber *attno_map, int map_length,
-                                       bool *found_whole_row)
+                                       Oid to_rowtype, bool *found_whole_row)
 {
        map_variable_attnos_context context;
 
@@ -1291,6 +1319,7 @@ map_variable_attnos(Node *node,
        context.sublevels_up = sublevels_up;
        context.attno_map = attno_map;
        context.map_length = map_length;
+       context.to_rowtype = to_rowtype;
        context.found_whole_row = found_whole_row;
 
        *found_whole_row = false;
index f10879a162a0068f4bd6ed8443c9a144bc4cc587..434ded37d74f07cc2c90e4bd7c9d02138f6a1746 100644 (file)
@@ -80,7 +80,8 @@ extern Oid    get_partition_parent(Oid relid);
 extern List *get_qual_from_partbound(Relation rel, Relation parent,
                                                PartitionBoundSpec *spec);
 extern List *map_partition_varattnos(List *expr, int target_varno,
-                                               Relation partrel, Relation parent);
+                                               Relation partrel, Relation parent,
+                                               bool *found_whole_row);
 extern List *RelationGetPartitionQual(Relation rel);
 extern Expr *get_partition_qual_relid(Oid relid);
 
index 282ff9967ffd1f4aea86c106a64f6907ee476e5f..f0a7a8b2cd0f16a302f4dd2b56137cd19bcc2d30 100644 (file)
@@ -72,7 +72,7 @@ extern Node *replace_rte_variables_mutator(Node *node,
 extern Node *map_variable_attnos(Node *node,
                                        int target_varno, int sublevels_up,
                                        const AttrNumber *attno_map, int map_length,
-                                       bool *found_whole_row);
+                                       Oid to_rowtype, bool *found_whole_row);
 
 extern Node *ReplaceVarsFromTargetList(Node *node,
                                                  int target_varno, int sublevels_up,
index 0dcc86fef48b5194d988cd87f50b60bf1c459cd4..a2d9469592e01e2f6b058349c6d093c62fb091ce 100644 (file)
@@ -659,3 +659,24 @@ select tableoid::regclass, * from mcrparted order by a, b;
 (11 rows)
 
 drop table mcrparted;
+-- check that wholerow vars in the RETURNING list work with partitioned tables
+create table returningwrtest (a int) partition by list (a);
+create table returningwrtest1 partition of returningwrtest for values in (1);
+insert into returningwrtest values (1) returning returningwrtest;
+ returningwrtest 
+-----------------
+ (1)
+(1 row)
+
+-- check also that the wholerow vars in RETURNING list are converted as needed
+alter table returningwrtest add b text;
+create table returningwrtest2 (b text, c int, a int);
+alter table returningwrtest2 drop c;
+alter table returningwrtest attach partition returningwrtest2 for values in (2);
+insert into returningwrtest values (2, 'foo') returning returningwrtest;
+ returningwrtest 
+-----------------
+ (2,foo)
+(1 row)
+
+drop table returningwrtest;
index eab5c0334c5daee2893de46d00035f01e94f7551..2090a411fe02f78b138f38494bb8fddce14380fc 100644 (file)
@@ -2428,3 +2428,29 @@ ERROR:  new row violates check option for view "ptv_wco"
 DETAIL:  Failing row contains (1, 2, null).
 drop view ptv, ptv_wco;
 drop table pt, pt1, pt11;
+-- check that wholerow vars appearing in WITH CHECK OPTION constraint expressions
+-- work fine with partitioned tables
+create table wcowrtest (a int) partition by list (a);
+create table wcowrtest1 partition of wcowrtest for values in (1);
+create view wcowrtest_v as select * from wcowrtest where wcowrtest = '(2)'::wcowrtest with check option;
+insert into wcowrtest_v values (1);
+ERROR:  new row violates check option for view "wcowrtest_v"
+DETAIL:  Failing row contains (1).
+alter table wcowrtest add b text;
+create table wcowrtest2 (b text, c int, a int);
+alter table wcowrtest2 drop c;
+alter table wcowrtest attach partition wcowrtest2 for values in (2);
+create table sometable (a int, b text);
+insert into sometable values (1, 'a'), (2, 'b');
+create view wcowrtest_v2 as
+    select *
+      from wcowrtest r
+      where r in (select s from sometable s where r.a = s.a)
+with check option;
+-- WITH CHECK qual will be processed with wcowrtest2's
+-- rowtype after tuple-routing
+insert into wcowrtest_v2 values (2, 'no such row in sometable');
+ERROR:  new row violates check option for view "wcowrtest_v2"
+DETAIL:  Failing row contains (2, no such row in sometable).
+drop view wcowrtest_v, wcowrtest_v2;
+drop table wcowrtest, sometable;
index 6adf25da40e4edb2cf21a24e686c981890677ba2..6f178720870800c5dae50cc749549ce6649f31db 100644 (file)
@@ -399,3 +399,16 @@ insert into mcrparted values ('aaa', 0), ('b', 0), ('bz', 10), ('c', -10),
     ('commons', 0), ('d', -10), ('e', 0);
 select tableoid::regclass, * from mcrparted order by a, b;
 drop table mcrparted;
+
+-- check that wholerow vars in the RETURNING list work with partitioned tables
+create table returningwrtest (a int) partition by list (a);
+create table returningwrtest1 partition of returningwrtest for values in (1);
+insert into returningwrtest values (1) returning returningwrtest;
+
+-- check also that the wholerow vars in RETURNING list are converted as needed
+alter table returningwrtest add b text;
+create table returningwrtest2 (b text, c int, a int);
+alter table returningwrtest2 drop c;
+alter table returningwrtest attach partition returningwrtest2 for values in (2);
+insert into returningwrtest values (2, 'foo') returning returningwrtest;
+drop table returningwrtest;
index 2ede44c02b165446263c144e9d9778bd302e7c13..a6ba5aad9e4fc373cecc5f7a159d475cdee6b733 100644 (file)
@@ -1141,3 +1141,30 @@ create view ptv_wco as select * from pt where a = 0 with check option;
 insert into ptv_wco values (1, 2);
 drop view ptv, ptv_wco;
 drop table pt, pt1, pt11;
+
+-- check that wholerow vars appearing in WITH CHECK OPTION constraint expressions
+-- work fine with partitioned tables
+create table wcowrtest (a int) partition by list (a);
+create table wcowrtest1 partition of wcowrtest for values in (1);
+create view wcowrtest_v as select * from wcowrtest where wcowrtest = '(2)'::wcowrtest with check option;
+insert into wcowrtest_v values (1);
+
+alter table wcowrtest add b text;
+create table wcowrtest2 (b text, c int, a int);
+alter table wcowrtest2 drop c;
+alter table wcowrtest attach partition wcowrtest2 for values in (2);
+
+create table sometable (a int, b text);
+insert into sometable values (1, 'a'), (2, 'b');
+create view wcowrtest_v2 as
+    select *
+      from wcowrtest r
+      where r in (select s from sometable s where r.a = s.a)
+with check option;
+
+-- WITH CHECK qual will be processed with wcowrtest2's
+-- rowtype after tuple-routing
+insert into wcowrtest_v2 values (2, 'no such row in sometable');
+
+drop view wcowrtest_v, wcowrtest_v2;
+drop table wcowrtest, sometable;