This patch moves some code for preprocessing FOR UPDATE from

grouping_planner() to preprocess_targetlist(), according to a comment
in grouping_planner(). I think the refactoring makes sense, and moves
some extraneous details out of grouping_planner().

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index d9e3ad0f84d06fabc423a7b6f985d02c892d5f20..969f3711389e252a8eb52bac9462bf64a10b8ca0 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.179 2005/03/10 23:21:22 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.180 2005/03/17 23:44:26 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -36,7 +36,6 @@
 #include "optimizer/subselect.h"
 #include "optimizer/tlist.h"
 #include "optimizer/var.h"
-#include "parser/analyze.h"
 #include "parser/parsetree.h"
 #include "parser/parse_expr.h"
 #include "parser/parse_oper.h"
@@ -698,65 +697,8 @@ grouping_planner(Query *parse, double tuple_fraction)
 
                MemSet(&agg_counts, 0, sizeof(AggClauseCounts));
 
-               /* Preprocess targetlist in case we are inside an INSERT/UPDATE. */
-               tlist = preprocess_targetlist(tlist,
-                                                                         parse->commandType,
-                                                                         parse->resultRelation,
-                                                                         parse->rtable);
-
-               /*
-                * Add TID targets for rels selected FOR UPDATE (should this be
-                * done in preprocess_targetlist?).  The executor uses the TID to
-                * know which rows to lock, much as for UPDATE or DELETE.
-                */
-               if (parse->rowMarks)
-               {
-                       ListCell   *l;
-
-                       /*
-                        * We've got trouble if the FOR UPDATE appears inside
-                        * grouping, since grouping renders a reference to individual
-                        * tuple CTIDs invalid.  This is also checked at parse time,
-                        * but that's insufficient because of rule substitution, query
-                        * pullup, etc.
-                        */
-                       CheckSelectForUpdate(parse);
-
-                       /*
-                        * Currently the executor only supports FOR UPDATE at top
-                        * level
-                        */
-                       if (PlannerQueryLevel > 1)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-                                                errmsg("SELECT FOR UPDATE is not allowed in subqueries")));
-
-                       foreach(l, parse->rowMarks)
-                       {
-                               Index           rti = lfirst_int(l);
-                               char       *resname;
-                               Resdom     *resdom;
-                               Var                *var;
-                               TargetEntry *ctid;
-
-                               resname = (char *) palloc(32);
-                               snprintf(resname, 32, "ctid%u", rti);
-                               resdom = makeResdom(list_length(tlist) + 1,
-                                                                       TIDOID,
-                                                                       -1,
-                                                                       resname,
-                                                                       true);
-
-                               var = makeVar(rti,
-                                                         SelfItemPointerAttributeNumber,
-                                                         TIDOID,
-                                                         -1,
-                                                         0);
-
-                               ctid = makeTargetEntry(resdom, (Expr *) var);
-                               tlist = lappend(tlist, ctid);
-                       }
-               }
+               /* Preprocess targetlist */
+               tlist = preprocess_targetlist(parse, tlist);
 
                /*
                 * Generate appropriate target list for subplan; may be different

diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index 3ca5195e7cb7a7fcde9683748d700944385b0f80..69dc30c63d5da7a8735eda1c74e532806ae06834 100644 (file)
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.72 2004/12/31 22:00:20 pgsql Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/prep/preptlist.c,v 1.73 2005/03/17 23:44:52 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -26,6 +26,8 @@
 #include "catalog/pg_type.h"
 #include "nodes/makefuncs.h"
 #include "optimizer/prep.h"
+#include "optimizer/subselect.h"
+#include "parser/analyze.h"
 #include "parser/parsetree.h"
 #include "parser/parse_coerce.h"
 
@@ -41,11 +43,12 @@ static List *expand_targetlist(List *tlist, int command_type,
  *       Returns the new targetlist.
  */
 List *
-preprocess_targetlist(List *tlist,
-                                         int command_type,
-                                         Index result_relation,
-                                         List *range_table)
+preprocess_targetlist(Query *parse, List *tlist)
 {
+       int             result_relation = parse->resultRelation;
+       List   *range_table = parse->rtable;
+       CmdType command_type = parse->commandType;
+
        /*
         * Sanity check: if there is a result relation, it'd better be a real
         * relation not a subquery.  Else parser or rewriter messed up.
@@ -99,6 +102,60 @@ preprocess_targetlist(List *tlist,
                tlist = lappend(tlist, makeTargetEntry(resdom, (Expr *) var));
        }
 
+       /*
+        * Add TID targets for rels selected FOR UPDATE.  The executor
+        * uses the TID to know which rows to lock, much as for UPDATE or
+        * DELETE.
+        */
+       if (parse->rowMarks)
+       {
+               ListCell   *l;
+
+               /*
+                * We've got trouble if the FOR UPDATE appears inside
+                * grouping, since grouping renders a reference to individual
+                * tuple CTIDs invalid.  This is also checked at parse time,
+                * but that's insufficient because of rule substitution, query
+                * pullup, etc.
+                */
+               CheckSelectForUpdate(parse);
+
+               /*
+                * Currently the executor only supports FOR UPDATE at top
+                * level
+                */
+               if (PlannerQueryLevel > 1)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                        errmsg("SELECT FOR UPDATE is not allowed in subqueries")));
+
+               foreach(l, parse->rowMarks)
+               {
+                       Index           rti = lfirst_int(l);
+                       char       *resname;
+                       Resdom     *resdom;
+                       Var                *var;
+                       TargetEntry *ctid;
+
+                       resname = (char *) palloc(32);
+                       snprintf(resname, 32, "ctid%u", rti);
+                       resdom = makeResdom(list_length(tlist) + 1,
+                                                               TIDOID,
+                                                               -1,
+                                                               resname,
+                                                               true);
+
+                       var = makeVar(rti,
+                                                 SelfItemPointerAttributeNumber,
+                                                 TIDOID,
+                                                 -1,
+                                                 0);
+
+                       ctid = makeTargetEntry(resdom, (Expr *) var);
+                       tlist = lappend(tlist, ctid);
+               }
+       }
+
        return tlist;
 }
 

diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h
index 5ce9b6cd7667e9b6ada53639e68589d75dc9ea2d..223c86eb1e5813229795e1ed0dc46ceec6fcad07 100644 (file)
--- a/src/include/optimizer/prep.h
+++ b/src/include/optimizer/prep.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.47 2004/12/31 22:03:36 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/optimizer/prep.h,v 1.48 2005/03/17 23:45:09 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -42,8 +42,7 @@ extern Node *flatten_andors(Node *node);
 /*
  * prototypes for preptlist.c
  */
-extern List *preprocess_targetlist(List *tlist, int command_type,
-                                         Index result_relation, List *range_table);
+extern List *preprocess_targetlist(Query *parse, List *tlist);
 
 /*
  * prototypes for prepunion.c