Clean up inefficiency in ExecRelCheck, and cause it to do the right
authorTom Lane
Sun, 6 Aug 2000 04:26:40 +0000 (04:26 +0000)
committerTom Lane
Sun, 6 Aug 2000 04:26:40 +0000 (04:26 +0000)
thing when there are multiple result relations.  Formerly, during
something like 'UPDATE foo*', foo's constraints and *only* foo's
constraints would be applied to all foo's children.  Wrong-o ...

src/backend/commands/copy.c
src/backend/executor/execMain.c
src/include/executor/executor.h
src/include/nodes/execnodes.h

index 52100e92dd92db72b0bc6c480b8df7d768e6fb37..c71d08180988da47f1d024fcd750cd674db380af 100644 (file)
@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.119 2000/07/14 22:17:42 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/commands/copy.c,v 1.120 2000/08/06 04:26:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -827,9 +827,10 @@ CopyFrom(Relation rel, bool binary, bool oids, FILE *fp,
             * Check the constraints of the tuple
             * ----------------
             */
+           ExecStoreTuple(tuple, slot, InvalidBuffer, false);
 
            if (rel->rd_att->constr)
-               ExecConstraints("CopyFrom", rel, tuple, estate);
+               ExecConstraints("CopyFrom", rel, slot, estate);
 
            /* ----------------
             * OK, store the tuple and create index entries for it
@@ -838,10 +839,7 @@ CopyFrom(Relation rel, bool binary, bool oids, FILE *fp,
            heap_insert(rel, tuple);
 
            if (relationInfo->ri_NumIndices > 0)
-           {
-               ExecStoreTuple(tuple, slot, InvalidBuffer, false);
                ExecInsertIndexTuples(slot, &(tuple->t_self), estate, false);
-           }
 
            /* AFTER ROW INSERT Triggers */
            if (rel->trigdesc &&
index 3125bf175e9cb18e14e4739fedaa7350e0799683..e66107ce7d1e2150fbb0b86492f3627092f9e607 100644 (file)
@@ -27,7 +27,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.122 2000/07/12 02:37:00 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/executor/execMain.c,v 1.123 2000/08/06 04:26:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1296,10 +1296,6 @@ ExecAppend(TupleTableSlot *slot,
    resultRelationInfo = estate->es_result_relation_info;
    resultRelationDesc = resultRelationInfo->ri_RelationDesc;
 
-   /*
-    * have to add code to preform unique checking here. cim -12/1/89
-    */
-
    /* BEFORE ROW INSERT Triggers */
    if (resultRelationDesc->trigdesc &&
    resultRelationDesc->trigdesc->n_before_row[TRIGGER_EVENT_INSERT] > 0)
@@ -1320,18 +1316,20 @@ ExecAppend(TupleTableSlot *slot,
    }
 
    /*
-    * Check the constraints of a tuple
+    * Check the constraints of the tuple
     */
 
    if (resultRelationDesc->rd_att->constr)
-       ExecConstraints("ExecAppend", resultRelationDesc, tuple, estate);
+       ExecConstraints("ExecAppend", resultRelationDesc, slot, estate);
 
    /*
     * insert the tuple
     */
-   newId = heap_insert(resultRelationDesc,     /* relation desc */
-                       tuple); /* heap tuple */
+   newId = heap_insert(resultRelationDesc, tuple);
+
    IncrAppended();
+   (estate->es_processed)++;
+   estate->es_lastoid = newId;
 
    /*
     * process indices
@@ -1343,8 +1341,6 @@ ExecAppend(TupleTableSlot *slot,
    numIndices = resultRelationInfo->ri_NumIndices;
    if (numIndices > 0)
        ExecInsertIndexTuples(slot, &(tuple->t_self), estate, false);
-   (estate->es_processed)++;
-   estate->es_lastoid = newId;
 
    /* AFTER ROW INSERT Triggers */
    if (resultRelationDesc->trigdesc)
@@ -1466,7 +1462,7 @@ ExecReplace(TupleTableSlot *slot,
     */
    if (IsBootstrapProcessingMode())
    {
-       elog(DEBUG, "ExecReplace: replace can't run without transactions");
+       elog(NOTICE, "ExecReplace: replace can't run without transactions");
        return;
    }
 
@@ -1481,12 +1477,6 @@ ExecReplace(TupleTableSlot *slot,
    resultRelationInfo = estate->es_result_relation_info;
    resultRelationDesc = resultRelationInfo->ri_RelationDesc;
 
-   /*
-    * have to add code to preform unique checking here. in the event of
-    * unique tuples, this becomes a deletion of the original tuple
-    * affected by the replace. cim -12/1/89
-    */
-
    /* BEFORE ROW UPDATE Triggers */
    if (resultRelationDesc->trigdesc &&
    resultRelationDesc->trigdesc->n_before_row[TRIGGER_EVENT_UPDATE] > 0)
@@ -1507,11 +1497,11 @@ ExecReplace(TupleTableSlot *slot,
    }
 
    /*
-    * Check the constraints of a tuple
+    * Check the constraints of the tuple
     */
 
    if (resultRelationDesc->rd_att->constr)
-       ExecConstraints("ExecReplace", resultRelationDesc, tuple, estate);
+       ExecConstraints("ExecReplace", resultRelationDesc, slot, estate);
 
    /*
     * replace the heap tuple
@@ -1555,9 +1545,9 @@ lreplace:;
    /*
     * Note: instead of having to update the old index tuples associated
     * with the heap tuple, all we do is form and insert new index
-    * tuples..  This is because replaces are actually deletes and inserts
-    * and index tuple deletion is done automagically by the vaccuum
-    * deamon.. All we do is insert new index tuples.  -cim 9/27/89
+    * tuples.  This is because replaces are actually deletes and inserts
+    * and index tuple deletion is done automagically by the vacuum
+    * daemon. All we do is insert new index tuples.  -cim 9/27/89
     */
 
    /*
@@ -1579,109 +1569,55 @@ lreplace:;
        ExecARUpdateTriggers(estate, tupleid, tuple);
 }
 
-#ifdef NOT_USED
-static HeapTuple
-ExecAttrDefault(Relation rel, HeapTuple tuple)
-{
-   int         ndef = rel->rd_att->constr->num_defval;
-   AttrDefault *attrdef = rel->rd_att->constr->defval;
-   ExprContext *econtext = MakeExprContext(NULL, CurrentMemoryContext);
-   HeapTuple   newtuple;
-   Node       *expr;
-   bool        isnull;
-   bool        isdone;
-   Datum       val;
-   Datum      *replValue = NULL;
-   char       *replNull = NULL;
-   char       *repl = NULL;
-   int         i;
-
-   for (i = 0; i < ndef; i++)
-   {
-       if (!heap_attisnull(tuple, attrdef[i].adnum))
-           continue;
-       expr = (Node *) stringToNode(attrdef[i].adbin);
-
-       val = ExecEvalExprSwitchContext(expr, econtext, &isnull, &isdone);
-
-       if (isnull)
-           continue;
-
-       if (repl == NULL)
-       {
-           repl = (char *) palloc(rel->rd_att->natts * sizeof(char));
-           replNull = (char *) palloc(rel->rd_att->natts * sizeof(char));
-           replValue = (Datum *) palloc(rel->rd_att->natts * sizeof(Datum));
-           MemSet(repl, ' ', rel->rd_att->natts * sizeof(char));
-       }
-
-       repl[attrdef[i].adnum - 1] = 'r';
-       replNull[attrdef[i].adnum - 1] = ' ';
-       replValue[attrdef[i].adnum - 1] = val;
-
-   }
-
-   if (repl == NULL)
-   {
-       /* no changes needed */
-       newtuple = tuple;
-   }
-   else
-   {
-       newtuple = heap_modifytuple(tuple, rel, replValue, replNull, repl);
-
-       pfree(repl);
-       pfree(replNull);
-       pfree(replValue);
-       heap_freetuple(tuple);
-   }
-
-   FreeMemoryContext(econtext);
-
-   return newtuple;
-}
-
-#endif
-
 static char *
-ExecRelCheck(Relation rel, HeapTuple tuple, EState *estate)
+ExecRelCheck(Relation rel, TupleTableSlot *slot, EState *estate)
 {
    int         ncheck = rel->rd_att->constr->num_check;
    ConstrCheck *check = rel->rd_att->constr->check;
-   TupleTableSlot *slot = makeNode(TupleTableSlot);
-   RangeTblEntry *rte = makeNode(RangeTblEntry);
-   ExprContext *econtext = MakeExprContext(slot,
-                                           TransactionCommandContext);
-   List       *rtlist;
+   MemoryContext oldContext;
+   ExprContext *econtext;
    List       *qual;
    int         i;
 
-   slot->val = tuple;
-   slot->ttc_shouldFree = false;
-   slot->ttc_descIsNew = true;
-   slot->ttc_tupleDescriptor = rel->rd_att;
-   slot->ttc_buffer = InvalidBuffer;
-   slot->ttc_whichplan = -1;
-   rte->relname = RelationGetRelationName(rel);
-   rte->ref = makeNode(Attr);
-   rte->ref->relname = rte->relname;
-   rte->relid = RelationGetRelid(rel);
-   /* inh, inFromCl, inJoinSet, skipAcl won't be used, leave them zero */
-   rtlist = lcons(rte, NIL);
-   econtext->ecxt_range_table = rtlist; /* phony range table */
+   /*
+    * Make sure econtext, expressions, etc are placed in appropriate context.
+    */
+   oldContext = MemoryContextSwitchTo(TransactionCommandContext);
+
+   /*
+    * Create or reset the exprcontext for evaluating constraint expressions.
+    */
+   econtext = estate->es_constraint_exprcontext;
+   if (econtext == NULL)
+       estate->es_constraint_exprcontext = econtext =
+           MakeExprContext(NULL, TransactionCommandContext);
+   else
+       ResetExprContext(econtext);
 
    /*
-    * Save the de-stringized constraint expressions in command-level
-    * memory context.  XXX should build the above stuff there too,
-    * instead of doing it over for each tuple.
-    * XXX Is it sufficient to have just one es_result_relation_constraints
-    * in an inherited insert/update?
+    * If first time through for current result relation, set up econtext's
+    * range table to refer to result rel, and build expression nodetrees
+    * for rel's constraint expressions.  All this stuff is kept in
+    * TransactionCommandContext so it will still be here next time through.
+    *
+    * NOTE: if there are multiple result relations (eg, due to inheritance)
+    * then we leak storage for prior rel's expressions and rangetable.
+    * This should not be a big problem as long as result rels are processed
+    * sequentially within a command.
     */
-   if (estate->es_result_relation_constraints == NULL)
+   if (econtext->ecxt_range_table == NIL ||
+       getrelid(1, econtext->ecxt_range_table) != RelationGetRelid(rel))
    {
-       MemoryContext oldContext;
+       RangeTblEntry *rte = makeNode(RangeTblEntry);
+
+       rte->relname = RelationGetRelationName(rel);
+       rte->ref = makeNode(Attr);
+       rte->ref->relname = rte->relname;
+       rte->relid = RelationGetRelid(rel);
+       /* inh, inFromCl, inJoinSet, skipAcl won't be used, leave them zero */
 
-       oldContext = MemoryContextSwitchTo(TransactionCommandContext);
+       /* Set up single-entry range table */
+       econtext->ecxt_range_table = lcons(rte, NIL);
 
        estate->es_result_relation_constraints =
            (List **) palloc(ncheck * sizeof(List *));
@@ -1691,10 +1627,15 @@ ExecRelCheck(Relation rel, HeapTuple tuple, EState *estate)
            qual = (List *) stringToNode(check[i].ccbin);
            estate->es_result_relation_constraints[i] = qual;
        }
-
-       MemoryContextSwitchTo(oldContext);
    }
 
+   /* Done with building long-lived items */
+   MemoryContextSwitchTo(oldContext);
+
+   /* Arrange for econtext's scan tuple to be the tuple under test */
+   econtext->ecxt_scantuple = slot;
+
+   /* And evaluate the constraints */
    for (i = 0; i < ncheck; i++)
    {
        qual = estate->es_result_relation_constraints[i];
@@ -1708,25 +1649,25 @@ ExecRelCheck(Relation rel, HeapTuple tuple, EState *estate)
            return check[i].ccname;
    }
 
-   pfree(slot);
-   pfree(rte);
-   pfree(rtlist);
-
-   FreeExprContext(econtext);
-
+   /* NULL result means no error */
    return (char *) NULL;
 }
 
 void
-ExecConstraints(char *caller, Relation rel, HeapTuple tuple, EState *estate)
+ExecConstraints(char *caller, Relation rel,
+               TupleTableSlot *slot, EState *estate)
 {
-   Assert(rel->rd_att->constr);
+   HeapTuple   tuple = slot->val;
+   TupleConstr *constr = rel->rd_att->constr;
+
+   Assert(constr);
 
-   if (rel->rd_att->constr->has_not_null)
+   if (constr->has_not_null)
    {
+       int         natts = rel->rd_att->natts;
        int         attrChk;
 
-       for (attrChk = 1; attrChk <= rel->rd_att->natts; attrChk++)
+       for (attrChk = 1; attrChk <= natts; attrChk++)
        {
            if (rel->rd_att->attrs[attrChk-1]->attnotnull &&
                heap_attisnull(tuple, attrChk))
@@ -1735,11 +1676,11 @@ ExecConstraints(char *caller, Relation rel, HeapTuple tuple, EState *estate)
        }
    }
 
-   if (rel->rd_att->constr->num_check > 0)
+   if (constr->num_check > 0)
    {
        char       *failed;
 
-       if ((failed = ExecRelCheck(rel, tuple, estate)) != NULL)
+       if ((failed = ExecRelCheck(rel, slot, estate)) != NULL)
            elog(ERROR, "%s: rejected due to CHECK constraint %s",
                 caller, failed);
    }
index b835b29044007ded607451fbbc5ce50843d567be..7849e87a78a398486cefe1f32c93819b028058ed 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: executor.h,v 1.46 2000/07/12 02:37:30 tgl Exp $
+ * $Id: executor.h,v 1.47 2000/08/06 04:26:27 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -56,8 +56,8 @@ extern TupleDesc ExecutorStart(QueryDesc *queryDesc, EState *estate);
 extern TupleTableSlot *ExecutorRun(QueryDesc *queryDesc, EState *estate,
            int feature, Node *limoffset, Node *limcount);
 extern void ExecutorEnd(QueryDesc *queryDesc, EState *estate);
-extern void ExecConstraints(char *caller, Relation rel, HeapTuple tuple,
-               EState *estate);
+extern void ExecConstraints(char *caller, Relation rel,
+                           TupleTableSlot *slot, EState *estate);
 extern TupleTableSlot *EvalPlanQual(EState *estate, Index rti,
                                    ItemPointer tid);
 
index 62a887d754ba3c77a6a1d52e11fa88733fcd2112..a9ef6e78ae3d99a41b1f92452a53af2f2aae38e8 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2000, PostgreSQL, Inc
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: execnodes.h,v 1.44 2000/07/14 22:17:58 tgl Exp $
+ * $Id: execnodes.h,v 1.45 2000/08/06 04:26:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -215,7 +215,6 @@ typedef struct EState
    Snapshot    es_snapshot;
    List       *es_range_table;
    RelationInfo *es_result_relation_info;
-   List      **es_result_relation_constraints;
    Relation    es_into_relation_descriptor;
    ParamListInfo es_param_list_info;
    ParamExecData *es_param_exec_vals;  /* this is for subselects */
@@ -224,6 +223,9 @@ typedef struct EState
    uint32      es_processed;   /* # of tuples processed */
    Oid         es_lastoid;     /* last oid processed (by INSERT) */
    List       *es_rowMark;     /* not good place, but there is no other */
+   /* these two fields are storage space for ExecConstraints(): */
+   List      **es_result_relation_constraints;
+   ExprContext *es_constraint_exprcontext;
    /* Below is to re-evaluate plan qual in READ COMMITTED mode */
    struct Plan *es_origPlan;
    Pointer     es_evalPlanQual;