Hi all

Here's an updated version of the updatable security barrier views patch
that's proposed as the next stage of progressing row-security toward
useful and maintainable inclusion in core.

If updatable security barrier views are available then the row-security
code won't have to play around with remapping vars and attrs during
query planning when it injects its subquery late. We'll simply stop the
planner flattening an updatable security barrier view, and for
row-security we'll dynamically inject one during rewriting when we see a
relation that has a row-security attribute.

This patch just implements updatable security barrier views. It is a
work in progress with at least two known crash bugs and limited testing.
I'd greatly appreciate comments (and advice) from those who are
interested in the problem domain as we proceed further into work on 9.4.

The patch is attached; it's on top of
46328916eefc5f9eaf249518e96f68afcd35923b, current head. It doens't yet
touch the documentation, but the only change needed should be to remove
the restriction on security_barrier views from the definition of what a
"simply updatable" view is.


There are couple of known issues: a crash with INSERT ... RETURNING;
DEFAULT handling through views isn't implemented yet; and a crash I just
found on UPDATE of a view that re-orders the original table columns. As
a result it doesn't survive "make check" yet.

I'm still working on fixing these issues and on finding more.
Suggestions/comments would be appreciated. I'll post specifics of the
INSERT ... RETURNING one soon, as I'm increasingly stuck on it.


Simple demo:

-- The 'id' is 'integer' not 'serial' because of the limitation with
-- DEFAULT mentioned below.

CREATE TABLE t (id integer primary key, secret text);

INSERT INTO t(id, secret)
SELECT x, 'secret'||x FROM generate_series(1,100) x;

CREATE VIEW t_even AS
SELECT id, secret FROM t WHERE id % 2 = 0;

CREATE VIEW t_even_sb WITH (security_barrier) AS
SELECT id, secret FROM t WHERE id % 2 = 0;

CREATE VIEW t_even_check WITH (check_option = 'cascaded') AS
SELECT id, secret FROM t WHERE id % 2 = 0;

CREATE VIEW t_even_check_sb WITH (check_option = 'cascaded',
security_barrier) AS
SELECT id, secret FROM t WHERE id % 2 = 0;


You'll find that the same f_leak tests used in the original read
security barrier views work here, too.



-- Subsets of cols work:

CREATE VIEW just_id AS SELECT id FROM t;
INSERT INTO just_id(id) VALUES (101);

CREATE VIEW just_id_sb WITH (security_barrier) AS
SELECT id FROM t;
INSERT INTO just_id_sb(id) VALUES (101);


-- Reversed column-lists work:

CREATE VIEW reversed AS SELECT secret, id FROM t;
INSERT INTO reversed(id, secret) VALUES (102, 'fred');

CREATE VIEW reversed_sb WITH (security_barrier) AS
SELECT secret, id FROM t;
INSERT INTO reversed_sb(id, secret) VALUES (102, 'fred');

-- WITH CHECK OPTION is working

postgres=# INSERT INTO t_even_check(id, secret) values (296, 'blah');
INSERT 0 1
postgres=# INSERT INTO t_even_check(id, secret) values (297, 'blah');
ERROR:  new row violates WITH CHECK OPTION for view "t_even_check"
DETAIL:  Failing row contains (297, blah).


postgres=# INSERT INTO t_even_check_sb(id, secret) values (298, 'blah');
INSERT 0 1
postgres=# INSERT INTO t_even_check_sb(id, secret) values (299, 'blah');
ERROR:  new row violates WITH CHECK OPTION for view "t_even_check_sb"
DETAIL:  Failing row contains (299, blah).






-- 3-col views are OK with various permutations

CREATE TABLE t3 ( id integer primary key, aa text, bb text );

CREATE VIEW t3_bai AS SELECT bb, aa, id FROM t3;
INSERT INTO t3_bai VALUES ('bb','aa',3);
UPDATE t3_bai SET bb = 'whatever' WHERE id = 3 RETURNING *;
DELETE FROM t3_bai RETURNING *;


CREATE VIEW t3_bai_sb WITH (security_barrier)
AS SELECT bb, aa, id FROM t3;
INSERT INTO t3_bai_sb VALUES ('bb','aa',3);

