Thanks Fuita-san and Amit for reviewing.

On 2017/08/02 1:33, Amit Khandekar wrote:
> On 1 August 2017 at 15:11, Etsuro Fujita <fujita.ets...@lab.ntt.co.jp> wrote:
>> On 2017/07/31 18:56, Amit Langote wrote:
>>> Yes, that's what's needed here.  So we need to teach
>>> map_variable_attnos_mutator() to convert whole-row vars just like it's
>>> done in adjust_appendrel_attrs_mutator().
>>
>>
>> Seems reasonable.  (Though I think it might be better to do this kind of
>> conversion in the planner, not the executor, because that would increase the
>> efficiency of cached plans.)

That's a good point, although it sounds like a bigger project that, IMHO,
should be undertaken separately, because that would involve designing for
considerations of expanding inheritance even in the INSERT case.

> I think the work of shifting to planner should be taken as a different
> task when we shift the preparation of DispatchInfo to the planner.

Yeah, I think it'd be a good idea to do those projects together.  That is,
doing what Fujita-san suggested and expanding partitioned tables in
partition bound order in the planner.

>>> Attached 2 patches:
>>>
>>> 0001: Addresses the bug that Rajkumar reported (handling whole-row vars in
>>>        WITH CHECK and RETURNING expressions at all)
>>>
>>> 0002: Addressed the bug that Amit reported (converting whole-row vars
>>>        that could occur in WITH CHECK and RETURNING expressions)
>>
>>
>> I took a quick look at the patches.  One thing I noticed is this:
>>
>>  map_variable_attnos(Node *node,
>>                                         int target_varno, int sublevels_up,
>>                                         const AttrNumber *attno_map, int
>> map_length,
>> +                                       Oid from_rowtype, Oid to_rowtype,
>>                                         bool *found_whole_row)
>>  {
>>         map_variable_attnos_context context;
>> @@ -1291,6 +1318,8 @@ map_variable_attnos(Node *node,
>>         context.sublevels_up = sublevels_up;
>>         context.attno_map = attno_map;
>>         context.map_length = map_length;
>> +       context.from_rowtype = from_rowtype;
>> +       context.to_rowtype = to_rowtype;
>>         context.found_whole_row = found_whole_row;
>>
>> You added two arguments to pass to map_variable_attnos(): from_rowtype and
>> to_rowtype.  But I'm not sure that we really need the from_rowtype argument
>> because it's possible to get the Oid from the vartype of target whole-row
>> Vars.  And in this:
>>
>> +                               /*
>> +                                * If the callers expects us to convert the
>> same, do so if
>> +                                * necessary.
>> +                                */
>> +                               if (OidIsValid(context->to_rowtype) &&
>> +                                       OidIsValid(context->from_rowtype) &&
>> +                                       context->to_rowtype !=
>> context->from_rowtype)
>> +                               {
>> +                                       ConvertRowtypeExpr *r =
>> makeNode(ConvertRowtypeExpr);
>> +
>> +                                       r->arg = (Expr *) newvar;
>> +                                       r->resulttype =
>> context->from_rowtype;
>> +                                       r->convertformat =
>> COERCE_IMPLICIT_CAST;
>> +                                       r->location = -1;
>> +                                       /* Make sure the Var node has the
>> right type ID, too */
>> +                                       newvar->vartype =
>> context->to_rowtype;
>> +                                       return (Node *) r;
>> +                               }
>>
>> I think we could set r->resulttype to the vartype (ie, "r->resulttype =
>> newvar->vartype" instead of "r->resulttype = context->from_rowtype").
> 
> I agree.

You're right, from_rowtype is unnecessary.

> -------
> 
> Few more comments :
> 
> @@ -1240,7 +1247,7 @@ map_variable_attnos_mutator(Node *node,
>  var->varlevelsup == context->sublevels_up)
>  {
>  /* Found a matching variable, make the substitution */
> 
> - Var                *newvar = (Var *) palloc(sizeof(Var));
> + Var                *newvar = copyObject(var);
>   int                     attno = var->varattno;
> 
>  *newvar = *var;
> 
> Here, "*newvar = *var" should be removed.

Done.

> -------
> 
> -       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");
> 
> Instead of callers of map_partition_varattnos() reporting error, we
> can have map_partition_varattnos() itself report error. Instead of the
> found_whole_row parameter of map_partition_varattnos(), we can have
> error_on_whole_row parameter. So callers who don't expect whole row,
> would pass error_on_whole_row=true to map_partition_varattnos(). This
> will simplify the resultant code a bit.

Actually, the first patch I posted on this thread did exactly the same
thing (it added a wholerow_ok argument to map_partition_varattnos similar
in spirit to your error_on_whole_row), but later dropped that idea because
of the ambiguity I felt about the error message text in
map_partition_varattnos().

Actually, unlike other callers of map_variable_attnos(),
map_partition_varattnos *can* in fact convert the whole-row Vars, because
it has all the available information to do so (some of the other callers
don't).  It's just that some callers of map_partition_varattnos() convert
expressions containing only the partition key Vars which cannot be
whole-row Vars, so any whole-row Var that map_variable_attnos() finds is
an unexpected error condition.  So the current text reads: "unexpected
whole-row reference found in partition key".  There are other callers of
map_partition_varattnos() (more will crop up in the future) that don't
necessarily pass expressions containing only the partition key, so having
the above error message sounds odd.  So, I think it's only the callers of
map_partition_varattnos() who know whether finding whole-row vars is an
error and *what kind* of error.  I also think this reasoning is similar to
why map_variable_attnos_mutator() itself does not output error on seeing a
whole-row var.

Attached updated patches.

Thanks,
Amit
From ec2188336c0316bc977e24088659d46b6f157c32 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Wed, 26 Jul 2017 16:45:46 +0900
Subject: [PATCH 1/2] Fix map_partition_varattnos to sometimes accept wholerow
 vars

It was thought that it would never encount wholerow vars in its input
expressions (partition constraint expressions for example).  But then
it was used to convert expressions where wholerow vars are legal, such
as, WCO constraint expressions and RETURNING target list members.  So,
add an argument to tell it whether or not to error on finding wholerows.

Adds test in insert.sql and updatable_views.sql.

Reported by: Rajkumar Raghuwanshi
Report: 
https://postgr.es/m/CAKcux6%3Dz38gH4K6YAFi%2BYvo5tHTwBL4tam4VM33CAPZ5dDMk1Q%40mail.gmail.com
---
 src/backend/catalog/partition.c               | 17 ++++++++++-------
 src/backend/commands/tablecmds.c              |  8 +++++++-
 src/backend/executor/nodeModifyTable.c        | 18 ++++++++++++++----
 src/include/catalog/partition.h               |  3 ++-
 src/test/regress/expected/insert.out          | 10 ++++++++++
 src/test/regress/expected/updatable_views.out | 10 ++++++++++
 src/test/regress/sql/insert.sql               |  6 ++++++
 src/test/regress/sql/updatable_views.sql      |  9 +++++++++
 8 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 9d50efb6a0..3521e9459e 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -904,10 +904,10 @@ get_qual_from_partbound(Relation rel, Relation parent,
  */
 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;
 
        if (expr == NIL)
                return NIL;
@@ -915,14 +915,12 @@ map_partition_varattnos(List *expr, int target_varno,
        part_attnos = convert_tuples_by_name_map(RelationGetDescr(partrel),
                                                                                
         RelationGetDescr(parent),
                                                                                
         gettext_noop("could not convert row type"));
+       *found_whole_row = false;
        expr = (List *) map_variable_attnos((Node *) expr,
                                                                                
target_varno, 0,
                                                                                
part_attnos,
                                                                                
RelationGetDescr(parent)->natts,
-                                                                               
&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");
+                                                                               
found_whole_row);
 
        return expr;
 }
@@ -1783,6 +1781,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 +1824,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);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index bb00858ad1..cc5d3d6faf 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -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);
diff --git a/src/backend/executor/nodeModifyTable.c 
b/src/backend/executor/nodeModifyTable.c
index 0dde0ed6eb..21205244da 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1992,11 +1992,16 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, 
int eflags)
                        List       *mapped_wcoList;
                        List       *wcoExprs = NIL;
                        ListCell   *ll;
+                       bool            found_whole_row;
 
-                       /* varno = node->nominalRelation */
+                       /*
+                        * We are passing node->nominalRelation as the varno.  
wcoList
+                        * might contain whole-row vars which is legal.
+                        */
                        mapped_wcoList = map_partition_varattnos(wcoList,
                                                                                
                         node->nominalRelation,
-                                                                               
                         partrel, rel);
+                                                                               
                         partrel, rel,
+                                                                               
                         &found_whole_row);
                        foreach(ll, mapped_wcoList)
                        {
                                WithCheckOption *wco = 
castNode(WithCheckOption, lfirst(ll));
@@ -2065,11 +2070,16 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, 
int eflags)
                {
                        Relation        partrel = 
resultRelInfo->ri_RelationDesc;
                        List       *rlist;
+                       bool            found_whole_row;
 
-                       /* varno = node->nominalRelation */
+                       /*
+                        * We are passing node->nominalRelation as the varno.
+                        * returningList might contain wholerow vars which is 
legal.
+                        */
                        rlist = map_partition_varattnos(returningList,
                                                                                
        node->nominalRelation,
-                                                                               
        partrel, rel);
+                                                                               
        partrel, rel,
+                                                                               
        &found_whole_row);
                        resultRelInfo->ri_projectReturning =
                                ExecBuildProjectionInfo(rlist, econtext, slot, 
&mtstate->ps,
                                                                                
resultRelInfo->ri_RelationDesc->rd_att);
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h
index f10879a162..434ded37d7 100644
--- a/src/include/catalog/partition.h
+++ b/src/include/catalog/partition.h
@@ -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);
 
diff --git a/src/test/regress/expected/insert.out 
b/src/test/regress/expected/insert.out
index 0dcc86fef4..b642de39d5 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -659,3 +659,13 @@ select tableoid::regclass, * from mcrparted order by a, b;
 (11 rows)
 
 drop table mcrparted;
+-- check that wholerow vars in the RETUNING 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)
+
+drop table returningwrtest;
diff --git a/src/test/regress/expected/updatable_views.out 
b/src/test/regress/expected/updatable_views.out
index eab5c0334c..51a21f10c2 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2428,3 +2428,13 @@ 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).
+drop view wcowrtest_v;
+drop table wcowrtest;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 6adf25da40..2eff832e59 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -399,3 +399,9 @@ 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 RETUNING 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;
+drop table returningwrtest;
diff --git a/src/test/regress/sql/updatable_views.sql 
b/src/test/regress/sql/updatable_views.sql
index 2ede44c02b..af8499a019 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -1141,3 +1141,12 @@ 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);
+drop view wcowrtest_v;
+drop table wcowrtest;
-- 
2.11.0

