Re: Writeable CTE patch - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Writeable CTE patch
Date
Msg-id 2888.1259343722@sss.pgh.pa.us
Whole thread Raw
In response to Re: Writeable CTE patch  (Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi>)
Responses Re: Writeable CTE patch
List pgsql-hackers
Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes:
> Tom Lane wrote:
>> since OIDs in user tables have been deprecated for several versions
>> now, I'm thinking that maybe the case doesn't arise often enough to
>> justify keeping such a wart in the executor.

> Under the circumstances I'd lean towards this option.

I've been fooling around with this further and have gotten as far as
the attached patch.  It passes regression tests but suffers from an
additional performance loss: the physical-tlist optimization is disabled
when scanning a relation having OIDs.  (That is, we'll always use
ExecProject even if the scan is "SELECT * FROM ...".)  I think this loss
is worth worrying about since it would apply to queries on system
catalogs, even if the database has no OIDs in user tables.  The trick
is to make the knowledge of the required hasoid state available at
ExecAssignResultType time, so that the plan node's result tupdesc is
constructed correctly.

What seems like the best bet is to merge ExecAssignResultTypeFromTL
and ExecAssignScanProjectionInfo into a single function that should
be used by scan node types.  It'll do the determination of whether
a physical-tlist optimization is possible, and then set up both the
output tupdesc and the projection info accordingly.  This will make the
patch diff a good bit longer but not much more interesting, so I'm
sending it along at this stage.

I think this is worth doing since it cleans up one of the grottier
parts of executor initialization.  The whole thing around
ExecContextForcesOids was never pretty, and it's been the source of
more than one bug if memory serves.

Comments?

            regards, tom lane
Index: src/backend/commands/copy.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.318
diff -c -r1.318 copy.c
*** src/backend/commands/copy.c    20 Nov 2009 20:38:10 -0000    1.318
--- src/backend/commands/copy.c    27 Nov 2009 17:21:55 -0000
***************
*** 1811,1817 ****

      estate->es_result_relations = resultRelInfo;
      estate->es_num_result_relations = 1;
-     estate->es_result_relation_info = resultRelInfo;

      /* Set up a tuple slot too */
      slot = ExecInitExtraTupleSlot(estate);
--- 1811,1816 ----
***************
*** 2165,2171 ****
              heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);

              if (resultRelInfo->ri_NumIndices > 0)
!                 recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
                                                         estate, false);

              /* AFTER ROW INSERT Triggers */
--- 2164,2171 ----
              heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);

              if (resultRelInfo->ri_NumIndices > 0)
!                 recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
!                                                        slot, &(tuple->t_self),
                                                         estate, false);

              /* AFTER ROW INSERT Triggers */
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.306
diff -c -r1.306 tablecmds.c
*** src/backend/commands/tablecmds.c    20 Nov 2009 20:38:10 -0000    1.306
--- src/backend/commands/tablecmds.c    27 Nov 2009 17:21:55 -0000
***************
*** 941,947 ****
      resultRelInfo = resultRelInfos;
      foreach(cell, rels)
      {
-         estate->es_result_relation_info = resultRelInfo;
          ExecBSTruncateTriggers(estate, resultRelInfo);
          resultRelInfo++;
      }
--- 941,946 ----
***************
*** 1009,1015 ****
      resultRelInfo = resultRelInfos;
      foreach(cell, rels)
      {
-         estate->es_result_relation_info = resultRelInfo;
          ExecASTruncateTriggers(estate, resultRelInfo);
          resultRelInfo++;
      }
--- 1008,1013 ----
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.396
diff -c -r1.396 vacuum.c
*** src/backend/commands/vacuum.c    16 Nov 2009 21:32:06 -0000    1.396
--- src/backend/commands/vacuum.c    27 Nov 2009 17:21:55 -0000
***************
*** 181,187 ****

      ec->estate->es_result_relations = ec->resultRelInfo;
      ec->estate->es_num_result_relations = 1;
-     ec->estate->es_result_relation_info = ec->resultRelInfo;

      /* Set up a tuple slot too */
      ec->slot = MakeSingleTupleTableSlot(tupdesc);
--- 181,186 ----
***************
*** 3099,3105 ****
      if (ec->resultRelInfo->ri_NumIndices > 0)
      {
          ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
!         ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
          ResetPerTupleExprContext(ec->estate);
      }
  }
--- 3098,3105 ----
      if (ec->resultRelInfo->ri_NumIndices > 0)
      {
          ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
!         ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self),
!                               ec->estate, true);
          ResetPerTupleExprContext(ec->estate);
      }
  }
***************
*** 3225,3231 ****
      if (ec->resultRelInfo->ri_NumIndices > 0)
      {
          ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
!         ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
          ResetPerTupleExprContext(ec->estate);
      }
  }
