Fix some foreign key constraint related problems that occur when a row refers to itself.

FossilOrigin-Name: 9e503e2d0428c9e8df878c7c6594790232cca4e0

manifest

@@ -1,5 +1,5 @@
-C When\sALTER\sTABLE\sRENAME\sTO\sis\sused\sto\schange\sthe\sname\sof\sa\stable\sthat\sis\sthe\sparent\stable\sof\sa\sforeign\skey\sconstraint,\smodify\sthat\sforeign\skey\sconstraint\sto\suse\sthe\snew\stable\sname.
+C Fix\ssome\sforeign\skey\sconstraint\srelated\sproblems\sthat\soccur\swhen\sa\srow\srefers\sto\sitself.
-D 2009-09-26T17:51:48
+D 2009-09-28T11:54:22
 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0
 F Makefile.in 4ca3f1dd6efa2075bcb27f4dc43eef749877740d
 F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654
@@ -116,7 +116,7 @@ F src/date.c 657ff12ca0f1195b531561afacbb38b772d16638
 F src/delete.c 2a3d6fc0861b2f8dbd9feb7847b390267b281c60
 F src/expr.c c7f3f718bd5c392344ec8694a41c1824f30cf375
 F src/fault.c dc88c821842157460750d2d61a8a8b4197d047ff
-F src/fkey.c ee4e0d2465c2c092b2b6ab4be7e33ae33d6c6dac
+F src/fkey.c 5bbe13f5d9ffba1a21ec16fdc3ac1c0964a28597
 F src/func.c e536218d193b8d326aab91120bc4c6f28aa2b606
 F src/global.c 271952d199a8cc59d4ce840b3bbbfd2f30c8ba32
 F src/hash.c ebcaa921ffd9d86f7ea5ae16a0a29d1c871130a7
@@ -202,7 +202,7 @@ F src/test_thread.c b8a1ab7ca1a632f18e8a361880d5d65eeea08eac
 F src/test_wsd.c 3ae5101de6cbfda2720152ab659ea84079719241
 F src/tokenize.c af8a56e6a50c5042fc305bfa796275e9bf26ff2b
 F src/trigger.c d2f31617a1852f7365aca54c5c85743b9114d462
-F src/update.c 5df5c39dbe427d6cd5e87c78ae8c1b1d1cb5e304
+F src/update.c a7a5f60bf65a88557b434ac40f2ae6d2f7ad2d2d
 F src/utf.c 99cf927eabb104621ba889ac0dd075fc1657ad30
 F src/util.c 59d4e9456bf1fe581f415a783fa0cee6115c8f35
 F src/vacuum.c 869d08eaab64e2a4eaf4ef9ea34b851892b65a75
@@ -330,7 +330,7 @@ F test/expr.test 9f521ae22f00e074959f72ce2e55d46b9ed23f68
 F test/filectrl.test 8923a6dc7630f31c8a9dd3d3d740aa0922df7bf8
 F test/filefmt.test 84e3d0fe9f12d0d2ac852465c6f8450aea0d6f43
 F test/fkey1.test 01c7de578e11747e720c2d9aeef27f239853c4da
-F test/fkey2.test 13e99ef0cafe6947638f748aea7b1a7ee8aecb1a
+F test/fkey2.test b879ba958126da0bfdfcf5d4532840c0f5956c72
 F test/fkey3.test 2183cac9075f3aae4875106eb9255bb73618444e
 F test/fkey_malloc.test da912d000bb6ceb1cd11b655de1989762fa71ceb
 F test/format4.test 1f0cac8ff3895e9359ed87e41aaabee982a812eb
@@ -755,7 +755,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff
 F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224
 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e
 F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f
-P 519144ac437b5842e4213f0e81e05c709939c2ab
+P b4a10c39e726dc190e9597e382baddc034294114
-R 67b182d615cf1918bbb959f1c1561217
+R 8c38d646c8a9549ba486066fb27ea41b
 U dan
-Z 1c73bf12a1b7498f5704501d82682485
+Z 7018674b123841c492121f6d9b33b5de