-- This crashes, with or without RETURNING. Bug in re-ord
-- UPDATE t3_bai_sb SET bb = 'whatever' WHERE id = 3 RETURNING *;

-- This is OK:
DELETE FROM t3_bai_sb RETURNING *;




-- 










Known issues:

DEFAULT injection doesn't occur correctly through the view. Needs some
changes in the rewriter where expand_target_list is called. Haven't
investigated in detail yet. Causes inserts through views to fail if
there's a default not null constraint, among other issues.

Any INSERT with a RETURNING clause through a view causes a crash (simple
VALUES clauses) or fails with "no relation entry for relid 1" (INSERT
... SELECT). UPDATE and DELETE is fine. Seems to be doing subquery
pull-up, producing a simple result sub-plan that incorrectly has a Var
reference but doesn't perform any scan. Issue traced to plan_subquery,
but no deeper yet.


Privilege enforcement has not yet been through a thorough test matrix.


UPDATE of a subset of columns fails. E.g.:

CREATE VIEW just_secret AS SELECT secret FROM t;
UPDATE just_secret SET secret = 'fred';



Known failing queries. Note that it doesn't matter what you choose from
RETURNING, any reference to a result relation will fail; constant
expressions succeed.

INSERT INTO t_even(id, secret) VALUES (218, 'fred')
RETURNING *;
-- **crash**


INSERT INTO t_even_sb(id, secret) VALUES (218, 'fred')
RETURNING *;
-- **crash**


INSERT INTO t_even_sb
SELECT x, 'secret'||x from generate_series(220,225) x
RETURNING *;
-- ERROR:  no relation entry for relid 1

INSERT INTO t_even
SELECT x, 'secret'||x from generate_series(226,230) x
RETURNING *;
-- ERROR:  no relation entry for relid 1


-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 8bd08e9bb9c984d017512536a17b4578f4c1c157 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Fri, 13 Dec 2013 21:47:22 +0800
Subject: [PATCH] Updatable views WIP 2

---
 src/backend/commands/tablecmds.c       |   6 +-
 src/backend/commands/view.c            |  11 +-
 src/backend/optimizer/plan/planmain.c  |  16 ++
 src/backend/optimizer/plan/setrefs.c   |  12 +-
 src/backend/optimizer/prep/preptlist.c |   9 +-
 src/backend/rewrite/rewriteHandler.c   | 333 +++++++++++++--------------------
 src/include/nodes/relation.h           |  14 +-
 src/include/optimizer/prep.h           |   3 +
 src/include/rewrite/rewriteHandler.h   |   1 -
 9 files changed, 185 insertions(+), 220 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b9cd88d..a984c03 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8806,7 +8806,6 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 		List	   *view_options = untransformRelOptions(newOptions);
 		ListCell   *cell;
 		bool		check_option = false;
-		bool		security_barrier = false;
 
 		foreach(cell, view_options)
 		{
@@ -8814,8 +8813,6 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 
 			if (pg_strcasecmp(defel->defname, "check_option") == 0)
 				check_option = true;
-			if (pg_strcasecmp(defel->defname, "security_barrier") == 0)
-				security_barrier = defGetBoolean(defel);
 		}
 
 		/*
@@ -8825,8 +8822,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 		if (check_option)
 		{
 			const char *view_updatable_error =
-				view_query_is_auto_updatable(view_query,
-											 security_barrier, true);
+				view_query_is_auto_updatable(view_query, true);
 
 			if (view_updatable_error)
 				ereport(ERROR,
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 0703c05..ff84cfb 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -314,8 +314,11 @@ DefineViewRules(Oid viewOid, Query *viewParse, bool replace)
 					   list_make1(viewParse));
 
 	/*
-	 * Someday: automatic ON INSERT, etc
+	 * No automatic ON INSERT, etc is needed, since DML can operate
+	 * directly on the subquery expansion of the SELECT view with
+	 * a little massaging. See the rewriter.
 	 */
+
 }
 
 /*---------------------------------------------------------------
@@ -396,7 +399,6 @@ DefineView(ViewStmt *stmt, const char *queryString)
 	RangeVar   *view;
 	ListCell   *cell;
 	bool		check_option;
-	bool		security_barrier;
 
 	/*
 	 * Run parse analysis to convert the raw parse tree to a Query.  Note this
@@ -451,7 +453,6 @@ DefineView(ViewStmt *stmt, const char *queryString)
 	 * specified.
 	 */
 	check_option = false;