--- 3225,3232 ----
      if (ec->resultRelInfo->ri_NumIndices > 0)
      {
          ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
!         ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self),
!                               ec->estate, true);
          ResetPerTupleExprContext(ec->estate);
      }
  }
Index: src/backend/executor/execJunk.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execJunk.c,v
retrieving revision 1.59
diff -c -r1.59 execJunk.c
*** src/backend/executor/execJunk.c    10 Oct 2009 01:43:45 -0000    1.59
--- src/backend/executor/execJunk.c    27 Nov 2009 17:21:56 -0000
***************
*** 34,40 ****
   * called 'resjunk'. If the value of this field is true then the
   * corresponding attribute is a "junk" attribute.
   *
!  * When we initialize a plan we call ExecInitJunkFilter to create a filter.
   *
   * We then execute the plan, treating the resjunk attributes like any others.
   *
--- 34,42 ----
   * called 'resjunk'. If the value of this field is true then the
   * corresponding attribute is a "junk" attribute.
   *
!  * When we initialize a plan we call ExecInitJunkFilter to create a filter,
!  * if one is needed.  (We need one either if there are junk attributes,
!  * or if we have to add or remove an OID column from the result rowtype.)
   *
   * We then execute the plan, treating the resjunk attributes like any others.
   *
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.335
diff -c -r1.335 execMain.c
*** src/backend/executor/execMain.c    20 Nov 2009 20:38:10 -0000    1.335
--- src/backend/executor/execMain.c    27 Nov 2009 17:21:56 -0000
***************
*** 339,345 ****
      /*
       * Close the SELECT INTO relation if any
       */
!     if (estate->es_select_into)
          CloseIntoRel(queryDesc);

      /* do away with our snapshots */
--- 339,345 ----
      /*
       * Close the SELECT INTO relation if any
       */
!     if (queryDesc->dest && queryDesc->dest->mydest == DestIntoRel)
          CloseIntoRel(queryDesc);

      /* do away with our snapshots */
***************
*** 624,629 ****
--- 624,630 ----
      EState       *estate = queryDesc->estate;
      PlanState  *planstate;
      TupleDesc    tupType;
+     bool        select_into;
      ListCell   *l;
      int            i;

***************
*** 671,678 ****
          }
          estate->es_result_relations = resultRelInfos;
          estate->es_num_result_relations = numResultRelations;
-         /* es_result_relation_info is NULL except when within ModifyTable */
-         estate->es_result_relation_info = NULL;
      }
      else
      {
--- 672,677 ----
***************
*** 681,687 ****
           */
          estate->es_result_relations = NULL;
          estate->es_num_result_relations = 0;
-         estate->es_result_relation_info = NULL;
      }

      /*
--- 680,685 ----
***************
*** 736,751 ****
      }

      /*
!      * Detect whether we're doing SELECT INTO.  If so, set the es_into_oids
!      * flag appropriately so that the plan tree will be initialized with the
!      * correct tuple descriptors.  (Other SELECT INTO stuff comes later.)
       */
!     estate->es_select_into = false;
!     if (operation == CMD_SELECT && plannedstmt->intoClause != NULL)
!     {
!         estate->es_select_into = true;
!         estate->es_into_oids = interpretOidsOption(plannedstmt->intoClause->options);
!     }

      /*
       * Initialize the executor's tuple table to empty.
--- 734,742 ----
      }

      /*
!      * Detect whether we're doing SELECT INTO.
       */