manifest.uuid

@@ -1 +1 @@
-b4a10c39e726dc190e9597e382baddc034294114
+9e503e2d0428c9e8df878c7c6594790232cca4e0

src/fkey.c

@@ -327,6 +327,7 @@ static void fkLookupParent(
   if( pIdx==0 ){
     /* If pIdx is NULL, then the parent key is the INTEGER PRIMARY KEY
     ** column of the parent table (table pTab).  */
+    int iMustBeInt;               /* Address of MustBeInt instruction */
     int regTemp = sqlite3GetTempReg(pParse);
 
     /* Invoke MustBeInt to coerce the child key value to an integer (i.e. 
@@ -335,16 +336,22 @@ static void fkLookupParent(
     ** the value. Otherwise, the value inserted into the child key column
     ** will have INTEGER affinity applied to it, which may not be correct.  */
     sqlite3VdbeAddOp2(v, OP_SCopy, aiCol[0]+1+regData, regTemp);
-    sqlite3VdbeAddOp2(v, OP_MustBeInt, regTemp, 0);
+    iMustBeInt = sqlite3VdbeAddOp2(v, OP_MustBeInt, regTemp, 0);
+
+    /* If the parent table is the same as the child table, and we are about
+    ** to increment the constraint-counter (i.e. this is an INSERT operation), 
+    ** then check if the row being inserted matches itself. If so, do not
+    ** increment the constraint-counter.  */
+    if( pTab==pFKey->pFrom && nIncr==1 ){
+      sqlite3VdbeAddOp3(v, OP_Eq, regData, iOk, regTemp);
+    }
+
     sqlite3OpenTable(pParse, iCur, iDb, pTab, OP_OpenRead);
     sqlite3VdbeAddOp3(v, OP_NotExists, iCur, 0, regTemp);
     sqlite3VdbeAddOp2(v, OP_Goto, 0, iOk);
     sqlite3VdbeJumpHere(v, sqlite3VdbeCurrentAddr(v)-2);
-    sqlite3VdbeJumpHere(v, sqlite3VdbeCurrentAddr(v)-4);
+    sqlite3VdbeJumpHere(v, iMustBeInt);
     sqlite3ReleaseTempReg(pParse, regTemp);
-    assert( 
-      sqlite3VdbeGetOp(v, sqlite3VdbeCurrentAddr(v)-4)->opcode==OP_MustBeInt 
-    );
   }else{
     int nCol = pFKey->nCol;
     int regTemp = sqlite3GetTempRange(pParse, nCol);
@@ -356,9 +363,25 @@ static void fkLookupParent(
     for(i=0; i<nCol; i++){
       sqlite3VdbeAddOp2(v, OP_SCopy, aiCol[i]+1+regData, regTemp+i);
     }
+
+    /* If the parent table is the same as the child table, and we are about
+    ** to increment the constraint-counter (i.e. this is an INSERT operation), 
+    ** then check if the row being inserted matches itself. If so, do not
+    ** increment the constraint-counter.  */
+    if( pTab==pFKey->pFrom && nIncr==1 ){
+      int iJump = sqlite3VdbeCurrentAddr(v) + nCol + 1;
+      for(i=0; i<nCol; i++){
+        int iChild = aiCol[i]+1+regData;
+        int iParent = pIdx->aiColumn[i]+1+regData;
+        sqlite3VdbeAddOp3(v, OP_Ne, iChild, iJump, iParent);
+      }
+      sqlite3VdbeAddOp2(v, OP_Goto, 0, iOk);
+    }
+
     sqlite3VdbeAddOp3(v, OP_MakeRecord, regTemp, nCol, regRec);
     sqlite3VdbeChangeP4(v, -1, sqlite3IndexAffinityStr(v, pIdx), 0);
     sqlite3VdbeAddOp3(v, OP_Found, iCur, iOk, regRec);
+
     sqlite3ReleaseTempReg(pParse, regRec);
     sqlite3ReleaseTempRange(pParse, regTemp, nCol);
   }
@@ -413,6 +436,7 @@ static void fkLookupParent(
 static void fkScanChildren(
   Parse *pParse,                  /* Parse context */
   SrcList *pSrc,                  /* SrcList containing the table to scan */
+  Table *pTab,
   Index *pIdx,                    /* Foreign key index */
   FKey *pFKey,                    /* Foreign key relationship */
   int *aiCol,                     /* Map from pIdx cols to child table cols */
@@ -427,6 +451,8 @@ static void fkScanChildren(
   int iFkIfZero = 0;              /* Address of OP_FkIfZero */
   Vdbe *v = sqlite3GetVdbe(pParse);
 
+  assert( !pIdx || pIdx->pTable==pTab );
+
   if( nIncr<0 ){
     iFkIfZero = sqlite3VdbeAddOp2(v, OP_FkIfZero, pFKey->isDeferred, 0);
   }
@@ -469,6 +495,26 @@ static void fkScanChildren(
     pWhere = sqlite3ExprAnd(db, pWhere, pEq);
   }
 
+  /* If the child table is the same as the parent table, and this scan
+  ** is taking place as part of a DELETE operation (operation D.2), omit the
+  ** row being deleted from the scan by adding ($rowid != rowid) to the WHERE 
+  ** clause, where $rowid is the rowid of the row being deleted.  */
+  if( pTab==pFKey->pFrom && nIncr>0 ){
+    Expr *pEq;                    /* Expression (pLeft = pRight) */
+    Expr *pLeft;                  /* Value from parent table row */
+    Expr *pRight;                 /* Column ref to child table */
+    pLeft = sqlite3Expr(db, TK_REGISTER, 0);
+    pRight = sqlite3Expr(db, TK_COLUMN, 0);
+    if( pLeft && pRight ){
+      pLeft->iTable = regData;
+      pLeft->affinity = SQLITE_AFF_INTEGER;
+      pRight->iTable = pSrc->a[0].iCursor;
+      pRight->iColumn = -1;
+    }
+    pEq = sqlite3PExpr(pParse, TK_NE, pLeft, pRight, 0);
+    pWhere = sqlite3ExprAnd(db, pWhere, pEq);
+  }
+
   /* Resolve the references in the WHERE clause. */
   memset(&sNameContext, 0, sizeof(NameContext));
   sNameContext.pSrcList = pSrc;
@@ -535,6 +581,7 @@ static void fkTriggerDelete(sqlite3 *dbMem, Trigger *p){
     TriggerStep *pStep = p->step_list;
     sqlite3ExprDelete(dbMem, pStep->pWhere);
     sqlite3ExprListDelete(dbMem, pStep->pExprList);
+    sqlite3SelectDelete(dbMem, pStep->pSelect);
     sqlite3ExprDelete(dbMem, p->pWhen);
     sqlite3DbFree(dbMem, p);
   }
@@ -551,17 +598,16 @@ static void fkTriggerDelete(sqlite3 *dbMem, Trigger *p){
 ** of the row being deleted, from left to right. Parameter regNew is passed
 ** zero in this case.
 **
-** For an UPDATE operation, regOld is the first in an array of (pTab->nCol+1)
-** registers containing the old rowid and column values of the row being
-** updated, and regNew is the first in an array of the same size containing
-** the corresponding new values. Parameter pChanges is passed the list of
-** columns being updated by the statement.
-**
 ** For an INSERT operation, regOld is passed zero and regNew is passed the
 ** first register of an array of (pTab->nCol+1) registers containing the new
 ** row data.
 **
-** If an error occurs, an error message is left in the pParse structure.
+** For an UPDATE operation, this function is called twice. Once before
+** the original record is deleted from the table using the calling convention
+** described for DELETE. Then again after the original record is deleted
+** but before the new record is inserted using the INSERT convention. In
+** both cases parameter pChanges is passed the list of columns being 
+** updated by the statement.
 */
 void sqlite3FkCheck(
   Parse *pParse,                  /* Parse context */
@@ -576,7 +622,7 @@ void sqlite3FkCheck(
   int iDb;                        /* Index of database containing pTab */
   const char *zDb;                /* Name of database containing pTab */
 
-  assert( ( pChanges &&  regOld &&  regNew)           /* UPDATE operation */
+  assert( ( pChanges &&  (regOld==0)!=(regNew==0))    /* UPDATE operation */
        || (!pChanges && !regOld &&  regNew)           /* INSERT operation */
        || (!pChanges &&  regOld && !regNew)           /* DELETE operation */
   );
@@ -678,26 +724,8 @@ void sqlite3FkCheck(
       pSrc->a->pTab->nRef++;
       pSrc->a->iCursor = pParse->nTab++;
   
-      /* If this is an UPDATE, and none of the columns associated with this
-      ** FK have been modified, do not scan the child table. Unlike the 
-      ** compile-time test implemented above, this is not just an 
-      ** optimization. It is required so that immediate foreign keys do not 
-      ** throw exceptions when the user executes a statement like:
-      **
-      **     UPDATE refd_table SET refd_column = refd_column
-      */
-      if( pChanges ){
-        int i;
-        int iJump = sqlite3VdbeCurrentAddr(v) + pFKey->nCol + 1;
-        for(i=0; i<pFKey->nCol; i++){
-          int iOff = (pIdx ? pIdx->aiColumn[i] : -1) + 1;
-          sqlite3VdbeAddOp3(v, OP_Ne, regOld+iOff, iJump, regNew+iOff);
-        }
-        iGoto = sqlite3VdbeAddOp0(v, OP_Goto);
-      }
-  
       if( regNew!=0 ){
-        fkScanChildren(pParse, pSrc, pIdx, pFKey, aiCol, regNew, -1);
+        fkScanChildren(pParse, pSrc, pTab, pIdx, pFKey, aiCol, regNew, -1);
       }
       if( regOld!=0 ){
         /* If there is a RESTRICT action configured for the current operation
@@ -706,9 +734,7 @@ void sqlite3FkCheck(
         ** deferred trigger. That's what RESTRICT means. To defer checking
         ** the constraint, the FK should specify NO ACTION (represented
         ** using OE_None). NO ACTION is the default.  */
-        fkScanChildren(pParse, pSrc, pIdx, pFKey, aiCol, regOld, 
+        fkScanChildren(pParse, pSrc, pTab, pIdx, pFKey, aiCol, regOld, 1);
-            pFKey->aAction[pChanges!=0]!=OE_Restrict
-        );
       }
   
       if( pChanges ){
@@ -815,10 +841,7 @@ static Trigger *fkActionTrigger(
   action = pFKey->aAction[iAction];
   pTrigger = pFKey->apTrigger[iAction];
 
-  assert( OE_SetNull>OE_Restrict && OE_SetDflt>OE_Restrict );
+  if( action!=OE_None && !pTrigger ){
-  assert( OE_Cascade>OE_Restrict && OE_None<OE_Restrict );
-
-  if( action>OE_Restrict && !pTrigger ){
     u8 enableLookaside;           /* Copy of db->lookaside.bEnabled */
     char const *zFrom;            /* Name of child table */
     int nFrom;                    /* Length in bytes of zFrom */
@@ -827,6 +850,7 @@ static Trigger *fkActionTrigger(
     TriggerStep *pStep;           /* First (only) step of trigger program */
     Expr *pWhere = 0;             /* WHERE clause of trigger step */
     ExprList *pList = 0;          /* Changes list if ON UPDATE CASCADE */
+    Select *pSelect = 0;          /* If RESTRICT, "SELECT RAISE(...)" */
     int i;                        /* Iterator variable */
     Expr *pWhen = 0;              /* WHEN clause for the trigger */
 
@@ -878,7 +902,7 @@ static Trigger *fkActionTrigger(
         pWhen = sqlite3ExprAnd(db, pWhen, pEq);
       }
   
-      if( action!=OE_Cascade || pChanges ){
+      if( action!=OE_Restrict && (action!=OE_Cascade || pChanges) ){
         Expr *pNew;
         if( action==OE_Cascade ){
           pNew = sqlite3PExpr(pParse, TK_DOT, 
@@ -901,6 +925,28 @@ static Trigger *fkActionTrigger(
     }
     sqlite3DbFree(db, aiCol);
 
+    zFrom = pFKey->pFrom->zName;
+    nFrom = sqlite3Strlen30(zFrom);
+
+    if( action==OE_Restrict ){
+      Token tFrom;
+      Expr *pRaise; 
+
+      tFrom.z = zFrom;
+      tFrom.n = nFrom;
+      pRaise = sqlite3Expr(db, TK_RAISE, "foreign key constraint failed");
+      if( pRaise ){
+        pRaise->affinity = OE_Abort;
+      }
+      pSelect = sqlite3SelectNew(pParse, 
+          sqlite3ExprListAppend(pParse, 0, pRaise),
+          sqlite3SrcListAppend(db, 0, &tFrom, 0),
+          pWhere,
+          0, 0, 0, 0, 0, 0
+      );
+      pWhere = 0;
+    }
+
     /* In the current implementation, pTab->dbMem==0 for all tables except
     ** for temporary tables used to describe subqueries.  And temporary
     ** tables do not have foreign key constraints.  Hence, pTab->dbMem
@@ -909,8 +955,6 @@ static Trigger *fkActionTrigger(
     enableLookaside = db->lookaside.bEnabled;
     db->lookaside.bEnabled = 0;
 
-    zFrom = pFKey->pFrom->zName;
-    nFrom = sqlite3Strlen30(zFrom);
     pTrigger = (Trigger *)sqlite3DbMallocZero(db, 
         sizeof(Trigger) +         /* struct Trigger */
         sizeof(TriggerStep) +     /* Single step in trigger program */
@@ -924,6 +968,7 @@ static Trigger *fkActionTrigger(
   
       pStep->pWhere = sqlite3ExprDup(db, pWhere, EXPRDUP_REDUCE);
       pStep->pExprList = sqlite3ExprListDup(db, pList, EXPRDUP_REDUCE);
+      pStep->pSelect = sqlite3SelectDup(db, pSelect, EXPRDUP_REDUCE);
       if( pWhen ){
         pWhen = sqlite3PExpr(pParse, TK_NOT, pWhen, 0, 0);
         pTrigger->pWhen = sqlite3ExprDup(db, pWhen, EXPRDUP_REDUCE);
@@ -936,12 +981,24 @@ static Trigger *fkActionTrigger(
     sqlite3ExprDelete(db, pWhere);
     sqlite3ExprDelete(db, pWhen);
     sqlite3ExprListDelete(db, pList);
+    sqlite3SelectDelete(db, pSelect);
     if( db->mallocFailed==1 ){
       fkTriggerDelete(db, pTrigger);
       return 0;
     }
 
-    pStep->op = (action!=OE_Cascade || pChanges) ? TK_UPDATE : TK_DELETE;
+    switch( action ){
+      case OE_Restrict:
+        pStep->op = TK_SELECT; 
+        break;
+      case OE_Cascade: 
+        if( !pChanges ){ 
+          pStep->op = TK_DELETE; 
+          break; 
+        }
+      default:
+        pStep->op = TK_UPDATE;
+    }
     pStep->pTrig = pTrigger;
     pTrigger->pSchema = pTab->pSchema;
     pTrigger->pTabSchema = pTab->pSchema;

src/update.c

@@ -445,18 +445,20 @@ void sqlite3Update(
         aRegIdx, (chngRowid?regOldRowid:0), 1, onError, addr, 0);
 
     /* Do FK constraint checks. */
-    sqlite3FkCheck(pParse, pTab, pChanges, regOldRowid, regNewRowid);
+    sqlite3FkCheck(pParse, pTab, pChanges, regOldRowid, 0);
 
     /* Delete the index entries associated with the current record.  */
     j1 = sqlite3VdbeAddOp3(v, OP_NotExists, iCur, 0, regOldRowid);
     sqlite3GenerateRowIndexDelete(pParse, pTab, iCur, aRegIdx);
   
     /* If changing the record number, delete the old record.  */
-    if( chngRowid ){
+    if( hasFK || chngRowid ){
       sqlite3VdbeAddOp2(v, OP_Delete, iCur, 0);
     }
     sqlite3VdbeJumpHere(v, j1);
 
+    sqlite3FkCheck(pParse, pTab, pChanges, 0, regNewRowid);
+  
     /* Insert the new index entries and the new record. */
     sqlite3CompleteInsertion(pParse, pTab, iCur, regNewRowid, aRegIdx, 1, 0, 0);
 

test/fkey2.test

@@ -65,6 +65,9 @@ ifcapable {!foreignkey||!trigger} {
 #             table or deleting from a child table does not cause SQLite
 #             to check if this has repaired an outstanding violation.
 #
+# fkey2-16.*: Test that rows that refer to themselves may be inserted, 
+#             updated and deleted.
+#
 # fkey2-genfkey.*: Tests that were used with the shell tool .genfkey
 #            command. Recycled to test the built-in implementation.
 #
@@ -82,6 +85,18 @@ proc drop_all_tables {{db db}} {
 
 execsql { PRAGMA foreign_keys = on }
 
+if 0 {
+execsql {
+  CREATE TABLE t1(a PRIMARY KEY, b REFERENCES t1);
+  INSERT INTO t1 VALUES('aaa', 'aaa');
+}
+puts XXX
+explain { UPDATE t1 SET a = 'bbb' }
+execsql { PRAGMA vdbe_trace = 1 }
+execsql { UPDATE t1 SET a = 'bbb' }
+exit
+}
+
 set FkeySimpleSchema {
   PRAGMA foreign_keys = on;
   CREATE TABLE t1(a PRIMARY KEY, b);
@@ -915,6 +930,48 @@ do_test fkey2-15.1.7 {
   }
 } {2}
 
+#-------------------------------------------------------------------------
+# This next block of tests, fkey2-16.*, test that rows that refer to
+# themselves may be inserted and deleted.
+#
+foreach {tn zSchema} {
+  1 { CREATE TABLE self(a INTEGER PRIMARY KEY, b REFERENCES self(a)) }
+  2 { CREATE TABLE self(a PRIMARY KEY, b REFERENCES self(a)) }
+  3 { CREATE TABLE self(a UNIQUE, b INTEGER PRIMARY KEY REFERENCES self(a)) }
+} {
+  drop_all_tables
+  do_test fkey2-16.1.$tn.1 {
+    execsql $zSchema
+    execsql { INSERT INTO self VALUES(13, 13) }
+  } {}
+  do_test fkey2-16.1.$tn.2 {
+    execsql { UPDATE self SET a = 14, b = 14 }
+  } {}
+
+  do_test fkey2-16.1.$tn.3 {
+    catchsql { UPDATE self SET b = 15 }
+  } {1 {foreign key constraint failed}}
+
+  do_test fkey2-16.1.$tn.4 {
+    catchsql { UPDATE self SET a = 15 }
+  } {1 {foreign key constraint failed}}
+
+  do_test fkey2-16.1.$tn.5 {
+    catchsql { UPDATE self SET a = 15, b = 16 }
+  } {1 {foreign key constraint failed}}
+
+  do_test fkey2-16.1.$tn.6 {
+    catchsql { UPDATE self SET a = 17, b = 17 }
+  } {0 {}}
+
+  do_test fkey2-16.1.$tn.7 {
+    execsql { DELETE FROM self }
+  } {}
+  do_test fkey2-16.1.$tn.8 {
+    catchsql { INSERT INTO self VALUES(20, 21) }
+  } {1 {foreign key constraint failed}}
+}
+
 
 #-------------------------------------------------------------------------
 # The following block of tests, those prefixed with "fkey2-genfkey.", are