From 80d2260a95823bbc213b777076f94eb69cb91c6a Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Mon, 31 Jul 2017 17:48:07 +0900
Subject: [PATCH 2/2] Teach map_variable_attnos_mutator to convert whole-row
 Vars

Partitioning code that uses it requires it.
---
 src/backend/catalog/partition.c               |  6 ++--
 src/backend/commands/tablecmds.c              |  4 +--
 src/backend/executor/nodeModifyTable.c        | 10 ++----
 src/backend/parser/parse_utilcmd.c            |  6 ++--
 src/backend/rewrite/rewriteManip.c            | 50 ++++++++++++++++++++-------
 src/include/rewrite/rewriteManip.h            |  2 +-
 src/include/utils/rel.h                       |  7 ++++
 src/test/regress/expected/insert.out          | 10 ++++++
 src/test/regress/expected/updatable_views.out | 20 +++++++++--
 src/test/regress/sql/insert.sql               |  6 ++++
 src/test/regress/sql/updatable_views.sql      | 22 ++++++++++--
 11 files changed, 111 insertions(+), 32 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 3521e9459e..a385c8cf8c 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -908,6 +908,7 @@ map_partition_varattnos(List *expr, int target_varno,
                                                bool *found_whole_row)
 {
        AttrNumber *part_attnos;
+       Oid             to_rowtype;
 
        if (expr == NIL)
                return NIL;
@@ -915,12 +916,13 @@ map_partition_varattnos(List *expr, int target_varno,
        part_attnos = convert_tuples_by_name_map(RelationGetDescr(partrel),
                                                                                
         RelationGetDescr(parent),
                                                                                
         gettext_noop("could not convert row type"));
-       *found_whole_row = false;
+
+       to_rowtype = RelationGetRelType(partrel);
        expr = (List *) map_variable_attnos((Node *) expr,
                                                                                
target_varno, 0,
                                                                                
part_attnos,
                                                                                
RelationGetDescr(parent)->natts,
-                                                                               
found_whole_row);
+                                                                               
found_whole_row, to_rowtype);
 
        return expr;
 }
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cc5d3d6faf..86cefe4696 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -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);
+                                                                               
   &found_whole_row, InvalidOid);
 
                                /*
                                 * 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);
+                                                                               
&found_whole_row, InvalidOid);
                                if (found_whole_row)
                                        ereport(ERROR,
                                                        
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
diff --git a/src/backend/executor/nodeModifyTable.c 
b/src/backend/executor/nodeModifyTable.c
index 21205244da..fecf264b21 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1994,10 +1994,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, 
int eflags)
                        ListCell   *ll;
                        bool            found_whole_row;
 
-                       /*
-                        * We are passing node->nominalRelation as the varno.  
wcoList
-                        * might contain whole-row vars which is legal.
-                        */
+                       /* varno = node->nominalRelation */
                        mapped_wcoList = map_partition_varattnos(wcoList,
                                                                                
                         node->nominalRelation,
                                                                                
                         partrel, rel,
@@ -2072,10 +2069,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, 
int eflags)
                        List       *rlist;
                        bool            found_whole_row;
 
