From: dan <dan@noemail.net>
Date: Tue, 28 Jul 2015 16:46:49 +0000 (+0000)
Subject: Add some test cases and fix some small problems with BEGIN UNLOCKED transactions.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=37d36205f3e7061e5d17e1ed6c291bbcc0875bae;p=thirdparty%2Fsqlite.git

Add some test cases and fix some small problems with BEGIN UNLOCKED transactions.

FossilOrigin-Name: 6da0e962ad2aa5e52c1f1b5c3dbf77a2cb16ac2d
---

diff --git a/manifest b/manifest
index 62addb80e6..c4c044ae5c 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Add\san\sexperimental\s"BEGIN\sUNLOCKED"\scommand.
-D 2015-07-27T19:31:45.102
+C Add\ssome\stest\scases\sand\sfix\ssome\ssmall\sproblems\swith\sBEGIN\sUNLOCKED\stransactions.
+D 2015-07-28T16:46:49.291
 F Makefile.arm-wince-mingw32ce-gcc d6df77f1f48d690bd73162294bbba7f59507c72f
 F Makefile.in 4de3ef40c8b3b75c0c55ff4242a43c8ce1ad90ee
 F Makefile.linux-gcc 91d710bdc4998cb015f39edf3cb314ec4f4d7e23
@@ -402,7 +402,7 @@ F src/vdbesort.c f5009e7a35e3065635d8918b9a31f498a499976b
 F src/vdbetrace.c 8befe829faff6d9e6f6e4dee5a7d3f85cc85f1a0
 F src/vtab.c 082b35a25a26e3d36f365ca8cd73c1922532f05e
 F src/vxworks.h c18586c8edc1bddbc15c004fa16aeb1e1342b4fb
-F src/wal.c d588e26ed0d912584551dfe65d644d8c27cc2d1f
+F src/wal.c a2e35f042103519ce22050acaf167960446963c7
 F src/wal.h 5188cd85398dc612ae5ed2ed02d193377218be56
 F src/walker.c c253b95b4ee44b21c406e2a1052636c31ea27804
 F src/where.c 909eba3b6db984eb2adfbca9de2c237ee7056adb
@@ -1215,7 +1215,7 @@ F test/types3.test 99e009491a54f4dc02c06bdbc0c5eea56ae3e25a
 F test/unique.test 93f8b2ef5ea51b9495f8d6493429b1fd0f465264
 F test/unique2.test 41e7f83c6827605991160a31380148a9fc5f1339
 F test/unixexcl.test cd6c765f75e50e8e2c2ba763149e5d340ea19825
-F test/unlocked.test edd4ed073ab16c0f8c9f5124e039100657b422c9
+F test/unlocked.test 854f3f428bb2e0ae91cb2400596a8f1ab7eaa409
 F test/unordered.test ca7adce0419e4ca0c50f039885e76ed2c531eda8
 F test/update.test 6c68446b8a0a33d522a7c72b320934596a2d7d32
 F test/uri.test 23662b7b61958b0f0e47082de7d06341ccf85d5b
@@ -1367,10 +1367,7 @@ F tool/vdbe_profile.tcl 67746953071a9f8f2f668b73fe899074e2c6d8c1
 F tool/warnings-clang.sh f6aa929dc20ef1f856af04a730772f59283631d4
 F tool/warnings.sh 48bd54594752d5be3337f12c72f28d2080cb630b
 F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f
-P b8e92227a469de677a66da62e4361f099c0b79d0
-R 889966cd4fab23d4f4355c880be4edd8
-T *branch * experimental-begin-unlocked
-T *sym-experimental-begin-unlocked *
-T -sym-trunk *
+P 8079421604dbd40d03471dad6d12115119b554c2
+R 52e6230fe0fe2d8cb4434e54d864e964
 U dan
-Z 55f5e7a6a42bdc95bbb980b319cd8658
+Z 849da1cda4069d7075ecc7190a9058c5
diff --git a/manifest.uuid b/manifest.uuid
index 25c9006198..23abb2a199 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-8079421604dbd40d03471dad6d12115119b554c2
\ No newline at end of file
+6da0e962ad2aa5e52c1f1b5c3dbf77a2cb16ac2d
\ No newline at end of file
diff --git a/src/wal.c b/src/wal.c
index a2200e7005..be018ebc99 100644
--- a/src/wal.c
+++ b/src/wal.c
@@ -2508,6 +2508,34 @@ Pgno sqlite3WalDbsize(Wal *pWal){
   return 0;
 }
 
