Fix the OP_OpenDup opcode so that it is able to duplicate a cursor that

was itself opened by OP_OpenDup. Add additional verification of ephemeral tables. Fix for ticket [bb8a9fd4a9b7fce5]. FossilOrigin-Name: bcbe5308f3a3b94f965b0f5627cb29cce2e09343b86d757e2de889f7773576e7
oohtmeel1 · Mar 18, 2021 · 8d81aac · 8d81aac
2 parents 8df0149 + 76f7b16
commit 8d81aac
Show file tree

Hide file tree

Showing 7 changed files with 186 additions and 61 deletions.
diff --git a/manifest b/manifest
@@ -1,5 +1,5 @@
-C Fix\sthe\sdbsqlfuzz-derived\sauthorizer\sin\sfuzzcheck\sto\savoid\screating\sjunk\nfiles\swhen\srunning\srogue\stests.
-D 2021-03-18T14:36:19.107
+C Fix\sthe\sOP_OpenDup\sopcode\sso\sthat\sit\sis\sable\sto\sduplicate\sa\scursor\sthat\nwas\sitself\sopened\sby\sOP_OpenDup.\s\sAdd\sadditional\sverification\sof\nephemeral\stables.\s\sFix\sfor\sticket\s[bb8a9fd4a9b7fce5].
+D 2021-03-18T16:47:24.474
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -483,7 +483,7 @@ F src/auth.c 08954fdc4cc2da5264ba5b75cfd90b67a6fc7d1710a02ccf917c38eadec77853
 F src/backup.c 3014889fa06e20e6adfa0d07b60097eec1f6e5b06671625f476a714d2356513d
 F src/bitvec.c 17ea48eff8ba979f1f5b04cc484c7bb2be632f33
 F src/btmutex.c 8acc2f464ee76324bf13310df5692a262b801808984c1b79defb2503bbafadb6
-F src/btree.c f5334a1c0960dc0bc74fd0b263e352e8ba632f455b6107594c59cb0492efc4af
+F src/btree.c cfd2a37794532d765e235a2550ad2732924a6d06b07a3bc9f6a71750e3b3cca1
 F src/btree.h 096cc53baa58be22b02c896d1cf933c38cfc6d65f9253c1367ece8cc88a24de5
 F src/btreeInt.h 7bc15a24a02662409ebcd6aeaa1065522d14b7fda71573a2b0568b458f514ae0
 F src/build.c fec73c39d756f31d35ccbaa80fe1e040a8d675a318d4d30f41c444167bf3b860
@@ -614,11 +614,11 @@ F src/upsert.c df8f1727d62b5987c4fd302cd4d7c0c84ae57cd65683c5a34a740dfe24039235
 F src/utf.c ee39565f0843775cc2c81135751ddd93eceb91a673ea2c57f61c76f288b041a0
 F src/util.c 41c7a72da1df47864faa378a1c720b38adb288c6838cb6be5594511b6287a048
 F src/vacuum.c 492422c1463c076473bae1858799c7a0a5fe87a133d1223239447c422cd26286
-F src/vdbe.c 2cdbdb1e2df46fd65a72bcb2da7da2f9978407fa89b4af446e27e8f5c37a329e
+F src/vdbe.c 95105c16c8b6decd90645ac8cf3bf0bd34e08f3371fcdaa597853547bb114360
 F src/vdbe.h 25dabb25c7e157b84e59260cfb5b466c3ac103ede9f36f4db371332c47601abe
-F src/vdbeInt.h 3df118924e1711f1bbc8e30c46260d0ab6c3b029b32dd411f789111f76434f3c
+F src/vdbeInt.h 000d9ab1ea4cb55a80de15e28f3f595645b4fddef34bca4347fb3db8031d9041
 F src/vdbeapi.c 4a43e303ec3354c785f453e881521969378e85628278ab74ba4a9df790c0d93b
-F src/vdbeaux.c e982cb548cc29841a5b571eb56f666c3fe93e8c209ed172275fae3ff13b1ad88
+F src/vdbeaux.c 1b3eaa3a70d9d1877266e8ade0d0c3b2b4c6cf77d393d94dbcbd522b9bfefc15
 F src/vdbeblob.c 253ed82894924c362a7fa3079551d3554cd1cdace39aa833da77d3bc67e7c1b1
 F src/vdbemem.c 947f2a65910edb4014dc981d33e414a68c51f169f9df8c4c493a0ba840b6eb1f
 F src/vdbesort.c f5b5e473a7cee44e47a94817b042fd7172cf3aa2c0a7928a8339d612bcfdec5a