-                       /*
-                        * We are passing node->nominalRelation as the varno.
-                        * returningList might contain wholerow vars which is 
legal.
-                        */
+                       /* varno = node->nominalRelation */
                        rlist = map_partition_varattnos(returningList,
                                                                                
        node->nominalRelation,
                                                                                
        partrel, rel,
diff --git a/src/backend/parser/parse_utilcmd.c 
b/src/backend/parser/parse_utilcmd.c
index 9f37f1b920..f2f0ea540b 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -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);
+                                                                               
         &found_whole_row, InvalidOid);
 
                        /*
                         * 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);
+                                                                               
   &found_whole_row, InvalidOid);
 
                        /* 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);
+                                                                               
&found_whole_row, InvalidOid);
 
                /* As in transformTableLikeClause, reject whole-row variables */
                if (found_whole_row)
diff --git a/src/backend/rewrite/rewriteManip.c 
b/src/backend/rewrite/rewriteManip.c
index b89b435da0..bd3129b7cd 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -1202,15 +1202,18 @@ replace_rte_variables_mutator(Node *node,
  * A zero in the mapping array represents a dropped column, which should not
  * 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.)
+ * Depending on the caller, whole-row Vars in the expression tree are
+ * converted if the necessary information is provided (see below.)  If the
+ * caller cannot provide the information, it might mean that finding a
+ * whole-row Var in the first place is an error.  In that case, we let the
+ * caller know that a whole-row Var was found by returning *found_whole_row
+ * as TRUE, which the caller then can report as an error by providing useful
+ * context information.
+ *
+ * When the information necessary to convert whole-row Vars is present, we
+ * substitute the Var's vartype (replace by to_rowtype) and add a
+ * ConvertRowtypeExpr node to map back to the original vartype.  Currently,
+ * only map_partition_varattnos() requests conversion of whole-row Vars.
  *
  * 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