+/*
+** Take the WRITER lock on the WAL file. Return SQLITE_OK if successful,
+** or an SQLite error code otherwise. This routine does not invoke any
+** busy-handler callbacks, that is done at a higher level.
+*/
+static int walWriteLock(Wal *pWal){
+  int rc;
+
+  /* Cannot start a write transaction without first holding a read lock */
+  assert( pWal->readLock>=0 );
+  assert( pWal->writeLock==0 );
+
+  /* If this is a read-only connection, obtaining a write-lock is not
+  ** possible. In this case return SQLITE_READONLY. Otherwise, attempt
+  ** to grab the WRITER lock. Set Wal.writeLock to true and return
+  ** SQLITE_OK if successful, or leave Wal.writeLock clear and return 
+  ** an SQLite error code (possibly SQLITE_BUSY) otherwise. */
+  if( pWal->readOnly ){
+    rc = SQLITE_READONLY;
+  }else{
+    rc = walLockExclusive(pWal, WAL_WRITE_LOCK, 1, 0);
+    if( rc==SQLITE_OK ){
+      pWal->writeLock = 1;
+    }
+  }
+
+  return rc;
+}
 
 /* 
 ** This function starts a write transaction on the WAL.
@@ -2523,35 +2551,18 @@ Pgno sqlite3WalDbsize(Wal *pWal){
 ** There can only be a single writer active at a time.
 */
 int sqlite3WalBeginWriteTransaction(Wal *pWal){
-  int rc;
-
-  /* Cannot start a write transaction without first holding a read
-  ** transaction. */
-  assert( pWal->readLock>=0 );
-
-  if( pWal->readOnly ){
-    return SQLITE_READONLY;
-  }
-
-  /* Only one writer allowed at a time.  Get the write lock.  Return
-  ** SQLITE_BUSY if unable.
-  */
-  rc = walLockExclusive(pWal, WAL_WRITE_LOCK, 1, 0);
-  if( rc ){
-    return rc;
-  }
-  pWal->writeLock = 1;
-
-  /* If another connection has written to the database file since the
-  ** time the read transaction on this connection was started, then
-  ** the write is disallowed.
-  */
-  if( memcmp(&pWal->hdr, (void *)walIndexHdr(pWal), sizeof(WalIndexHdr))!=0 ){
-    walUnlockExclusive(pWal, WAL_WRITE_LOCK, 1);
-    pWal->writeLock = 0;
-    rc = SQLITE_BUSY_SNAPSHOT;
+  int rc = walWriteLock(pWal);
+  if( rc==SQLITE_OK ){
+    /* If another connection has written to the database file since the
+    ** time the read transaction on this connection was started, then
+    ** the write is disallowed. Release the WRITER lock and return
+    ** SQLITE_BUSY_SNAPSHOT in this case.  */
+    if( memcmp(&pWal->hdr, (void *)walIndexHdr(pWal), sizeof(WalIndexHdr))!=0 ){
+      walUnlockExclusive(pWal, WAL_WRITE_LOCK, 1);
+      pWal->writeLock = 0;
+      rc = SQLITE_BUSY_SNAPSHOT;
+    }
   }
-
   return rc;
 }
 
@@ -2563,25 +2574,7 @@ int sqlite3WalBeginWriteTransaction(Wal *pWal){
 ** the wal file.
 */
 int sqlite3WalLockForCommit(Wal *pWal, PgHdr *pList, PgHdr *pPage1){
-  volatile WalIndexHdr *pHead;    /* Head of the wal file */
-  int rc;
-
-  /* Cannot start a write transaction without first holding a read
-  ** transaction. */
-  assert( pWal->readLock>=0 );
-
-  if( pWal->readOnly ){
-    return SQLITE_READONLY;
-  }
-
-  /* Only one writer allowed at a time.  Get the write lock.  Return
-  ** SQLITE_BUSY if unable.
-  */
-  rc = walLockExclusive(pWal, WAL_WRITE_LOCK, 1, 0);
-  if( rc ){
-    return rc;
-  }
-  pWal->writeLock = 1;
+  int rc = walWriteLock(pWal);
 
   /* If the database has been modified since this transaction was started,
   ** check if it is still possible to commit. The transaction can be 
@@ -2593,41 +2586,53 @@ int sqlite3WalLockForCommit(Wal *pWal, PgHdr *pList, PgHdr *pPage1){
   **   b) The database schema cookie has not been modified since the
   **      transaction was started.
   */
-  pHead = walIndexHdr(pWal);
-  if( memcmp(&pWal->hdr, (void*)pHead, sizeof(WalIndexHdr))!=0 ){
-    /* TODO: Is this safe? Because it holds the WRITER lock this thread
-    ** has exclusive access to the live header, but might it be corrupt? */
-    PgHdr *pPg;
-    u32 iLast = pHead->mxFrame;
-    for(pPg=pList; rc==SQLITE_OK && pPg; pPg=pPg->pDirty){
-      u32 iSlot = 0;
-      rc = walFindFrame(pWal, pPg->pgno, iLast, &iSlot);
-      if( iSlot>pWal->hdr.mxFrame ){
-        sqlite3_log(SQLITE_OK,
-            "cannot commit UNLOCKED transaction (conflict at page %d)",
-            (int)pPg->pgno
-        );
-        rc = SQLITE_BUSY_SNAPSHOT;
-      }
-    }
-
-    if( rc==SQLITE_OK ){
-      /* Read the newest schema cookie from the wal file. */
-      u32 iSlot = 0;
-      rc = walFindFrame(pWal, 1, iLast, &iSlot);
-      if( rc==SQLITE_OK && iSlot>pWal->hdr.mxFrame ){
-        u8 aNew[4];
-        u8 *aOld = &((u8*)pPage1->pData)[40];
-        int sz;
-        i64 iOffset;
-        sz = pWal->hdr.szPage;
-        sz = (sz&0xfe00) + ((sz&0x0001)<<16);
-        iOffset = walFrameOffset(iSlot, sz) + WAL_FRAME_HDRSIZE + 40;
-        rc = sqlite3OsRead(pWal->pWalFd, aNew, sizeof(aNew), iOffset);
-        if( rc==SQLITE_OK && memcmp(aOld, aNew, sizeof(aNew)) ){
-          /* TODO: New error code? SQLITE_BUSY_SCHEMA. */
+  if( rc==SQLITE_OK ){
+    volatile WalIndexHdr *pHead;    /* Head of the wal file */
+    pHead = walIndexHdr(pWal);
+    if( memcmp(&pWal->hdr, (void*)pHead, sizeof(WalIndexHdr))!=0 ){
+      int bSeenPage1 = 0;           /* True if page 1 is in list pList */
+  
+      /* TODO: Is this safe? Because it holds the WRITER lock this thread
+      ** has exclusive access to the live header, but might it be corrupt? */
+      PgHdr *pPg;
+      u32 iLast = pHead->mxFrame;
+      for(pPg=pList; rc==SQLITE_OK && pPg; pPg=pPg->pDirty){
+        u32 iSlot = 0;
+        rc = walFindFrame(pWal, pPg->pgno, iLast, &iSlot);
+        if( iSlot>pWal->hdr.mxFrame ){
+          sqlite3_log(SQLITE_OK,
+              "cannot commit UNLOCKED transaction (conflict at page %d)",
+              (int)pPg->pgno
+          );
           rc = SQLITE_BUSY_SNAPSHOT;
         }
+        if( pPg->pgno==1 ) bSeenPage1 = 1;
+      }
+  
+      /* If the current transaction does not modify page 1 of the database,
+      ** check if page 1 has been modified since the transaction was started.
+      ** If it has, check if the schema cookie value (the 4 bytes beginning at
+      ** byte offset 40) is the same as it is on pPage1. If not, this indicates
+      ** that the current head of the wal file uses a different schema than 
+      ** the snapshot against which the current transaction was prepared. Return
+      ** SQLITE_BUSY_SNAPSHOT in this case.  */
+      if( rc==SQLITE_OK && bSeenPage1==0 ){
+        u32 iSlot = 0;
+        rc = walFindFrame(pWal, 1, iLast, &iSlot);
+        if( rc==SQLITE_OK && iSlot>pWal->hdr.mxFrame ){
+          u8 aNew[4];
+          u8 *aOld = &((u8*)pPage1->pData)[40];
+          int sz;
+          i64 iOffset;
+          sz = pWal->hdr.szPage;
+          sz = (sz&0xfe00) + ((sz&0x0001)<<16);
+          iOffset = walFrameOffset(iSlot, sz) + WAL_FRAME_HDRSIZE + 40;
+          rc = sqlite3OsRead(pWal->pWalFd, aNew, sizeof(aNew), iOffset);
+          if( rc==SQLITE_OK && memcmp(aOld, aNew, sizeof(aNew)) ){
+            /* TODO: New error code? SQLITE_BUSY_SCHEMA. */
+            rc = SQLITE_BUSY_SNAPSHOT;
+          }
+        }
       }
     }
   }
@@ -2636,9 +2641,18 @@ int sqlite3WalLockForCommit(Wal *pWal, PgHdr *pList, PgHdr *pPage1){
 }
 
 /*
-** The caller holds the WRITER lock. This function returns true if a snapshot
-** upgrade is required before the transaction can be committed, or false
-** otherwise.
+** This function is only ever called while committing an UNLOCKED 
+** transaction, after the caller has already obtained the WRITER lock
+** (by calling the sqlite3WalLockForCommit() routine). This function 
+** returns true if the transaction was prepared against a database 
+** snapshot older than the current head of the wal file.
+**
+** Note that this will only work as described if the database is 
+** currently executing an UNLOCKED transaction, as it assumes that 
+** pWal->hdr has not been modified since the beginning of the 
+** transaction. This may not be true for a non-UNLOCKED transaction,
+** as pWal->hdr is updated if any pages are spilled to the wal file
+** while the transaction is executing.
 */
 int sqlite3WalCommitRequiresUpgrade(Wal *pWal){
   assert( pWal->writeLock );
@@ -2711,7 +2725,6 @@ int sqlite3WalUndo(Wal *pWal, int (*xUndo)(void *, Pgno), void *pUndoCtx){
 ** point in the event of a savepoint rollback (via WalSavepointUndo()).
 */
 void sqlite3WalSavepoint(Wal *pWal, u32 *aWalData){
-  /* assert( pWal->writeLock ); */
   aWalData[0] = pWal->hdr.mxFrame;
   aWalData[1] = pWal->hdr.aFrameCksum[0];
   aWalData[2] = pWal->hdr.aFrameCksum[1];
diff --git a/test/unlocked.test b/test/unlocked.test
index e5f7868a20..c7ae0466e5 100644
--- a/test/unlocked.test
+++ b/test/unlocked.test
@@ -279,7 +279,6 @@ do_multiclient_test tn {
   # 3. Attempt to commit the UNLOCKED write. This is an SQLITE_BUSY_SNAPSHOT,
   #    even though there is no page collision.
   #
-  
   do_test 2.$tn.5.1 {
     sql1 {
       BEGIN UNLOCKED;
@@ -300,6 +299,71 @@ do_multiclient_test tn {
   } {ok}
   catch { sql1 ROLLBACK }
 
+  #-----------------------------------------------------------------------
+  # The "schema cookie" issue.
+  #
+  # 1. Begin an UNLOCKED write to "t1" using [db]
+  #
+  # 2. Lots of inserts into t2. Enough to grow the db file.
+  #
+  # 3. Check that the UNLOCKED transaction can still be committed.
+  #
+  do_test 2.$tn.6.1 {
+    sql1 {
+      BEGIN UNLOCKED;
+        INSERT INTO t1 VALUES(6, 'six');
+    }
+  } {}
+
+  do_test 2.$tn.6.2 {
+    sql2 { 
+      WITH src(a,b) AS (
+        VALUES(1,1) UNION ALL SELECT a+1,b+1 FROM src WHERE a<10000
+      ) INSERT INTO t2 SELECT * FROM src;
+    }
+  } {}
+
+  do_test 2.$tn.6.3 {
+    sql1 {
+      SELECT count(*) FROM t2;
+      COMMIT;
+      SELECT count(*) FROM t2;
+    }
+  } {1 10001}
+
+  #-----------------------------------------------------------------------
+  # 
+  # 1. Begin an big UNLOCKED write to "t1" using [db] - large enough to
+  #    grow the db file.
+  #
+  # 2. Lots of inserts into t2. Also enough to grow the db file.
+  #
+  # 3. Check that the UNLOCKED transaction cannot be committed (due to a clash
+  #    on page 1 - the db size field).
+  #
+  do_test 2.$tn.7.1 {
+    sql1 {
+      BEGIN UNLOCKED;
+        WITH src(a,b) AS (
+          VALUES(10000,10000) UNION ALL SELECT a+1,b+1 FROM src WHERE a<20000
+        ) INSERT INTO t1 SELECT * FROM src;
+    }
+  } {}
+
+  do_test 2.$tn.7.2 {
+    sql2 { 
+      WITH src(a,b) AS (
+        VALUES(1,1) UNION ALL SELECT a+1,b+1 FROM src WHERE a<10000
+      ) INSERT INTO t2 SELECT * FROM src;
+    }
+  } {}
+
+  do_test 2.$tn.7.3 {
+    list [catch { sql1 { COMMIT } } msg] $msg [sqlite3_errcode db]
+  } {1 {database is locked} SQLITE_BUSY_SNAPSHOT}
+  sql1 ROLLBACK
+
+
 }