@@ -1788,7 +1788,7 @@ F test/windowerr.test a8b752402109c15aa1c5efe1b93ccb0ce1ef84fa964ae1cd6684dd0b3c
 F test/windowfault.test d543d46571b32d19f198cb04b6505747fabf3cc369970daae47074ee793612be
 F test/windowpushd.test 5b9c114e8173c3addacf58a0fcd941437b14649f2033700184479a13f188ad00
 F test/with1.test 780be387f01e290e768bdfd1827280f9e37ba37223eb4736aba386864fac5a94
-F test/with2.test e0030e2f0267a910d6c0e4f46f2dfe941c1cc0d4f659ba69b3597728e7e8f1ab
+F test/with2.test 7f890304fad815e49123d658012f4e463c7a2e9682044187ce21495e34a08ab9
 F test/with3.test 85e059bf4c2ef5626411ee59f399b4bb4b4a0f009bcb7db86f254e570ed11831
 F test/with4.test 257be66c0c67fee1defbbac0f685c3465e2cad037f21ce65f23f86084f198205
 F test/with5.test 6248213c41fab36290b5b73aa3f937309dfba337004d9d8434c3fabc8c7d4be8
@@ -1910,7 +1910,8 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P ff1eca7fcdcbc550a5f23db9d85281e5373500912699a7c7ae54bc0df0547e26
-R e02544653902282c973fe57144348e27
+P eca21a620630b0c8d21a91ad6a8760a6c87270ab2a45cafa18bd31b305c53dba edbfdcaefae1e9a42c046c058341bf5b0f533c0b0cbd39543fcaa2a83d67561d
+R 1fb218ba66105d95823b496a6214c38f
+T +closed edbfdcaefae1e9a42c046c058341bf5b0f533c0b0cbd39543fcaa2a83d67561d
 U drh