@@ -1224,6 +1227,8 @@ typedef struct
        const AttrNumber *attno_map;    /* map array for user attnos */
        int                     map_length;             /* number of entries in 
attno_map[] */
        bool       *found_whole_row;    /* output flag */
+       /* Target type when converting whole-row vars */
+       Oid                     to_rowtype;
 } map_variable_attnos_context;
 
 static Node *
@@ -1240,10 +1245,9 @@ map_variable_attnos_mutator(Node *node,
                        var->varlevelsup == context->sublevels_up)
                {
                        /* Found a matching variable, make the substitution */
-                       Var                *newvar = (Var *) 
palloc(sizeof(Var));
+                       Var                *newvar = copyObject(var);
                        int                     attno = var->varattno;
 
-                       *newvar = *var;
                        if (attno > 0)
                        {
                                /* user-defined column, replace attno */
@@ -1257,6 +1261,27 @@ 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))
+                               {
+                                       ConvertRowtypeExpr *r;
+
+                                       /* Var itself is converted to the 
requested rowtype. */
+                                       newvar->vartype = context->to_rowtype;
+
+                                       /*
+                                        * And a conversion step 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 +1308,7 @@ Node *
 map_variable_attnos(Node *node,
                                        int target_varno, int sublevels_up,
                                        const AttrNumber *attno_map, int 
map_length,
-                                       bool *found_whole_row)
+                                       bool *found_whole_row, Oid to_rowtype)
 {
        map_variable_attnos_context context;
 
@@ -1291,6 +1316,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;
diff --git a/src/include/rewrite/rewriteManip.h 
b/src/include/rewrite/rewriteManip.h
index 282ff9967f..56f9bb8013 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -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);
+                                       bool *found_whole_row, Oid to_rowtype);
 
 extern Node *ReplaceVarsFromTargetList(Node *node,
                                                  int target_varno, int 
sublevels_up,
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 4bc61e5380..bd5428f72f 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -444,6 +444,13 @@ typedef struct ViewOptions
        ((relation)->rd_rel->relnamespace)
 
 /*
+ * RelationGetRelType
+ *             Returns the rel's pg_type OID.
+ */
+#define RelationGetRelType(relation) \
+       ((relation)->rd_rel->reltype)
+
+/*
  * RelationIsMapped
  *             True if the relation uses the relfilenode map.
  *
diff --git a/src/test/regress/expected/insert.out 
b/src/test/regress/expected/insert.out
index b642de39d5..38b364d10f 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -668,4 +668,14 @@ insert into returningwrtest values (1) returning 
returningwrtest;
  (1)
 (1 row)
 
+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;
diff --git a/src/test/regress/expected/updatable_views.out 
b/src/test/regress/expected/updatable_views.out
index 51a21f10c2..2090a411fe 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -2436,5 +2436,21 @@ create view wcowrtest_v as select * from wcowrtest where 
wcowrtest = '(2)'::wcow
 insert into wcowrtest_v values (1);
 ERROR:  new row violates check option for view "wcowrtest_v"
 DETAIL:  Failing row contains (1).
-drop view wcowrtest_v;
-drop table wcowrtest;
+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;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index 2eff832e59..f51d72484d 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -404,4 +404,10 @@ drop table mcrparted;
 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;
+
+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;
diff --git a/src/test/regress/sql/updatable_views.sql 
b/src/test/regress/sql/updatable_views.sql
index af8499a019..a6ba5aad9e 100644
--- a/src/test/regress/sql/updatable_views.sql
+++ b/src/test/regress/sql/updatable_views.sql
@@ -1148,5 +1148,23 @@ 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);
-drop view wcowrtest_v;
-drop table wcowrtest;
+
+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;
-- 
2.11.0

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to