On 2017/07/07 18:47, Amit Langote wrote:
On 2017/07/06 16:06, Etsuro Fujita wrote:
I think this should be fixed. Attached is a patch for that.
Looking up the ResultRelInfo using the proposed method in
ExecFindResultRelInfo() can be unreliable sometimes, that is, the method
of using the root table OID to pin down the RangeTableEntry to get the the
modified columns set.
You are right. Thank you for pointing that out.
How about setting ri_RangeTableIndex of the partition ResultRelInfo
correctly in the first place? If you look at the way InitResultRelInfo()
is called in ExecSetupPartitionTupleRouting(), a dummy RT index of 1 is
passed. We could instead pass the correct one. I think
ModifyTable.nominalRelation is that (at least in the INSERT case.
The attached patch implements that. It modifies
ExecSetupPartitionTupleRouting to accept one more argument resultRTindex
and passes along the same to InitResultRelInfo(). Later when
ExecConstraints() wants to build the modifiedCols set, it looks up the
correct RTE using the partition ResultRelInfo, which has the appropriate
ri_RangeTableIndex set and uses the same.
Looks good to me.
The patch keeps tests that you added in your patch. Added this to the
open items list.
Another thing I noticed is the error handling in ExecWithCheckOptions;
it doesn't take any care for partition tables, so the column description
in the error message for WCO_VIEW_CHECK is built using the partition
table's rowtype, not the root table's. But I think it'd be better to
build that using the root table's rowtype, like ExecConstraints, because
that would make the column description easier to understand since the
parent view(s) (from which WithCheckOptions evaluated there are created)
would have been defined on the root table. This seems independent from
the above issue, so I created a separate patch, which I'm attaching.
What do you think about that?
Best regards,
Etsuro Fujita
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 2097,2102 **** ExecWithCheckOptions(WCOKind kind, ResultRelInfo
*resultRelInfo,
--- 2097,2121 ----
* USING policy.
*/
case WCO_VIEW_CHECK:
+ /* See the comment in
ExecConstraints(). */
+ if (resultRelInfo->ri_PartitionRoot)
+ {
+ HeapTuple tuple =
ExecFetchSlotTuple(slot);
+ TupleDesc old_tupdesc =
RelationGetDescr(rel);
+ TupleConversionMap *map;
+
+ rel =
resultRelInfo->ri_PartitionRoot;
+ tupdesc = RelationGetDescr(rel);
+ /* a reverse map */
+ map =
convert_tuples_by_name(old_tupdesc, tupdesc,
+
gettext_noop("could not convert row type"));
+ if (map != NULL)
+ {
+ tuple =
do_convert_tuple(tuple, map);
+ ExecStoreTuple(tuple,
slot, InvalidBuffer, false);
+ }
+ }
+
insertedCols =
GetInsertedColumns(resultRelInfo, estate);
updatedCols =
GetUpdatedColumns(resultRelInfo, estate);
modifiedCols = bms_union(insertedCols,
updatedCols);
*** a/src/test/regress/expected/updatable_views.out
--- b/src/test/regress/expected/updatable_views.out
***************
*** 2424,2429 **** select tableoid::regclass, * from pt;
create view ptv_wco as select * from pt where a = 0 with check option;
insert into ptv_wco values (1, 2);
ERROR: new row violates check option for view "ptv_wco"
! DETAIL: Failing row contains (2, 1).
drop view ptv, ptv_wco;
drop table pt, pt1, pt11;
--- 2424,2429 ----
create view ptv_wco as select * from pt where a = 0 with check option;
insert into ptv_wco values (1, 2);
ERROR: new row violates check option for view "ptv_wco"
! DETAIL: Failing row contains (1, 2).
drop view ptv, ptv_wco;
drop table pt, pt1, pt11;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