-Z a256c4c094c08cccb9232d4f40769c58
+Z 313966120306b4f08219912928f53326
diff --git a/manifest.uuid b/manifest.uuid
@@ -1 +1 @@
-eca21a620630b0c8d21a91ad6a8760a6c87270ab2a45cafa18bd31b305c53dba
+bcbe5308f3a3b94f965b0f5627cb29cce2e09343b86d757e2de889f7773576e7
diff --git a/src/btree.c b/src/btree.c
@@ -2737,19 +2737,23 @@ static void freeTempSpace(BtShared *pBt){
 */
 int sqlite3BtreeClose(Btree *p){
   BtShared *pBt = p->pBt;
-  BtCursor *pCur;
 
   /* Close all cursors opened via this handle.  */
   assert( sqlite3_mutex_held(p->db->mutex) );
   sqlite3BtreeEnter(p);
-  pCur = pBt->pCursor;
-  while( pCur ){
-    BtCursor *pTmp = pCur;
-    pCur = pCur->pNext;
-    if( pTmp->pBtree==p ){
-      sqlite3BtreeCloseCursor(pTmp);
+
+  /* Verify that no other cursors have this Btree open */
+#ifdef SQLITE_DEBUG
+  {
+    BtCursor *pCur = pBt->pCursor;
+    while( pCur ){
+      BtCursor *pTmp = pCur;
+      pCur = pCur->pNext;
+      assert( pTmp->pBtree!=p );
+
     }
   }
+#endif
 
   /* Rollback any active transaction and free the handle structure.
   ** The call to sqlite3BtreeRollback() drops any table-locks held by
@@ -4541,7 +4545,14 @@ int sqlite3BtreeCloseCursor(BtCursor *pCur){
     unlockBtreeIfUnused(pBt);
     sqlite3_free(pCur->aOverflow);
     sqlite3_free(pCur->pKey);
-    sqlite3BtreeLeave(pBtree);
+    if( (pBt->openFlags & BTREE_SINGLE) && pBt->pCursor==0 ){
+      /* Since the BtShared is not sharable, there is no need to
+      ** worry about the missing sqlite3BtreeLeave() call here.  */
+      assert( pBtree->sharable==0 );
+      sqlite3BtreeClose(pBtree);
+    }else{
+      sqlite3BtreeLeave(pBtree);
+    }
     pCur->pBtree = 0;
   }
   return SQLITE_OK;

diff --git a/src/vdbe.c b/src/vdbe.c
@@ -272,11 +272,6 @@ static VdbeCursor *allocateCursor(
 
   assert( iCur>=0 && iCur<p->nCursor );
   if( p->apCsr[iCur] ){ /*OPTIMIZATION-IF-FALSE*/
-    /* Before calling sqlite3VdbeFreeCursor(), ensure the isEphemeral flag
-    ** is clear. Otherwise, if this is an ephemeral cursor created by 
-    ** OP_OpenDup, the cursor will not be closed and will still be part
-    ** of a BtShared.pCursor list.  */
-    if( p->apCsr[iCur]->pBtx==0 ) p->apCsr[iCur]->isEphemeral = 0;
     sqlite3VdbeFreeCursor(p, p->apCsr[iCur]);
     p->apCsr[iCur] = 0;
   }
@@ -3873,7 +3868,7 @@ case OP_OpenDup: {
 
   pOrig = p->apCsr[pOp->p2];
   assert( pOrig );
-  assert( pOrig->pBtx!=0 );  /* Only ephemeral cursors can be duplicated */
+  assert( pOrig->isEphemeral );  /* Only ephemeral cursors can be duplicated */
 
   pCx = allocateCursor(p, pOp->p1, pOrig->nField, -1, CURTYPE_BTREE);
   if( pCx==0 ) goto no_mem;
@@ -3883,7 +3878,10 @@ case OP_OpenDup: {
   pCx->isTable = pOrig->isTable;
   pCx->pgnoRoot = pOrig->pgnoRoot;
   pCx->isOrdered = pOrig->isOrdered;
-  rc = sqlite3BtreeCursor(pOrig->pBtx, pCx->pgnoRoot, BTREE_WRCSR,
+  pCx->pBtx = pOrig->pBtx;
+  pCx->hasBeenDuped = 1;
+  pOrig->hasBeenDuped = 1;
+  rc = sqlite3BtreeCursor(pCx->pBtx, pCx->pgnoRoot, BTREE_WRCSR, 
                           pCx->pKeyInfo, pCx->uc.pCursor);
   /* The sqlite3BtreeCursor() routine can only fail for the first cursor
   ** opened for a database.  Since there is already an open cursor when this
@@ -3949,9 +3947,10 @@ case OP_OpenEphemeral: {
     aMem[pOp->p3].z = "";
   }
   pCx = p->apCsr[pOp->p1];
-  if( pCx && ALWAYS(pCx->pBtx) ){
-    /* If the ephermeral table is already open, erase all existing content
-    ** so that the table is empty again, rather than creating a new table. */
+  if( pCx && !pCx->hasBeenDuped ){
+    /* If the ephermeral table is already open and has no duplicates from
+    ** OP_OpenDup, then erase all existing content so that the table is
+    ** empty again, rather than creating a new table. */
     assert( pCx->isEphemeral );
     pCx->seqCount = 0;
     pCx->cacheStatus = CACHE_STALE;
@@ -3965,33 +3964,36 @@ case OP_OpenEphemeral: {
                           vfsFlags);
     if( rc==SQLITE_OK ){
       rc = sqlite3BtreeBeginTrans(pCx->pBtx, 1, 0);
-    }
-    if( rc==SQLITE_OK ){
-      /* If a transient index is required, create it by calling
-      ** sqlite3BtreeCreateTable() with the BTREE_BLOBKEY flag before
-      ** opening it. If a transient table is required, just use the
-      ** automatically created table with root-page 1 (an BLOB_INTKEY table).
-      */
-      if( (pCx->pKeyInfo = pKeyInfo = pOp->p4.pKeyInfo)!=0 ){
-        assert( pOp->p4type==P4_KEYINFO );
-        rc = sqlite3BtreeCreateTable(pCx->pBtx, &pCx->pgnoRoot,
-                                     BTREE_BLOBKEY | pOp->p5); 
-        if( rc==SQLITE_OK ){
-          assert( pCx->pgnoRoot==SCHEMA_ROOT+1 );
-          assert( pKeyInfo->db==db );
-          assert( pKeyInfo->enc==ENC(db) );
-          rc = sqlite3BtreeCursor(pCx->pBtx, pCx->pgnoRoot, BTREE_WRCSR,
-                                  pKeyInfo, pCx->uc.pCursor);
+      if( rc==SQLITE_OK ){
+        /* If a transient index is required, create it by calling
+        ** sqlite3BtreeCreateTable() with the BTREE_BLOBKEY flag before
+        ** opening it. If a transient table is required, just use the
+        ** automatically created table with root-page 1 (an BLOB_INTKEY table).
+        */
+        if( (pCx->pKeyInfo = pKeyInfo = pOp->p4.pKeyInfo)!=0 ){
+          assert( pOp->p4type==P4_KEYINFO );
+          rc = sqlite3BtreeCreateTable(pCx->pBtx, &pCx->pgnoRoot,
+              BTREE_BLOBKEY | pOp->p5); 
+          if( rc==SQLITE_OK ){
+            assert( pCx->pgnoRoot==SCHEMA_ROOT+1 );
+            assert( pKeyInfo->db==db );
+            assert( pKeyInfo->enc==ENC(db) );
+            rc = sqlite3BtreeCursor(pCx->pBtx, pCx->pgnoRoot, BTREE_WRCSR,
+                pKeyInfo, pCx->uc.pCursor);
+          }
+          pCx->isTable = 0;
+        }else{
+          pCx->pgnoRoot = SCHEMA_ROOT;
+          rc = sqlite3BtreeCursor(pCx->pBtx, SCHEMA_ROOT, BTREE_WRCSR,
+              0, pCx->uc.pCursor);
+          pCx->isTable = 1;
         }
-        pCx->isTable = 0;
-      }else{
-        pCx->pgnoRoot = SCHEMA_ROOT;
-        rc = sqlite3BtreeCursor(pCx->pBtx, SCHEMA_ROOT, BTREE_WRCSR,
-                                0, pCx->uc.pCursor);
-        pCx->isTable = 1;
+      }
+      pCx->isOrdered = (pOp->p5!=BTREE_UNORDERED);
+      if( rc ){
+        sqlite3BtreeClose(pCx->pBtx);
       }
     }
-    pCx->isOrdered = (pOp->p5!=BTREE_UNORDERED);
   }
   if( rc ) goto abort_due_to_error;
   pCx->nullRow = 1;

diff --git a/src/vdbeInt.h b/src/vdbeInt.h
@@ -86,6 +86,7 @@ struct VdbeCursor {
   Bool isEphemeral:1;     /* True for an ephemeral table */
   Bool useRandomRowid:1;  /* Generate new record numbers semi-randomly */
   Bool isOrdered:1;       /* True if the table is not BTREE_UNORDERED */
+  Bool hasBeenDuped:1;    /* This cursor was source or target of OP_OpenDup */
   u16 seekHit;            /* See the OP_SeekHit and OP_IfNoHope opcodes */
   Btree *pBtx;            /* Separate file holding temporary table */
   i64 seqCount;           /* Sequence counter */

diff --git a/src/vdbeaux.c b/src/vdbeaux.c
@@ -2479,14 +2479,8 @@ void sqlite3VdbeFreeCursor(Vdbe *p, VdbeCursor *pCx){
       break;
     }
     case CURTYPE_BTREE: {
-      if( pCx->isEphemeral ){
-        if( pCx->pBtx ) sqlite3BtreeClose(pCx->pBtx);
-        /* The pCx->pCursor will be close automatically, if it exists, by
-        ** the call above. */
-      }else{
-        assert( pCx->uc.pCursor!=0 );
-        sqlite3BtreeCloseCursor(pCx->uc.pCursor);
-      }
+      assert( pCx->uc.pCursor!=0 );
+      sqlite3BtreeCloseCursor(pCx->uc.pCursor);
       break;
     }
 #ifndef SQLITE_OMIT_VIRTUALTABLE

diff --git a/test/with2.test b/test/with2.test
@@ -414,5 +414,121 @@ do_execsql_test 8.3 {
   SELECT * FROM q;
 } {1 2 3 4 5}
 
+# 2021-03-18
+# Ticket bb8a9fd4a9b7fce5
+reset_db
+do_execsql_test 9.1 {
+  WITH xyz(a) AS (
+    WITH abc AS ( SELECT 1234 ) SELECT * FROM abc
+  )
+  SELECT * FROM xyz AS one, xyz AS two, (
+    SELECT * FROM xyz UNION ALL SELECT * FROM xyz
+  );
+} {1234 1234 1234 1234 1234 1234}
+load_static_extension db series
+do_execsql_test 9.2 {
+  WITH
+    cst(rsx, rsy) AS  (
+      SELECT 100, 100
+    ),
+    cst2(minx, maxx, stepx, miny, maxy, stepy, qualitativex, qualitativey) AS (
+      SELECT NULL, NULL, NULL, NULL, NULL, NULL, 0, 0
+    ),
+    ds0(m, n, x, y, x2, y2, title, size, mark, label, markmode) AS (
+      SELECT 1, 2, 3, 4, 5, 6, 7 , 8, 9, 10, 11
+    ),
+    ds(m, n, x, y, x2, y2, title, size, mark, label, markmode) AS (
+      SELECT m, n, x,
+        y, x2,
+        y2,
+        title, size, mark, label, markmode
+      FROM ds0
+      WINDOW w AS (PARTITION BY m, x ORDER BY n)
+    ),
+    d(m, n, x, y, x2, y2, labelx,labely,title,size,mark,label,markmode) AS (
+      SELECT m, n, x, y,  x2, y2, x, y, title, size, mark, label, markmode
+      FROM ds, cst2
+    ),
+    ylabels(y, label) AS (
+      SELECT y, MIN(labely) FROM d GROUP BY y
+    ),
+    yaxis(maxy, miny, stepy , minstepy) AS (
+      WITH
+        xt0(minx, maxx) AS (
+          SELECT  coalesce(miny, min(min(y2),
+                  min(y))), coalesce(maxy, max(max(y2),
+                  max(y))) + qualitativey
+           FROM d, cst2
+        ),
+        xt1(mx, mn) AS (SELECT maxx, minx FROM xt0),
+        xt2(mx, mn, step) AS (SELECT mx, mn, (mx-mn)  FROM xt1),
+
+        xt3(mx, mn, ms) AS (
+          SELECT mx, mn, first_value(rs) OVER (order by x desc) AS ms
+            FROM (SELECT mx, mn, step, f,(mx-mn) as rng,
+                         1.0*step/f as rs, 1.0*(mx-mn)/(step/f) AS x
+                    FROM xt2, (SELECT 1 AS f UNION ALL SELECT 2
+                                UNION ALL SELECT 4
+                                UNION ALL SELECT 5)) AS src
+                   WHERE x < 10 limit 1),
+        xt4(minstepy) AS (
+          SELECT MIN(abs(y2-y)) FROM d WHERE y2 != y
+        )
+      SELECT (mx/ms)*ms, (mn/ms)*ms, coalesce(stepy, ms),
+                     coalesce(minstepy, ms, stepy)  FROM xt3, cst2,xt4
+    ),
+    distinct_mark_n_m(mark, ze, zem, title) AS (
+      SELECT DISTINCT mark, n AS ze, m AS zem, title FROM ds0
+    ),
+    facet0(m, mi, title, radial) AS (
+      SELECT md, row_number() OVER () - 1, title, 'radial'
+                      IN (SELECT mark FROM distinct_mark_n_m WHERE zem = md)
+      FROM (SELECT DISTINCT zem AS md, title AS title
+                       FROM distinct_mark_n_m ORDER BY 2, 1)
+    ),
+    facet(m, mi, xorigin, yorigin, title, radial) AS (
+      SELECT m, mi,
+        rsx * 1.2 * IFNULL(CASE WHEN (
+          0
+        ) > 0 THEN mi / (
+          0
+        ) ELSE mi % (
+          2
+        )  END, mi),
+        rsy  * 1.2 * IFNULL(CASE WHEN (
+          2
+        ) > 0 THEN mi / (
+          2
+        ) ELSE mi / (
+          0
+        )  END, 0),
+        title, radial FROM facet0, cst
+    ),
+    radygrid(m, mi, tty, wty, ttx, ttx2, xorigin, yorigin) AS (
+      SELECT m, mi,  rsy / 2 / ((maxy-miny)/stepy) * (value-1) AS tty,
+             coalesce(NULL, miny + stepy * (value-1)) AS wty,
+             xorigin, xorigin+rsx, xorigin + rsx / 2,
+             yorigin + rsy / 2
+        FROM generate_series(1), yaxis, cst,
+             facet LEFT JOIN ylabels ON ylabels.y = (miny + (value-1) * stepy)
+       WHERE radial AND stop = 1+1.0*(maxy-miny)/stepy
+    ),
+    ypos(m, mi, pcx, pcy, radial) AS (
+      SELECT m, mi, xorigin, yorigin + CASE
+        WHEN 0 BETWEEN miny AND maxy THEN
+          rsy - (0 - miny) * rsy / (maxy-miny)
+        WHEN 0 >= maxy THEN 0
+        ELSE  rsy
+      END, radial FROM yaxis, cst, facet WHERE NOT radial
+      UNION ALL
+      SELECT m, mi, xorigin + rsx / 2, yorigin + (CASE
+        WHEN 0 BETWEEN miny AND maxy THEN
+          rsy - (0 - miny) * rsy / 2 / (maxy-miny)
+        WHEN 0 >= maxy THEN 0
+        ELSE  rsy
+      END ) / 2, radial FROM yaxis, cst, facet WHERE radial
+    )
+  SELECT * FROM radygrid , ypos;
+} {}
 
 finish_test
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		eca21a620630b0c8d21a91ad6a8760a6c87270ab2a45cafa18bd31b305c53dba
		bcbe5308f3a3b94f965b0f5627cb29cce2e09343b86d757e2de889f7773576e7