refactor preprocess_targetlist - Mailing list pgsql-patches

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().

Barring any objections, I'll apply this to HEAD tonight or tomorrow.

-Neil
Index: src/backend/optimizer/plan/planner.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/backend/optimizer/plan/planner.c,v
retrieving revision 1.179
diff -c -r1.179 planner.c
*** src/backend/optimizer/plan/planner.c    10 Mar 2005 23:21:22 -0000    1.179
--- src/backend/optimizer/plan/planner.c    17 Mar 2005 06:33:03 -0000
***************
*** 36,42 ****
  #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"
--- 36,41 ----
***************
*** 698,762 ****

          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);
!             }
!         }

          /*
           * Generate appropriate target list for subplan; may be different
--- 697,704 ----

          MemSet(&agg_counts, 0, sizeof(AggClauseCounts));

!         /* Preprocess targetlist */
!         tlist = preprocess_targetlist(parse, tlist);

          /*
           * Generate appropriate target list for subplan; may be different
Index: src/backend/optimizer/prep/preptlist.c
===================================================================
RCS file: /var/lib/cvs/pgsql/src/backend/optimizer/prep/preptlist.c,v
retrieving revision 1.72
diff -c -r1.72 preptlist.c
*** src/backend/optimizer/prep/preptlist.c    31 Dec 2004 22:00:20 -0000    1.72
--- src/backend/optimizer/prep/preptlist.c    17 Mar 2005 06:34:37 -0000
***************
*** 26,31 ****
--- 26,33 ----
  #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,51 ****
   *      Returns the new targetlist.
   */
  List *
! preprocess_targetlist(List *tlist,
!                       int command_type,
!                       Index result_relation,
!                       List *range_table)
  {
      /*
       * Sanity check: if there is a result relation, it'd better be a real
       * relation not a subquery.  Else parser or rewriter messed up.
--- 43,54 ----
   *      Returns the new targetlist.
   */
  List *
! 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,104 ****
--- 102,161 ----
          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;
  }

Index: src/include/optimizer/prep.h
===================================================================
RCS file: /var/lib/cvs/pgsql/src/include/optimizer/prep.h,v
retrieving revision 1.47
diff -c -r1.47 prep.h
*** src/include/optimizer/prep.h    31 Dec 2004 22:03:36 -0000    1.47
--- src/include/optimizer/prep.h    17 Mar 2005 06:22:32 -0000
***************
*** 42,49 ****
  /*
   * prototypes for preptlist.c
   */
! extern List *preprocess_targetlist(List *tlist, int command_type,
!                       Index result_relation, List *range_table);

  /*
   * prototypes for prepunion.c
--- 42,48 ----
  /*
   * prototypes for preptlist.c
   */
! extern List *preprocess_targetlist(Query *parse, List *tlist);

  /*
   * prototypes for prepunion.c