-	security_barrier = false;
 
 	foreach(cell, stmt->options)
 	{
@@ -459,8 +460,6 @@ DefineView(ViewStmt *stmt, const char *queryString)
 
 		if (pg_strcasecmp(defel->defname, "check_option") == 0)
 			check_option = true;
-		if (pg_strcasecmp(defel->defname, "security_barrier") == 0)
-			security_barrier = defGetBoolean(defel);
 	}
 
 	/*
@@ -470,7 +469,7 @@ DefineView(ViewStmt *stmt, const char *queryString)
 	if (check_option)
 	{
 		const char *view_updatable_error =
-			view_query_is_auto_updatable(viewParse, security_barrier, true);
+			view_query_is_auto_updatable(viewParse, true);
 
 		if (view_updatable_error)
 			ereport(ERROR,
diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index 3a4e836..5bd7e31 100644
--- a/src/backend/optimizer/plan/planmain.c
+++ b/src/backend/optimizer/plan/planmain.c
@@ -126,6 +126,22 @@ query_planner(PlannerInfo *root, List *tlist,
 	add_base_rels_to_query(root, (Node *) parse->jointree);
 
 	/*
+	 * The top query's resultRelation RTI may not be referenced in the query
+	 * tree its self, so we need to ensure it gets cached too. Curently arises
+	 * when we're doing DML on a view, where the injected top-level RTE for the
+	 * view's base rel isn't referenced in the query tree.
+	 *
+	 * It is marked RELOPT_RESULTREL because it isn't quite "dead" - it
+	 * must appear in the top level flattened range table of the plan tree
+	 * - but neither does it appear in the join tree and shouldn't be
+	 *   considered in path generation.
+	 */
+	if (root->parse->resultRelation && root->simple_rel_array[root->parse->resultRelation] == NULL)
+		(void) build_simple_rel(root, root->parse->resultRelation, RELOPT_RESULTREL);
+
+	Assert(root->parse->resultRelation == 0 || root->simple_rel_array[root->parse->resultRelation] != NULL);
+
+	/*
 	 * Examine the targetlist and join tree, adding entries to baserel
 	 * targetlists for all referenced Vars, and generating PlaceHolderInfo
 	 * entries for all referenced PlaceHolderVars.	Restrict and join clauses
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 5c9f3d6..f503b83 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -268,6 +268,9 @@ add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing)
 	 * Note: this pass over the rangetable can't be combined with the previous
 	 * one, because that would mess up the numbering of the live RTEs in the
 	 * flattened rangetable.
+	 *
+	 * This *only* scans the range-table; it won't see dead subqueries in
+	 * WHERE clauses, etc.
 	 */
 	rti = 1;
 	foreach(lc, root->parse->rtable)
@@ -279,8 +282,15 @@ add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing)
 		 * pulled up into our rangetable already.  Also ignore any subquery
 		 * RTEs without matching RelOptInfos, as they likewise have been
 		 * pulled up.
+		 *
+		 * Testing simple_rel_array_size here ensures that we don't try to access
+		 * root->simple_rel_array when it isn't defined in the simple-query fast-path
+		 * where the join tree is empty - see query_planner(...) in planmain.c.
+		 * This code only ever worked without this test because we don't add dead
+		 * subqueries in WHERE/HAVING/etc to the range-table so if the join tree
+		 * was empty this condition could never be true.
 		 */