!     select_into = (operation == CMD_SELECT && plannedstmt->intoClause != NULL);

      /*
       * Initialize the executor's tuple table to empty.
***************
*** 804,825 ****
      tupType = ExecGetResultType(planstate);

      /*
!      * Initialize the junk filter if needed.  SELECT queries need a
!      * filter if there are any junk attrs in the top-level tlist.
       */
      if (operation == CMD_SELECT)
      {
          bool        junk_filter_needed = false;
!         ListCell   *tlist;

!         foreach(tlist, plan->targetlist)
          {
!             TargetEntry *tle = (TargetEntry *) lfirst(tlist);

!             if (tle->resjunk)
              {
!                 junk_filter_needed = true;
!                 break;
              }
          }

--- 795,832 ----
      tupType = ExecGetResultType(planstate);

      /*
!      * Initialize the junk filter if needed.  SELECT queries need a filter
!      * if there are any junk attrs in the top-level tlist, or if we're doing
!      * SELECT INTO and the result type doesn't match the required hasOids
!      * status.
       */
      if (operation == CMD_SELECT)
      {
          bool        junk_filter_needed = false;
!         bool        junk_filter_hasoid = false;

!         if (select_into)
          {
!             /* output hasoid state must match the expected rel option */
!             junk_filter_hasoid =
!                 interpretOidsOption(plannedstmt->intoClause->options);
!             if (tupType->tdhasoid != junk_filter_hasoid)
!                 junk_filter_needed = true;
!         }

!         if (!junk_filter_needed)
!         {
!             ListCell   *tlist;
!
!             foreach(tlist, plan->targetlist)
              {
!                 TargetEntry *tle = (TargetEntry *) lfirst(tlist);
!
!                 if (tle->resjunk)
!                 {
!                     junk_filter_needed = true;
!                     break;
!                 }
              }
          }

***************
*** 828,834 ****
              JunkFilter *j;

              j = ExecInitJunkFilter(planstate->plan->targetlist,
!                                    tupType->tdhasoid,
                                     ExecInitExtraTupleSlot(estate));
              estate->es_junkFilter = j;

--- 835,841 ----
              JunkFilter *j;

              j = ExecInitJunkFilter(planstate->plan->targetlist,
!                                    junk_filter_hasoid,
                                     ExecInitExtraTupleSlot(estate));
              estate->es_junkFilter = j;

***************
*** 847,853 ****
       *
       * If EXPLAIN, skip creating the "into" relation.
       */
!     if (estate->es_select_into && !(eflags & EXEC_FLAG_EXPLAIN_ONLY))
          OpenIntoRel(queryDesc);
  }

--- 854,860 ----
       *
       * If EXPLAIN, skip creating the "into" relation.
       */
!     if (select_into && !(eflags & EXEC_FLAG_EXPLAIN_ONLY))
          OpenIntoRel(queryDesc);
  }

***************
*** 1010,1072 ****
      return rInfo;
  }

- /*
-  *        ExecContextForcesOids
-  *
-  * This is pretty grotty: when doing INSERT, UPDATE, or SELECT INTO,
-  * we need to ensure that result tuples have space for an OID iff they are
-  * going to be stored into a relation that has OIDs.  In other contexts
-  * we are free to choose whether to leave space for OIDs in result tuples
-  * (we generally don't want to, but we do if a physical-tlist optimization
-  * is possible).  This routine checks the plan context and returns TRUE if the
-  * choice is forced, FALSE if the choice is not forced.  In the TRUE case,
-  * *hasoids is set to the required value.
-  *
-  * One reason this is ugly is that all plan nodes in the plan tree will emit
-  * tuples with space for an OID, though we really only need the topmost node
-  * to do so.  However, node types like Sort don't project new tuples but just
-  * return their inputs, and in those cases the requirement propagates down
-  * to the input node.  Eventually we might make this code smart enough to
-  * recognize how far down the requirement really goes, but for now we just
-  * make all plan nodes do the same thing if the top level forces the choice.
-  *
-  * We assume that if we are generating tuples for INSERT or UPDATE,
-  * estate->es_result_relation_info is already set up to describe the target
-  * relation.  Note that in an UPDATE that spans an inheritance tree, some of
-  * the target relations may have OIDs and some not.  We have to make the
-  * decisions on a per-relation basis as we initialize each of the subplans of
-  * the ModifyTable node, so ModifyTable has to set es_result_relation_info
-  * while initializing each subplan.
-  *
-  * SELECT INTO is even uglier, because we don't have the INTO relation's
-  * descriptor available when this code runs; we have to look aside at a
-  * flag set by InitPlan().
-  */
- bool
- ExecContextForcesOids(PlanState *planstate, bool *hasoids)
- {
-     ResultRelInfo *ri = planstate->state->es_result_relation_info;
-
-     if (ri != NULL)
-     {
-         Relation    rel = ri->ri_RelationDesc;
-
-         if (rel != NULL)
-         {
-             *hasoids = rel->rd_rel->relhasoids;
-             return true;
-         }
-     }
-
-     if (planstate->state->es_select_into)
-     {
-         *hasoids = planstate->state->es_into_oids;
-         return true;
-     }
-
-     return false;
- }
-
  /* ----------------------------------------------------------------
   *        ExecEndPlan
   *
--- 1017,1022 ----
***************
*** 1887,1898 ****
      estate->es_output_cid = parentestate->es_output_cid;
      estate->es_result_relations = parentestate->es_result_relations;
      estate->es_num_result_relations = parentestate->es_num_result_relations;
-     estate->es_result_relation_info = parentestate->es_result_relation_info;
      /* es_trig_target_relations must NOT be copied */
      estate->es_rowMarks = parentestate->es_rowMarks;
      estate->es_instrument = parentestate->es_instrument;
