>From 8bd08e9bb9c984d017512536a17b4578f4c1c157 Mon Sep 17 00:00:00 2001 From: Craig Ringer 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