-		if (rte->rtekind == RTE_SUBQUERY && !rte->inh)
+		if (rte->rtekind == RTE_SUBQUERY && !rte->inh && root->simple_rel_array_size != 0)
 		{
 			RelOptInfo *rel = root->simple_rel_array[rti];
 
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index fb67f9e..80db564 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -35,11 +35,6 @@
 #include "parser/parse_coerce.h"
 #include "utils/rel.h"
 
-
-static List *expand_targetlist(List *tlist, int command_type,
-				  Index result_relation, List *range_table);
-
-
 /*
  * preprocess_targetlist
  *	  Driver for preprocessing the parse tree targetlist.
@@ -58,6 +53,8 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
 	/*
 	 * Sanity check: if there is a result relation, it'd better be a real
 	 * relation not a subquery.  Else parser or rewriter messed up.
+	 * This is true even if we're updating a view/subquery; the result_relation
+	 * must still be the underlying relation RTE.
 	 */
 	if (result_relation)
 	{
@@ -193,7 +190,7 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
  *	  add targetlist entries for any missing attributes, and ensure the
  *	  non-junk attributes appear in proper field order.
  */
-static List *
+List *
 expand_targetlist(List *tlist, int command_type,
 				  Index result_relation, List *range_table)
 {
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 50cb753..bb32e45 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -28,6 +28,7 @@
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
+#include "nodes/print.h" /* Temporary for elog_print_display only */
 
 
 /* We use a list of these to detect recursion in RewriteQuery */
@@ -1973,7 +1974,7 @@ view_col_is_auto_updatable(RangeTblRef *rtr, TargetEntry *tle)
  * updatable.
  */
 const char *
-view_query_is_auto_updatable(Query *viewquery, bool security_barrier,
+view_query_is_auto_updatable(Query *viewquery,
 							 bool check_cols)
 {
 	RangeTblRef *rtr;
@@ -2048,14 +2049,6 @@ view_query_is_auto_updatable(Query *viewquery, bool security_barrier,
 		return gettext_noop("Views that return set-returning functions are not automatically updatable.");
 
 	/*
-	 * For now, we also don't support security-barrier views, because of the
-	 * difficulty of keeping upper-level qual expressions away from
-	 * lower-level data.  This might get relaxed in the future.
-	 */
-	if (security_barrier)
-		return gettext_noop("Security-barrier views are not automatically updatable.");
-
-	/*
 	 * The view query should select from a single base relation, which must be
 	 * a table or another view.
 	 */
@@ -2304,7 +2297,6 @@ relation_is_updatable(Oid reloid,
 		Query	   *viewquery = get_view_query(rel);
 
 		if (view_query_is_auto_updatable(viewquery,
-										 RelationIsSecurityView(rel),
 										 false) == NULL)
 		{
 			Bitmapset  *updatable_cols;
@@ -2452,15 +2444,14 @@ rewriteTargetView(Query *parsetree, Relation view)
 	RangeTblEntry *view_rte;
 	RangeTblEntry *new_rte;
 	Relation	base_rel;
-	List	   *view_targetlist;
 	ListCell   *lc;
+	const int command_type = parsetree->commandType;
 
 	/* The view must be updatable, else fail */
 	viewquery = get_view_query(view);
 
 	auto_update_detail =
 		view_query_is_auto_updatable(viewquery,
-									 RelationIsSecurityView(view),
 									 parsetree->commandType != CMD_DELETE);
 
 	if (auto_update_detail)
@@ -2589,15 +2580,32 @@ rewriteTargetView(Query *parsetree, Relation view)
 	heap_close(base_rel, NoLock);
 
 	/*
+	 * For UPDATE, add references to all columns of the base relation to the
+	 * expanded subquery if they weren't already present. We need all cols of
+	 * the base relation to generate the new tuple, even though they aren't
+	 * visible directly to the view user. These must not be revealed in
+	 * a RETURNING clause, and do *not* need the select permission bit on them
+	 * since the user won't ever actually see them.
+	 *
+	 * There's no need to sort these into the proper attribute ordering;
+	 * that'll be done for us in the next rewrite pass.
+	 */
+	if (command_type == CMD_INSERT || command_type == CMD_UPDATE)
+		parsetree->targetList = expand_targetlist(parsetree->targetList, command_type, 
+						  						  parsetree->resultRelation, parsetree->rtable);
+
+	/*
 	 * Create a new target RTE describing the base relation, and add it to the
-	 * outer query's rangetable.  (What's happening in the next few steps is
-	 * very much like what the planner would do to "pull up" the view into the
-	 * outer query.  Perhaps someday we should refactor things enough so that
-	 * we can share code with the planner.)
+	 * outer query's rangetable, then set it as the targetRelation for the query.
+	 * 
+	 * We don't do any pull-up here; the optimizer will do it for non-security-barrier
+	 * views later if it feels like it. The purpose of this RTE is to be the targetRelation,
+	 * and to be the vehicle for write-permission checks against this view.
 	 */
 	new_rte = (RangeTblEntry *) copyObject(base_rte);
 	parsetree->rtable = lappend(parsetree->rtable, new_rte);
 	new_rt_index = list_length(parsetree->rtable);
+	parsetree->resultRelation = new_rt_index;
 
 	/*
 	 * INSERTs never inherit.  For UPDATE/DELETE, we use the view query's
@@ -2607,21 +2615,6 @@ rewriteTargetView(Query *parsetree, Relation view)
 		new_rte->inh = false;
 
 	/*
-	 * Make a copy of the view's targetlist, adjusting its Vars to reference
-	 * the new target RTE, ie make their varnos be new_rt_index instead of
-	 * base_rt_index.  There can be no Vars for other rels in the tlist, so
-	 * this is sufficient to pull up the tlist expressions for use in the
-	 * outer query.  The tlist will provide the replacement expressions used
-	 * by ReplaceVarsFromTargetList below.
-	 */
-	view_targetlist = copyObject(viewquery->targetList);
-
-	ChangeVarNodes((Node *) view_targetlist,
-				   base_rt_index,
-				   new_rt_index,
-				   0);
-
-	/*
 	 * Mark the new target RTE for the permissions checks that we want to
 	 * enforce against the view owner, as distinct from the query caller.  At
 	 * the relation level, require the same INSERT/UPDATE/DELETE permissions
@@ -2650,179 +2643,123 @@ rewriteTargetView(Query *parsetree, Relation view)
 	 * that does not correspond to what happens in ordinary SELECT usage of a
 	 * view: all referenced columns must have read permission, even if
 	 * optimization finds that some of them can be discarded during query
-	 * transformation.	The flattening we're doing here is an optional
-	 * optimization, too.  (If you are unpersuaded and want to change this,
-	 * note that applying adjust_view_column_set to view_rte->selectedCols is
-	 * clearly *not* the right answer, since that neglects base-rel columns
-	 * used in the view's WHERE quals.)
+	 * transformation.
+	 *
+	 * (If you are unpersuaded and want to change this, note that applying
+	 * adjust_view_column_set to view_rte->selectedCols is clearly *not*
+	 * the right answer, since that neglects base-rel columns used in the
+	 * view's WHERE quals.)
 	 *
 	 * This step needs the modified view targetlist, so we have to do things
 	 * in this order.
 	 */
 	Assert(bms_is_empty(new_rte->modifiedCols));
 	new_rte->modifiedCols = adjust_view_column_set(view_rte->modifiedCols,
-												   view_targetlist);
-
-	/*
-	 * For UPDATE/DELETE, rewriteTargetListUD will have added a wholerow junk
-	 * TLE for the view to the end of the targetlist, which we no longer need.
-	 * Remove it to avoid unnecessary work when we process the targetlist.
-	 * Note that when we recurse through rewriteQuery a new junk TLE will be
-	 * added to allow the executor to find the proper row in the new target
-	 * relation.  (So, if we failed to do this, we might have multiple junk
-	 * TLEs with the same name, which would be disastrous.)
-	 */
-	if (parsetree->commandType != CMD_INSERT)
-	{
-		TargetEntry *tle = (TargetEntry *) llast(parsetree->targetList);
-
-		Assert(tle->resjunk);
-		Assert(IsA(tle->expr, Var) &&
-			   ((Var *) tle->expr)->varno == parsetree->resultRelation &&
-			   ((Var *) tle->expr)->varattno == 0);
-		parsetree->targetList = list_delete_ptr(parsetree->targetList, tle);
-	}
-
-	/*
-	 * Now update all Vars in the outer query that reference the view to
-	 * reference the appropriate column of the base relation instead.
-	 */
-	parsetree = (Query *)
-		ReplaceVarsFromTargetList((Node *) parsetree,
-								  parsetree->resultRelation,
-								  0,
-								  view_rte,
-								  view_targetlist,
-								  REPLACEVARS_REPORT_ERROR,
-								  0,
-								  &parsetree->hasSubLinks);
-
-	/*
-	 * Update all other RTI references in the query that point to the view
-	 * (for example, parsetree->resultRelation itself) to point to the new
-	 * base relation instead.  Vars will not be affected since none of them
-	 * reference parsetree->resultRelation any longer.
-	 */
-	ChangeVarNodes((Node *) parsetree,
-				   parsetree->resultRelation,
-				   new_rt_index,
-				   0);
-	Assert(parsetree->resultRelation == new_rt_index);
-
-	/*
-	 * For INSERT/UPDATE we must also update resnos in the targetlist to refer
-	 * to columns of the base relation, since those indicate the target
-	 * columns to be affected.
-	 *
-	 * Note that this destroys the resno ordering of the targetlist, but that
-	 * will be fixed when we recurse through rewriteQuery, which will invoke
-	 * rewriteTargetListIU again on the updated targetlist.
-	 */
-	if (parsetree->commandType != CMD_DELETE)
-	{
-		foreach(lc, parsetree->targetList)
-		{
-			TargetEntry *tle = (TargetEntry *) lfirst(lc);
-			TargetEntry *view_tle;
-
-			if (tle->resjunk)
-				continue;
-
-			view_tle = get_tle_by_resno(view_targetlist, tle->resno);
-			if (view_tle != NULL && !view_tle->resjunk && IsA(view_tle->expr, Var))
-				tle->resno = ((Var *) view_tle->expr)->varattno;
-			else
-				elog(ERROR, "attribute number %d not found in view targetlist",
-					 tle->resno);
-		}
-	}
-
-	/*
-	 * For UPDATE/DELETE, pull up any WHERE quals from the view.  We know that
-	 * any Vars in the quals must reference the one base relation, so we need
-	 * only adjust their varnos to reference the new target (just the same as
-	 * we did with the view targetlist).
-	 *
-	 * For INSERT, the view's quals can be ignored in the main query.
-	 */
-	if (parsetree->commandType != CMD_INSERT &&
-		viewquery->jointree->quals != NULL)
-	{
-		Node	   *viewqual = (Node *) copyObject(viewquery->jointree->quals);
-
-		ChangeVarNodes(viewqual, base_rt_index, new_rt_index, 0);
-		AddQual(parsetree, (Node *) viewqual);
-	}
-
-	/*
-	 * For INSERT/UPDATE, if the view has the WITH CHECK OPTION, or any parent
-	 * view specified WITH CASCADED CHECK OPTION, add the quals from the view
-	 * to the query's withCheckOptions list.
-	 */
-	if (parsetree->commandType != CMD_DELETE)
-	{
-		bool		has_wco = RelationHasCheckOption(view);
-		bool		cascaded = RelationHasCascadedCheckOption(view);
-
-		/*
-		 * If the parent view has a cascaded check option, treat this view as
-		 * if it also had a cascaded check option.
-		 *
-		 * New WithCheckOptions are added to the start of the list, so if there
-		 * is a cascaded check option, it will be the first item in the list.
-		 */
-		if (parsetree->withCheckOptions != NIL)
-		{
-			WithCheckOption *parent_wco =
-				(WithCheckOption *) linitial(parsetree->withCheckOptions);
-
-			if (parent_wco->cascaded)
-			{
-				has_wco = true;
-				cascaded = true;
-			}
-		}
-
-		/*
-		 * Add the new WithCheckOption to the start of the list, so that
-		 * checks on inner views are run before checks on outer views, as
-		 * required by the SQL standard.
-		 *
-		 * If the new check is CASCADED, we need to add it even if this view
-		 * has no quals, since there may be quals on child views.  A LOCAL
-		 * check can be omitted if this view has no quals.
-		 */
-		if (has_wco && (cascaded || viewquery->jointree->quals != NULL))
-		{
-			WithCheckOption *wco;
-
-			wco = makeNode(WithCheckOption);
-			wco->viewname = pstrdup(RelationGetRelationName(view));
-			wco->qual = NULL;
-			wco->cascaded = cascaded;
-
-			parsetree->withCheckOptions = lcons(wco,
-												parsetree->withCheckOptions);
-
-			if (viewquery->jointree->quals != NULL)
-			{
-				wco->qual = (Node *) copyObject(viewquery->jointree->quals);
-				ChangeVarNodes(wco->qual, base_rt_index, new_rt_index, 0);
-
-				/*
-				 * Make sure that the query is marked correctly if the added
-				 * qual has sublinks.  We can skip this check if the query is
-				 * already marked, or if the command is an UPDATE, in which
-				 * case the same qual will have already been added to the
-				 * query's WHERE clause, and AddQual will have already done
-				 * this check.
-				 */
-				if (!parsetree->hasSubLinks &&
-					parsetree->commandType != CMD_UPDATE)
-					parsetree->hasSubLinks = checkExprHasSubLink(wco->qual);
-			}
-		}
-	}
+												   viewquery->targetList);
+
+ 	/*
+ 	 * For INSERT/UPDATE we must also update resnos in the targetlist to refer
+ 	 * to columns of the base relation, since those indicate the target
+ 	 * columns to be affected.
+ 	 *
+ 	 * Note that this destroys the resno ordering of the targetlist, but that
+ 	 * will be fixed when we recurse through rewriteQuery, which will invoke
+ 	 * rewriteTargetListIU again on the updated targetlist.
+ 	 *
+ 	 * Failure to perform this step would cause UPDATEs or INSERTs on 
+ 	 * a view with a different column ordering to the base relation to fail.
+ 	 */
+ 	if (parsetree->commandType != CMD_DELETE)
+ 	{
+ 		foreach(lc, parsetree->targetList)
+ 		{
+ 			TargetEntry *tle = (TargetEntry *) lfirst(lc);
+ 			TargetEntry *view_tle;
+ 
+ 			if (tle->resjunk)
+ 				continue;
+ 
+ 			view_tle = get_tle_by_resno(viewquery->targetList, tle->resno);
+ 			if (view_tle != NULL && !view_tle->resjunk && IsA(view_tle->expr, Var))
+ 				tle->resno = ((Var *) view_tle->expr)->varattno;
+ 			else
+ 				elog(ERROR, "attribute number %d not found in view targetlist",
+ 					 tle->resno);
+ 		}
+ 	}
+ 
+ 	/*
+ 	 * For INSERT/UPDATE, if the view has the WITH CHECK OPTION, or any parent
+ 	 * view specified WITH CASCADED CHECK OPTION, add the quals from the view
+ 	 * to the query's withCheckOptions list.
+ 	 */
+ 	if (parsetree->commandType != CMD_DELETE)
+ 	{
+ 		bool		has_wco = RelationHasCheckOption(view);
+ 		bool		cascaded = RelationHasCascadedCheckOption(view);
+ 
+ 		/*
+ 		 * If the parent view has a cascaded check option, treat this view as
+ 		 * if it also had a cascaded check option.
+ 		 *
+ 		 * New WithCheckOptions are added to the start of the list, so if there
+ 		 * is a cascaded check option, it will be the first item in the list.
+ 		 */
+ 		if (parsetree->withCheckOptions != NIL)
+ 		{
+ 			WithCheckOption *parent_wco =
+ 				(WithCheckOption *) linitial(parsetree->withCheckOptions);
+ 
+ 			if (parent_wco->cascaded)
+ 			{
+ 				has_wco = true;
+ 				cascaded = true;
+ 			}
+ 		}
+ 
+ 		/*
+ 		 * Add the new WithCheckOption to the start of the list, so that
+ 		 * checks on inner views are run before checks on outer views, as
+ 		 * required by the SQL standard.
+ 		 *
+ 		 * If the new check is CASCADED, we need to add it even if this view
+ 		 * has no quals, since there may be quals on child views.  A LOCAL
+ 		 * check can be omitted if this view has no quals.
+ 		 */
+ 		if (has_wco && (cascaded || viewquery->jointree->quals != NULL))
+ 		{
+ 			WithCheckOption *wco;
+ 
+ 			wco = makeNode(WithCheckOption);
+ 			wco->viewname = pstrdup(RelationGetRelationName(view));
+ 			wco->qual = NULL;
+ 			wco->cascaded = cascaded;
+ 
+ 			parsetree->withCheckOptions = lcons(wco,
+ 												parsetree->withCheckOptions);
+ 
+ 			if (viewquery->jointree->quals != NULL)
+ 			{
+ 				wco->qual = (Node *) copyObject(viewquery->jointree->quals);
+ 				ChangeVarNodes(wco->qual, base_rt_index, new_rt_index, 0);
+ 
+ 				/*
+ 				 * Make sure that the query is marked correctly if the added
+ 				 * qual has sublinks.  We can skip this check if the query is
+ 				 * already marked, or if the command is an UPDATE, in which
+ 				 * case the same qual will have already been added to the
+ 				 * query's WHERE clause, and AddQual will have already done
+ 				 * this check.
+ 				 */
+ 				if (!parsetree->hasSubLinks &&
+ 					parsetree->commandType != CMD_UPDATE)
+ 					parsetree->hasSubLinks = checkExprHasSubLink(wco->qual);
+ 			}
+ 		}
+ 	}
+
+    elog_node_display(LOG, "parse_tree after upd view rewrite", parsetree,
+                      true);
 
 	return parsetree;
 }
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 6d7b594..70c2972 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -275,7 +275,7 @@ typedef struct PlannerInfo
  * is the joining of two or more base rels.  A joinrel is identified by
  * the set of RT indexes for its component baserels.  We create RelOptInfo
  * nodes for each baserel and joinrel, and store them in the PlannerInfo's
- * simple_rel_array and join_rel_list respectively.
+ * simple_rel_array and join_rel_list respectively. 
  *
  * Note that there is only one joinrel for any given set of component
  * baserels, no matter what order we assemble them in; so an unordered
@@ -283,10 +283,17 @@ typedef struct PlannerInfo
  *
  * We also have "other rels", which are like base rels in that they refer to
  * single RT indexes; but they are not part of the join tree, and are given
- * a different RelOptKind to identify them.  Lastly, there is a RelOptKind
+ * a different RelOptKind to identify them.  There is also a RelOptKind
  * for "dead" relations, which are base rels that we have proven we don't
  * need to join after all.
  *
+ * A "resultrel" (RELOPT_RESULTREL) is a relation that doesn't appear in the join
+ * tree, and is used only as the target for the ModifyTable node in an
+ * INSERT/UPDATE/DELETE. This arises when we're updating a view or subquery, as
+ * the RTE for the target is buried in a subquery and the ModifyTable node
+ * needs one at the top level. The injected RTE is marked RELOPT_RESULTREL
+ * so it isn't considered in join tree sanity checking, etc.
+ *
  * Currently the only kind of otherrels are those made for member relations
  * of an "append relation", that is an inheritance set or UNION ALL subquery.
  * An append relation has a parent RTE that is a base rel, which represents
@@ -404,7 +411,8 @@ typedef enum RelOptKind
 	RELOPT_BASEREL,
 	RELOPT_JOINREL,
 	RELOPT_OTHER_MEMBER_REL,
-	RELOPT_DEADREL
+	RELOPT_DEADREL,
+	RELOPT_RESULTREL
 } RelOptKind;
 
 typedef struct RelOptInfo
diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h
index 0934e63..e37b8a5 100644
--- a/src/include/optimizer/prep.h
+++ b/src/include/optimizer/prep.h
@@ -40,6 +40,9 @@ extern Expr *canonicalize_qual(Expr *qual);
  */
 extern List *preprocess_targetlist(PlannerInfo *root, List *tlist);
 
+extern List *expand_targetlist(List *tlist, int command_type,
+							   Index result_relation, List *range_table);
+
 extern PlanRowMark *get_plan_rowmark(List *rowmarks, Index rtindex);
 
 /*
diff --git a/src/include/rewrite/rewriteHandler.h b/src/include/rewrite/rewriteHandler.h
index c959590..ce56d7b 100644
--- a/src/include/rewrite/rewriteHandler.h
+++ b/src/include/rewrite/rewriteHandler.h
@@ -23,7 +23,6 @@ extern void AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown);
 extern Node *build_column_default(Relation rel, int attrno);
 extern Query *get_view_query(Relation view);
 extern const char *view_query_is_auto_updatable(Query *viewquery,
-										 bool security_barrier,
 										 bool check_cols);
 extern int	relation_is_updatable(Oid reloid,
 						  bool include_triggers,
-- 
1.8.3.1

-- 
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