-     estate->es_select_into = parentestate->es_select_into;
-     estate->es_into_oids = parentestate->es_into_oids;

      /*
       * The external param list is simply shared from parent.  The internal
--- 1837,1845 ----
Index: src/backend/executor/execScan.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execScan.c,v
retrieving revision 1.47
diff -c -r1.47 execScan.c
*** src/backend/executor/execScan.c    26 Oct 2009 02:26:29 -0000    1.47
--- src/backend/executor/execScan.c    27 Nov 2009 17:21:56 -0000
***************
*** 23,29 ****
  #include "utils/memutils.h"


! static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc);


  /*
--- 23,30 ----
  #include "utils/memutils.h"


! static bool tlist_matches_tupdesc(List *tlist, Index varno, TupleDesc tupdesc,
!                                   bool resulthasoid);


  /*
***************
*** 235,263 ****
   * the scan node, because the planner will preferentially generate a matching
   * tlist.
   *
!  * ExecAssignScanType must have been called already.
   */
  void
  ExecAssignScanProjectionInfo(ScanState *node)
  {
      Scan       *scan = (Scan *) node->ps.plan;

!     if (tlist_matches_tupdesc(&node->ps,
!                               scan->plan.targetlist,
                                scan->scanrelid,
!                               node->ss_ScanTupleSlot->tts_tupleDescriptor))
          node->ps.ps_ProjInfo = NULL;
      else
!         ExecAssignProjectionInfo(&node->ps,
!                                  node->ss_ScanTupleSlot->tts_tupleDescriptor);
  }

  static bool
! tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc)
  {
      int            numattrs = tupdesc->natts;
      int            attrno;
-     bool        hasoid;
      ListCell   *tlist_item = list_head(tlist);

      /* Check the tlist attributes */
--- 236,265 ----
   * the scan node, because the planner will preferentially generate a matching
   * tlist.
   *
!  * ExecAssignScanType and ExecAssignResultType must have been called already.
   */
  void
  ExecAssignScanProjectionInfo(ScanState *node)
  {
      Scan       *scan = (Scan *) node->ps.plan;
+     TupleDesc    scandesc = node->ss_ScanTupleSlot->tts_tupleDescriptor;
+     TupleDesc    resultdesc = node->ps.ps_ResultTupleSlot->tts_tupleDescriptor;

!     if (tlist_matches_tupdesc(scan->plan.targetlist,
                                scan->scanrelid,
!                               scandesc,
!                               resultdesc->tdhasoid))
          node->ps.ps_ProjInfo = NULL;
      else
!         ExecAssignProjectionInfo(&node->ps, scandesc);
  }

  static bool
! tlist_matches_tupdesc(List *tlist, Index varno, TupleDesc tupdesc,
!                       bool resulthasoid)
  {
      int            numattrs = tupdesc->natts;
      int            attrno;
      ListCell   *tlist_item = list_head(tlist);

      /* Check the tlist attributes */
***************
*** 301,311 ****
          return false;            /* tlist too long */

      /*
!      * If the plan context requires a particular hasoid setting, then that has
!      * to match, too.
       */
!     if (ExecContextForcesOids(ps, &hasoid) &&
!         hasoid != tupdesc->tdhasoid)
          return false;

      return true;
--- 303,311 ----
          return false;            /* tlist too long */

      /*
!      * The hasOids state has to match the output rowtype, too.
       */
!     if (tupdesc->tdhasoid != resulthasoid)
          return false;

      return true;
Index: src/backend/executor/execUtils.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execUtils.c,v
retrieving revision 1.166
diff -c -r1.166 execUtils.c
*** src/backend/executor/execUtils.c    20 Nov 2009 20:38:10 -0000    1.166
--- src/backend/executor/execUtils.c    27 Nov 2009 17:21:56 -0000
***************
*** 113,119 ****

      estate->es_result_relations = NULL;
      estate->es_num_result_relations = 0;
-     estate->es_result_relation_info = NULL;

      estate->es_trig_target_relations = NIL;
      estate->es_trig_tuple_slot = NULL;
--- 113,118 ----
***************
*** 132,139 ****
      estate->es_lastoid = InvalidOid;

      estate->es_instrument = false;
-     estate->es_select_into = false;
-     estate->es_into_oids = false;

      estate->es_exprcontexts = NIL;

--- 131,136 ----
***************
*** 440,454 ****
      bool        hasoid;
      TupleDesc    tupDesc;

!     if (ExecContextForcesOids(planstate, &hasoid))
!     {
!         /* context forces OID choice; hasoid is now set correctly */
!     }
!     else
!     {
!         /* given free choice, don't leave space for OIDs in result tuples */
!         hasoid = false;
!     }

      /*
       * ExecTypeFromTL needs the parse-time representation of the tlist, not a
--- 437,450 ----
      bool        hasoid;
      TupleDesc    tupDesc;

!     /*
!      * If the plan is a scan node on a relation that has OIDs, it would be
!      * desirable to allow OIDs in the output rowtype, so that a physical
!      * tlist optimization is possible.  It would require some refactoring
!      * to make that knowledge available here, however, so for the moment
!      * just set hasoid = false always.
!      */
!     hasoid = false;

      /*
       * ExecTypeFromTL needs the parse-time representation of the tlist, not a
***************
*** 969,981 ****
   * ----------------------------------------------------------------
   */
  List *
! ExecInsertIndexTuples(TupleTableSlot *slot,
                        ItemPointer tupleid,
                        EState *estate,
                        bool is_vacuum_full)
  {
      List       *result = NIL;
-     ResultRelInfo *resultRelInfo;
      int            i;
      int            numIndices;
      RelationPtr relationDescs;
--- 965,977 ----
   * ----------------------------------------------------------------
   */
  List *
! ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
!                       TupleTableSlot *slot,
                        ItemPointer tupleid,
                        EState *estate,
                        bool is_vacuum_full)
  {
      List       *result = NIL;
      int            i;
      int            numIndices;
      RelationPtr relationDescs;
***************
*** 988,994 ****
      /*
       * Get information from the result relation info structure.
       */
-     resultRelInfo = estate->es_result_relation_info;
      numIndices = resultRelInfo->ri_NumIndices;
      relationDescs = resultRelInfo->ri_IndexRelationDescs;
      indexInfoArray = resultRelInfo->ri_IndexRelationInfo;
--- 984,989 ----
Index: src/backend/executor/nodeModifyTable.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/nodeModifyTable.c,v
retrieving revision 1.3
diff -c -r1.3 nodeModifyTable.c
*** src/backend/executor/nodeModifyTable.c    20 Nov 2009 20:38:10 -0000    1.3
--- src/backend/executor/nodeModifyTable.c    27 Nov 2009 17:21:56 -0000
***************
*** 158,169 ****
   * ----------------------------------------------------------------
   */
  static TupleTableSlot *
! ExecInsert(TupleTableSlot *slot,
             TupleTableSlot *planSlot,
             EState *estate)
  {
      HeapTuple    tuple;
-     ResultRelInfo *resultRelInfo;
      Relation    resultRelationDesc;
      Oid            newId;
      List       *recheckIndexes = NIL;
--- 158,169 ----
   * ----------------------------------------------------------------
   */
  static TupleTableSlot *
! ExecInsert(ResultRelInfo *resultRelInfo,
!            TupleTableSlot *slot,
             TupleTableSlot *planSlot,
             EState *estate)
  {
      HeapTuple    tuple;
      Relation    resultRelationDesc;
      Oid            newId;
      List       *recheckIndexes = NIL;
***************
*** 177,183 ****
      /*
       * get information on the (current) result relation
       */
-     resultRelInfo = estate->es_result_relation_info;
      resultRelationDesc = resultRelInfo->ri_RelationDesc;

      /*
--- 177,182 ----
***************
*** 248,254 ****
       * insert index entries for tuple
       */
      if (resultRelInfo->ri_NumIndices > 0)
!         recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
                                                 estate, false);

      /* AFTER ROW INSERT Triggers */
--- 247,254 ----
       * insert index entries for tuple
       */
      if (resultRelInfo->ri_NumIndices > 0)
!         recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
!                                                slot, &(tuple->t_self),
                                                 estate, false);

      /* AFTER ROW INSERT Triggers */
***************
*** 272,283 ****
   * ----------------------------------------------------------------
   */
  static TupleTableSlot *
! ExecDelete(ItemPointer tupleid,
             TupleTableSlot *planSlot,
             EPQState *epqstate,
             EState *estate)
  {
-     ResultRelInfo *resultRelInfo;
      Relation    resultRelationDesc;
      HTSU_Result result;
      ItemPointerData update_ctid;
--- 272,283 ----
   * ----------------------------------------------------------------
   */
  static TupleTableSlot *
! ExecDelete(ResultRelInfo *resultRelInfo,
!            ItemPointer tupleid,
             TupleTableSlot *planSlot,
             EPQState *epqstate,
             EState *estate)
  {
      Relation    resultRelationDesc;
      HTSU_Result result;
      ItemPointerData update_ctid;
***************
*** 286,292 ****
      /*
       * get information on the (current) result relation
       */
-     resultRelInfo = estate->es_result_relation_info;
      resultRelationDesc = resultRelInfo->ri_RelationDesc;

      /* BEFORE ROW DELETE Triggers */
--- 286,291 ----
***************
*** 415,428 ****
   * ----------------------------------------------------------------
   */
  static TupleTableSlot *
! ExecUpdate(ItemPointer tupleid,
             TupleTableSlot *slot,
             TupleTableSlot *planSlot,
             EPQState *epqstate,
             EState *estate)
  {
      HeapTuple    tuple;
-     ResultRelInfo *resultRelInfo;
      Relation    resultRelationDesc;
      HTSU_Result result;
      ItemPointerData update_ctid;
--- 414,427 ----
   * ----------------------------------------------------------------
   */
  static TupleTableSlot *
! ExecUpdate(ResultRelInfo *resultRelInfo,
!            ItemPointer tupleid,
             TupleTableSlot *slot,
             TupleTableSlot *planSlot,
             EPQState *epqstate,
             EState *estate)
  {
      HeapTuple    tuple;
      Relation    resultRelationDesc;
      HTSU_Result result;
      ItemPointerData update_ctid;
***************
*** 444,450 ****
      /*
       * get information on the (current) result relation
       */
-     resultRelInfo = estate->es_result_relation_info;
      resultRelationDesc = resultRelInfo->ri_RelationDesc;

      /* BEFORE ROW UPDATE Triggers */
--- 443,448 ----
***************
*** 563,569 ****
       * If it's a HOT update, we mustn't insert new index entries.
       */
      if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
!         recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
                                                 estate, false);

      /* AFTER ROW UPDATE Triggers */
--- 561,568 ----
       * If it's a HOT update, we mustn't insert new index entries.
       */
      if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
!         recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
!                                                slot, &(tuple->t_self),
                                                 estate, false);

      /* AFTER ROW UPDATE Triggers */
***************
*** 645,650 ****
--- 644,650 ----
      EState *estate = node->ps.state;
      CmdType operation = node->operation;
      PlanState *subplanstate;
+     ResultRelInfo *resultRelInfo;
      JunkFilter *junkfilter;
      TupleTableSlot *slot;
      TupleTableSlot *planSlot;
***************
*** 660,676 ****
          node->fireBSTriggers = false;
      }

-     /*
-      * es_result_relation_info must point to the currently active result
-      * relation.  (Note we assume that ModifyTable nodes can't be nested.)
-      * We want it to be NULL whenever we're not within ModifyTable, though.
-      */
-     estate->es_result_relation_info =
-         estate->es_result_relations + node->mt_whichplan;
-
      /* Preload local variables */
      subplanstate = node->mt_plans[node->mt_whichplan];
!     junkfilter = estate->es_result_relation_info->ri_junkFilter;

      /*
       * Fetch rows from subplan(s), and execute the required table modification
--- 660,669 ----
          node->fireBSTriggers = false;
      }

      /* Preload local variables */
      subplanstate = node->mt_plans[node->mt_whichplan];
!     resultRelInfo = estate->es_result_relations + node->mt_whichplan;
!     junkfilter = resultRelInfo->ri_junkFilter;

      /*
       * Fetch rows from subplan(s), and execute the required table modification
***************
*** 686,694 ****
              node->mt_whichplan++;
              if (node->mt_whichplan < node->mt_nplans)
              {
-                 estate->es_result_relation_info++;
                  subplanstate = node->mt_plans[node->mt_whichplan];
!                 junkfilter = estate->es_result_relation_info->ri_junkFilter;
                  EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
                  continue;
              }
--- 679,687 ----
              node->mt_whichplan++;
              if (node->mt_whichplan < node->mt_nplans)
              {
                  subplanstate = node->mt_plans[node->mt_whichplan];
!                 resultRelInfo = estate->es_result_relations + node->mt_whichplan;
!                 junkfilter = resultRelInfo->ri_junkFilter;
                  EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
                  continue;
              }
***************
*** 730,743 ****
          switch (operation)
          {
              case CMD_INSERT:
!                 slot = ExecInsert(slot, planSlot, estate);
                  break;
              case CMD_UPDATE:
!                 slot = ExecUpdate(tupleid, slot, planSlot,
                                    &node->mt_epqstate, estate);
                  break;
              case CMD_DELETE:
!                 slot = ExecDelete(tupleid, planSlot,
                                    &node->mt_epqstate, estate);
                  break;
              default:
--- 723,739 ----
          switch (operation)
          {
              case CMD_INSERT:
!                 slot = ExecInsert(resultRelInfo,
!                                   slot, planSlot, estate);
                  break;
              case CMD_UPDATE:
!                 slot = ExecUpdate(resultRelInfo,
!                                   tupleid, slot, planSlot,
                                    &node->mt_epqstate, estate);
                  break;
              case CMD_DELETE:
!                 slot = ExecDelete(resultRelInfo,
!                                   tupleid, planSlot,
                                    &node->mt_epqstate, estate);
                  break;
              default:
***************
*** 750,764 ****
           * the work on next call.
           */
          if (slot)
-         {
-             estate->es_result_relation_info = NULL;
              return slot;
-         }
      }

-     /* Reset es_result_relation_info before exiting */
-     estate->es_result_relation_info = NULL;
-
      /*
       * We're done, but fire AFTER STATEMENT triggers before exiting.
       */
--- 746,754 ----
***************
*** 813,832 ****

      /*
       * call ExecInitNode on each of the plans to be executed and save the
!      * results into the array "mt_plans".  Note we *must* set
!      * estate->es_result_relation_info correctly while we initialize each
!      * sub-plan; ExecContextForcesOids depends on that!
       */
-     estate->es_result_relation_info = estate->es_result_relations;
      i = 0;
      foreach(l, node->plans)
      {
          subplan = (Plan *) lfirst(l);
          mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
-         estate->es_result_relation_info++;
          i++;
      }
-     estate->es_result_relation_info = NULL;

      /* select first subplan */
      mtstate->mt_whichplan = 0;
--- 803,817 ----

      /*
       * call ExecInitNode on each of the plans to be executed and save the
!      * results into the array "mt_plans".
       */
      i = 0;
      foreach(l, node->plans)
      {
          subplan = (Plan *) lfirst(l);
          mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
          i++;
      }

      /* select first subplan */
      mtstate->mt_whichplan = 0;
***************
*** 921,951 ****

      /*
       * Initialize the junk filter(s) if needed.  INSERT queries need a filter
!      * if there are any junk attrs in the tlist.  UPDATE and DELETE
!      * always need a filter, since there's always a junk 'ctid' attribute
!      * present --- no need to look first.
       *
       * If there are multiple result relations, each one needs its own junk
!      * filter.  Note multiple rels are only possible for UPDATE/DELETE, so we
!      * can't be fooled by some needing a filter and some not.
       *
       * This section of code is also a convenient place to verify that the
       * output of an INSERT or UPDATE matches the target table(s).
       */
      {
          bool        junk_filter_needed = false;

          switch (operation)
          {
              case CMD_INSERT:
!                 foreach(l, subplan->targetlist)
                  {
!                     TargetEntry *tle = (TargetEntry *) lfirst(l);
!
!                     if (tle->resjunk)
                      {
!                         junk_filter_needed = true;
!                         break;
                      }
                  }
                  break;
--- 906,958 ----

      /*
       * Initialize the junk filter(s) if needed.  INSERT queries need a filter
!      * if there are any junk attrs in the tlist, or if we have to add or
!      * remove an OID column.  UPDATE and DELETE always need a filter, since
!      * there's always a junk 'ctid' attribute present --- no need to look
!      * first.  (For DELETE, we won't actually use the filter to filter tuples,
!      * but we still need it for extracting the ctid attribute.)
       *
       * If there are multiple result relations, each one needs its own junk
!      * filter.
       *
       * This section of code is also a convenient place to verify that the
       * output of an INSERT or UPDATE matches the target table(s).
       */
+     resultRelInfo = estate->es_result_relations;
+     for (i = 0; i < nplans; i++)
      {
          bool        junk_filter_needed = false;
+         bool        junk_filter_hasoid = false;
+
+         subplan = mtstate->mt_plans[i]->plan;
+
+         if (operation == CMD_INSERT || operation == CMD_UPDATE)
+         {
+             TupleDesc    tupType;
+
+             ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
+                                 subplan->targetlist);
+             /* output hasoid state must match the rel */
+             junk_filter_hasoid = resultRelInfo->ri_RelationDesc->rd_att->tdhasoid;
+             tupType = ExecGetResultType(mtstate->mt_plans[i]);
+             if (tupType->tdhasoid != junk_filter_hasoid)
+                 junk_filter_needed = true;
+         }

          switch (operation)
          {
              case CMD_INSERT:
!                 if (!junk_filter_needed)
                  {
!                     foreach(l, subplan->targetlist)
                      {
!                         TargetEntry *tle = (TargetEntry *) lfirst(l);
!
!                         if (tle->resjunk)
!                         {
!                             junk_filter_needed = true;
!                             break;
!                         }
                      }
                  }
                  break;
***************
*** 960,997 ****

          if (junk_filter_needed)
          {
!             resultRelInfo = estate->es_result_relations;
!             for (i = 0; i < nplans; i++)
!             {
!                 JunkFilter *j;
!
!                 subplan = mtstate->mt_plans[i]->plan;
!                 if (operation == CMD_INSERT || operation == CMD_UPDATE)
!                     ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
!                                         subplan->targetlist);
!
!                 j = ExecInitJunkFilter(subplan->targetlist,
!                             resultRelInfo->ri_RelationDesc->rd_att->tdhasoid,
!                                        ExecInitExtraTupleSlot(estate));

!                 if (operation == CMD_UPDATE || operation == CMD_DELETE)
!                 {
!                     /* For UPDATE/DELETE, find the ctid junk attr now */
!                     j->jf_junkAttNo = ExecFindJunkAttribute(j, "ctid");
!                     if (!AttributeNumberIsValid(j->jf_junkAttNo))
!                         elog(ERROR, "could not find junk ctid column");
!                 }
!
!                 resultRelInfo->ri_junkFilter = j;
!                 resultRelInfo++;
              }
          }
!         else
!         {
!             if (operation == CMD_INSERT)
!                 ExecCheckPlanOutput(estate->es_result_relations->ri_RelationDesc,
!                                     subplan->targetlist);
!         }
      }

      /*
--- 967,989 ----

          if (junk_filter_needed)
          {
!             JunkFilter *j;

!             j = ExecInitJunkFilter(subplan->targetlist,
!                                    junk_filter_hasoid,
!                                    ExecInitExtraTupleSlot(estate));
!             if (operation == CMD_UPDATE || operation == CMD_DELETE)
!             {
!                 /* For UPDATE/DELETE, find the ctid junk attr now */
!                 j->jf_junkAttNo = ExecFindJunkAttribute(j, "ctid");
!                 if (!AttributeNumberIsValid(j->jf_junkAttNo))
!                     elog(ERROR, "could not find junk ctid column");
              }
+
+             resultRelInfo->ri_junkFilter = j;
          }
!
!         resultRelInfo++;
      }

      /*
Index: src/include/executor/executor.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/executor/executor.h,v
retrieving revision 1.163
diff -c -r1.163 executor.h
*** src/include/executor/executor.h    26 Oct 2009 02:26:41 -0000    1.163
--- src/include/executor/executor.h    27 Nov 2009 17:21:56 -0000
***************
*** 163,169 ****
                    CmdType operation,
                    bool doInstrument);
  extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
- extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
  extern void ExecConstraints(ResultRelInfo *resultRelInfo,
                  TupleTableSlot *slot, EState *estate);
  extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate,
--- 163,168 ----
***************
*** 319,325 ****

  extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
  extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
! extern List *ExecInsertIndexTuples(TupleTableSlot *slot, ItemPointer tupleid,
                        EState *estate, bool is_vacuum_full);

  extern void RegisterExprContextCallback(ExprContext *econtext,
--- 318,325 ----

  extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
  extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
! extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
!                       TupleTableSlot *slot, ItemPointer tupleid,
                        EState *estate, bool is_vacuum_full);

  extern void RegisterExprContextCallback(ExprContext *econtext,
Index: src/include/nodes/execnodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/execnodes.h,v
retrieving revision 1.212
diff -c -r1.212 execnodes.h
*** src/include/nodes/execnodes.h    20 Nov 2009 20:38:11 -0000    1.212
--- src/include/nodes/execnodes.h    27 Nov 2009 17:21:56 -0000
***************
*** 340,349 ****
      /* If query can insert/delete tuples, the command ID to mark them with */
      CommandId    es_output_cid;

!     /* Info about target table for insert/update/delete queries: */
      ResultRelInfo *es_result_relations; /* array of ResultRelInfos */
      int            es_num_result_relations;        /* length of array */
-     ResultRelInfo *es_result_relation_info;        /* currently active array elt */

      /* Stuff used for firing triggers: */
      List       *es_trig_target_relations;        /* trigger-only ResultRelInfos */
--- 340,348 ----
      /* If query can insert/delete tuples, the command ID to mark them with */
      CommandId    es_output_cid;

!     /* Info about target tables for insert/update/delete queries: */
      ResultRelInfo *es_result_relations; /* array of ResultRelInfos */
      int            es_num_result_relations;        /* length of array */

      /* Stuff used for firing triggers: */
      List       *es_trig_target_relations;        /* trigger-only ResultRelInfos */
***************
*** 365,372 ****
      Oid            es_lastoid;        /* last oid processed (by INSERT) */

      bool        es_instrument;    /* true requests runtime instrumentation */
-     bool        es_select_into; /* true if doing SELECT INTO */
-     bool        es_into_oids;    /* true to generate OIDs in SELECT INTO */

      List       *es_exprcontexts;    /* List of ExprContexts within EState */

--- 364,369 ----

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: KNNGiST for knn-search (WIP)
Next
From: Tom Lane
Date:
Subject: Re: Writeable CTE patch