An attempt to enhance PRAGMA integrity check so that it does data type

checking on non-STRICT tables.  Specifically:  (1) Columns with TEXT affinity
should not contain numeric values, and (2) columns with numeric affinity should
not contain text values that can be converted to numeric.

FossilOrigin-Name: 8b1e7f0524637728cebe81c7d3ff8ad8a5a55782eac6409b425dad538024f596

diff --git a/manifest b/manifest
index 59f182ca2807857f7877f7469276b57d4baecd29..f227e86d95e441db6cdec59fb73a475ab884f10d 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Improved\sdetection\sof\sdatabase\scorrupting\swhen\smoving\spages\son\san\nautovacuumed\sdatabase\swhen\screating\sa\snew\sbtree.\ndbsqlfuzz\s9a55397eae13cec64aebf1fd35489c3a90bdaac5
-D 2022-10-10T12:02:53.891
+C An\sattempt\sto\senhance\sPRAGMA\sintegrity\scheck\sso\sthat\sit\sdoes\sdata\stype\nchecking\son\snon-STRICT\stables.\s\sSpecifically:\s\s(1)\sColumns\swith\sTEXT\saffinity\nshould\snot\scontain\snumeric\svalues,\sand\s(2)\scolumns\swith\snumeric\saffinity\sshould\nnot\scontain\stext\svalues\sthat\scan\sbe\sconverted\sto\snumeric.
+D 2022-10-10T18:25:05.328
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -581,7 +581,7 @@ F src/parse.y 8e67d820030d2655b9942ffe61c1e7e6b96cea2f2f72183533299393907d0564
 F src/pcache.c f4268f7f73c6a3db12ce22fd25bc68dc42315d19599414ab1207d7cf32f79197
 F src/pcache.h 4f87acd914cef5016fae3030343540d75f5b85a1877eed1a2a19b9f284248586
 F src/pcache1.c dee95e3cd2b61e6512dc814c5ab76d5eb36f0bfc9441dbb4260fccc0d12bbddc
-F src/pragma.c 9bf7d8a2a9ad3bc36df3ec0d61817a44c38a1da527d59c26c203047f906e334a
+F src/pragma.c f9df5f90b20cbfc8a25f2e9129b185d8eba37ecd74ce156df814e1aaf8ce1183
 F src/pragma.h e690a356c18e98414d2e870ea791c1be1545a714ba623719deb63f7f226d8bb7
 F src/prepare.c 1b02be0441eda4579471fea097f678effcbb77ef0c39ab3f703c837822bcd674
 F src/printf.c e99ee9741e79ae3873458146f59644276657340385ade4e76a5f5d1c25793764
@@ -655,12 +655,12 @@ F src/threads.c 4ae07fa022a3dc7c5beb373cf744a85d3c5c6c3c
 F src/tokenize.c 1305797eab3542a0896b552c6e7669c972c1468e11e92b370533c1f37a37082b
 F src/treeview.c 07787f67cd297a6d09d04b8d70c06769c60c9c1d9080378f93929c16f8fd3298
 F src/trigger.c 4163ada044af89d51caba1cb713a73165347b2ec05fe84a283737c134d61fcd5
-F src/update.c c52a7991bece0453d22c77c08469512ee2f1391c12503fd347d1c939220c5877
+F src/update.c 832cfcf09edcfcc383032b6cc63eb578dc0658c9380e58e1d26f703bba3c5339
 F src/upsert.c 8789047a8f0a601ea42fa0256d1ba3190c13746b6ba940fe2d25643a7e991937
 F src/utf.c ee39565f0843775cc2c81135751ddd93eceb91a673ea2c57f61c76f288b041a0
 F src/util.c 0be191521ff6d2805995f4910f0b6231b42843678b2efdc1abecaf39929a673f
 F src/vacuum.c bb346170b0b54c6683bba4a5983aea40485597fdf605c87ec8bc2e199fe88cd8
-F src/vdbe.c a26eef5a6c0ebb97f83ec18bd483e0d17f0c6b079e7650085f2ef6a82fd78d99
+F src/vdbe.c af03458e12f39cddd6f355ff325059db5431dbe45233fff5a18a7eb0065e535c
 F src/vdbe.h 64619af62603dc3c4f5ff6ff6d2c8f389abd667a29ce6007ed44bd22b3211cd0
 F src/vdbeInt.h 17b7461ffcf9ee760d1341731715a419f6b8c763089a7ece25c2e8098d702b3f
 F src/vdbeapi.c fc3183daf72808b4311b228989120fdbc2dc44972fb0d77d5c453460cc0e5b2c
@@ -1262,7 +1262,7 @@ F test/memdb1.test 2c4e9cc10d21c6bf4e217d72b7f6b8ba9b2605971bb2c5e6df76018e189f9
 F test/memjournal.test 70f3a00c7f84ee2978ad14e831231caa1e7f23915a2c54b4f775a021d5740c6c
 F test/memjournal2.test 6b9083cfaab9a3281ec545c3da2487999e8025fb7501bbae10f713f80c56454c
 F test/memleak.test 10b9c6c57e19fc68c32941495e9ba1c50123f6e2
-F test/memsubsys1.test 9e7555a22173b8f1c96c281ce289b338fcba2abe8b157f8798ca195bbf1d347e
+F test/memsubsys1.test 86b8158752af9188ed5b32a30674a1ef71183e6bc4e6808e815cd658ca9058a6
 F test/memsubsys2.test 3e4a8d0c05fd3e5fa92017c64666730a520c7e08
 F test/merge1.test 2de6d6ef8d25402764b1aab49d8f9d7f89208c89a6674e437f76de4c812157b8
 F test/minmax.test fe638b55d77d2375531a8f549b338eafcd9adfbd2f72df37ed77d9b26ca0a71a
@@ -2002,8 +2002,11 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 1e2796b3741a4c0b966e6c01a7d62dea8de9d1f4bededea3d7ba7004dabc2ffe
-R 87396513be591e6dc5cb0d1c97d70179
+P 327965bc71023d9380a9d6805062659108dab4bfcd386b7aba813754f270d33a
+R 482ab94cca7e794e7641d30e6ad55424
+T *branch * integrity_check_datatypes
+T *sym-integrity_check_datatypes *
+T -sym-trunk *
 U drh
-Z bc3536edefe4a38a9e628abd562bbf72
+Z 974e346bb8769b9c6b996ee2e6df90d2
 # Remove this line to create a well-formed Fossil manifest.

diff --git a/manifest.uuid b/manifest.uuid
index c5ea5a3715501e551fad381274788e02bdc1761c..548a45a6712c93260f4e1e4cb073d965dfcb25a9 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-327965bc71023d9380a9d6805062659108dab4bfcd386b7aba813754f270d33a
\ No newline at end of file
+8b1e7f0524637728cebe81c7d3ff8ad8a5a55782eac6409b425dad538024f596
\ No newline at end of file

diff --git a/src/pragma.c b/src/pragma.c
index 9d46a10dcdbaea9ee566b0deea4b50e80b68e385..018fed5efd7902bc72ffff9c9d4756078399100f 100644 (file)
--- a/src/pragma.c
+++ b/src/pragma.c
@@ -1778,11 +1778,11 @@ void sqlite3Pragma(
         assert( sqlite3NoTempsInRange(pParse,1,7+j) );
         sqlite3VdbeAddOp2(v, OP_Rewind, iDataCur, 0); VdbeCoverage(v);
         loopTop = sqlite3VdbeAddOp2(v, OP_AddImm, 7, 1);
+        /* Sanity check on record header decoding */
+        sqlite3VdbeAddOp3(v, OP_Column, iDataCur, pTab->nNVCol-1,3);
+        sqlite3VdbeChangeP5(v, OPFLAG_TYPEOFARG);
+        VdbeComment((v, "(right-most column)"));
         if( !isQuick ){
-          /* Sanity check on record header decoding */
-          sqlite3VdbeAddOp3(v, OP_Column, iDataCur, pTab->nNVCol-1,3);
-          sqlite3VdbeChangeP5(v, OPFLAG_TYPEOFARG);
-          VdbeComment((v, "(right-most column)"));
           if( pPk ){
             /* Verify WITHOUT ROWID keys are in ascending order */
             int a1;
@@ -1802,22 +1802,60 @@ void sqlite3Pragma(
             }
           }
         }
-        /* Verify that all NOT NULL columns really are NOT NULL.  At the
-        ** same time verify the type of the content of STRICT tables */
+        /* Verify datatypes for all columns:
+        **
+        **   (1) NOT NULL columns may not contain a NULL
+        **   (2) Datatype must be exact for non-ANY columns in STRICT tables
+        **   (3) Datatype for TEXT columns in non-STRICT tables must be
+        **       NULL, TEXT, or BLOB.
+        **   (4) Datatype for numeric columns in non-STRICT tables must not
+        **       be a TEXT value that can be losslessly converted to numeric.
+        */
         bStrict = (pTab->tabFlags & TF_Strict)!=0;
         for(j=0; j<pTab->nCol; j++){
           char *zErr;
-          Column *pCol = pTab->aCol + j;
+          Column *pCol = pTab->aCol + j;  /* The column to be checked */
           int doError, jmp2;
+          int p1, p3, p4;    /* Operands to the OP_IsType opcode */
+          int doTypeCheck;   /* Data type check (other than NOT NULL) needed */
+
           if( j==pTab->iPKey ) continue;
-          if( pCol->notNull==0 && !bStrict ) continue;
-          doError = bStrict ? sqlite3VdbeMakeLabel(pParse) : 0;
-          sqlite3ExprCodeGetColumnOfTable(v, pTab, iDataCur, j, 3);
-          if( sqlite3VdbeGetLastOp(v)->opcode==OP_Column ){
-            sqlite3VdbeChangeP5(v, OPFLAG_TYPEOFARG);
+          if( bStrict ){
+            doTypeCheck = pCol->eCType>COLTYPE_ANY;
+          }else{
+            doTypeCheck = pCol->affinity>SQLITE_AFF_BLOB;
+          }
+          if( pCol->notNull==0 && !doTypeCheck ) continue;
+          p4 = 0x10;
+          if( pCol->colFlags & COLFLAG_VIRTUAL ){
+            sqlite3ExprCodeGetColumnOfTable(v, pTab, iDataCur, j, 3);
+            p1 = -1;
+            p3 = 3;
+          }else{
+            if( pCol->iDflt ){
+              sqlite3_value *pDfltValue = 0;
+              sqlite3ValueFromExpr(db, sqlite3ColumnExpr(pTab,pCol), ENC(db),
+                                   pCol->affinity, &pDfltValue);
+              if( pDfltValue ){
+                p4 = sqlite3_value_type(pDfltValue);
+                sqlite3ValueFree(pDfltValue);
+              }
+            }
+            p1 = iDataCur;
+            if( !HasRowid(pTab) ){
+              testcase( j!=sqlite3TableColumnToStorage(pTab, j) );
+              p3 = sqlite3TableColumnToIndex(sqlite3PrimaryKeyIndex(pTab), j);
+            }else{
+              p3 = sqlite3TableColumnToStorage(pTab,j);
+              testcase( p3!=j);
+            }
           }
+          doError = doTypeCheck ? sqlite3VdbeMakeLabel(pParse) : 0;
           if( pCol->notNull ){
-            jmp2 = sqlite3VdbeAddOp1(v, OP_NotNull, 3); VdbeCoverage(v);
+            /* (1) NOT NULL columns may not contain a NULL */
+            jmp2 = sqlite3VdbeAddOp4Int(v, OP_IsType, p1, 0, p3, p4);
+            sqlite3VdbeChangeP5(v, 0x0f);
+            VdbeCoverage(v);
             zErr = sqlite3MPrintf(db, "NULL value in %s.%s", pTab->zName,
                                 pCol->zCnName);
             sqlite3VdbeAddOp4(v, OP_String8, 0, 3, 0, zErr, P4_DYNAMIC);
@@ -1828,9 +1866,19 @@ void sqlite3Pragma(
             }
             sqlite3VdbeJumpHere(v, jmp2);
           }
-          if( bStrict && pCol->eCType!=COLTYPE_ANY ){
-            jmp2 = sqlite3VdbeAddOp3(v, OP_IsNullOrType, 3, 0, 
-                                     sqlite3StdTypeMap[pCol->eCType-1]);
+          if( bStrict && doTypeCheck ){
+            /* (2) Datatype must be exact for non-ANY columns in STRICT tables*/
+            static unsigned char aStdTypeMask[] = {
+               0x1f,    /* ANY */
+               0x18,    /* BLOB */
+               0x11,    /* INT */
+               0x11,    /* INTEGER */
+               0x13,    /* REAL */
+               0x14     /* TEXT */
+            };
+            jmp2 = sqlite3VdbeAddOp4Int(v, OP_IsType, p1, 0, p3, p4);
+            assert( pCol->eCType>=1 && pCol->eCType<=sizeof(aStdTypeMask) );
+            sqlite3VdbeChangeP5(v, aStdTypeMask[pCol->eCType-1]);
             VdbeCoverage(v);
             zErr = sqlite3MPrintf(db, "non-%s value in %s.%s",
                                   sqlite3StdType[pCol->eCType-1],
@@ -1840,6 +1888,41 @@ void sqlite3Pragma(
             integrityCheckResultRow(v);
             sqlite3VdbeJumpHere(v, jmp2);
           }
+          if( !bStrict && pCol->affinity==SQLITE_AFF_TEXT ){
+            /* (3) Datatype for TEXT columns in non-STRICT tables must be
+            **     NULL, TEXT, or BLOB. */
+            jmp2 = sqlite3VdbeAddOp4Int(v, OP_IsType, p1, 0, p3, p4);
+            sqlite3VdbeChangeP5(v, 0x1c); /* NULL, TEXT, or BLOB */
+            VdbeCoverage(v);
+            zErr = sqlite3MPrintf(db, "NUMERIC value in %s.%s",
+                                  pTab->zName, pTab->aCol[j].zCnName);
+            sqlite3VdbeAddOp4(v, OP_String8, 0, 3, 0, zErr, P4_DYNAMIC);
+            sqlite3VdbeResolveLabel(v, doError);
+            integrityCheckResultRow(v);
+            sqlite3VdbeJumpHere(v, jmp2);
+          }
+          if( !bStrict && pCol->affinity>=SQLITE_AFF_NUMERIC ){
+            /* (4) Datatype for numeric columns in non-STRICT tables must not
+            **     be a TEXT value that can be converted to numeric. */
+            int jmp3;
+            jmp2 = sqlite3VdbeAddOp4Int(v, OP_IsType, p1, 0, p3, p4);
+            sqlite3VdbeChangeP5(v, 0x1b); /* NULL, INT, FLOAT, or BLOB */
+            VdbeCoverage(v);
+            if( p1>=0 ){
+              sqlite3ExprCodeGetColumnOfTable(v, pTab, iDataCur, j, 3);
+            }
+            sqlite3VdbeAddOp4(v, OP_Affinity, 3, 1, 0, "C", P4_STATIC);
+            jmp3 = sqlite3VdbeAddOp4Int(v, OP_IsType, -1, 0, 3, p4);
+            sqlite3VdbeChangeP5(v, 0x1c); /* NULL, TEXT, or BLOB */
+            VdbeCoverage(v);
+            zErr = sqlite3MPrintf(db, "TEXT value in %s.%s",
+                                  pTab->zName, pTab->aCol[j].zCnName);
+            sqlite3VdbeAddOp4(v, OP_String8, 0, 3, 0, zErr, P4_DYNAMIC);
+            sqlite3VdbeResolveLabel(v, doError);
+            integrityCheckResultRow(v);
+            sqlite3VdbeJumpHere(v, jmp2);
+            sqlite3VdbeJumpHere(v, jmp3);         
+          }
         }
         /* Verify CHECK constraints */
         if( pTab->pCheck && (db->flags & SQLITE_IgnoreChecks)==0 ){

diff --git a/src/update.c b/src/update.c
index 3fd78a0b5af545861b3cabc49a7361559ea8382f..a90dfaaeab34a236fda5cdadf93f7ce87d72cea6 100644 (file)
--- a/src/update.c
+++ b/src/update.c
@@ -59,11 +59,14 @@ static void updateVirtualTable(
 ** it has been converted into REAL.
 */
 void sqlite3ColumnDefault(Vdbe *v, Table *pTab, int i, int iReg){
+  Column *pCol = &pTab->aCol[i];
   assert( pTab!=0 );
-  if( !IsView(pTab) ){
+  assert( pTab->nCol>i );
+  pCol = &pTab->aCol[i];
+  if( pCol->iDflt ){
     sqlite3_value *pValue = 0;
     u8 enc = ENC(sqlite3VdbeDb(v));
-    Column *pCol = &pTab->aCol[i];
+    assert( !IsView(pTab) );
     VdbeComment((v, "%s.%s", pTab->zName, pCol->zCnName));
     assert( i<pTab->nCol );
     sqlite3ValueFromExpr(sqlite3VdbeDb(v), 
@@ -74,7 +77,7 @@ void sqlite3ColumnDefault(Vdbe *v, Table *pTab, int i, int iReg){
     }
   }
 #ifndef SQLITE_OMIT_FLOATING_POINT
-  if( pTab->aCol[i].affinity==SQLITE_AFF_REAL && !IsVirtual(pTab) ){
+  if( pCol->affinity==SQLITE_AFF_REAL && !IsVirtual(pTab) ){
     sqlite3VdbeAddOp1(v, OP_RealAffinity, iReg);
   }
 #endif

diff --git a/src/vdbe.c b/src/vdbe.c
index 1797aca006b25e40476f758be4e07bf1b7a992b1..4d21eec117c1ee3690d29975ff8d62395f78b66b 100644 (file)
--- a/src/vdbe.c
+++ b/src/vdbe.c
@@ -2588,19 +2588,66 @@ case OP_IsNull: {            /* same as TK_ISNULL, jump, in1 */
   break;
 }
 
-/* Opcode: IsNullOrType P1 P2 P3 * *
-** Synopsis: if typeof(r[P1]) IN (P3,5) goto P2
+/* Opcode: IsType P1 P2 P3 P4 P5
+** Synopsis: if typeof(P1.P3) in P5 goto P2
+**
+** Jump to P2 if the type of a column in a btree is one of the types specified
+** by the P5 bitmask.
+**
+** P1 is normally a cursor on a btree for which the row decode cache is
+** valid through at least column P3.  In other words, there should have been
+** a prior OP_Column for column P3 or greater.  If the cursor is not valid,
+** then this opcode might give spurious results.
+** The the btree row has fewer than P3 columns, then use P4 as the
+** datatype.
+**
+** If P1 is -1, then P3 is a register number and the datatype is taken
+** from the value in that register.
+**
+** P5 is a bitmask of data types.  SQLITE_INTEGER is the least significant
+** (0x01) bit. SQLITE_FLOAT is the 0x02 bit. SQLITE_TEXT is 0x04.
+** SQLITE_BLOB is 0x08.  SQLITE_NULL is 0x10.
+**
+** Take the jump to address P2 if and only if the datatype of the
+** value determined by P1 and P3 corresponds to one of the bits in the
+** P5 bitmask.
 **
-** Jump to P2 if the value in register P1 is NULL or has a datatype P3.
-** P3 is an integer which should be one of SQLITE_INTEGER, SQLITE_FLOAT,
-** SQLITE_BLOB, SQLITE_NULL, or SQLITE_TEXT.
 */
-case OP_IsNullOrType: {      /* jump, in1 */
-  int doTheJump;
-  pIn1 = &aMem[pOp->p1];
-  doTheJump = (pIn1->flags & MEM_Null)!=0 || sqlite3_value_type(pIn1)==pOp->p3;
-  VdbeBranchTaken( doTheJump, 2);
-  if( doTheJump ) goto jump_to_p2;
+case OP_IsType: {
+  VdbeCursor *pC;
+  u16 typeMask;
+  u32 serialType;
+
+  assert( pOp->p1>=(-1) && pOp->p1<p->nCursor );
+  assert( pOp->p1>=0 || (pOp->p3>=0 && pOp->p3<=(p->nMem+1 - p->nCursor)) );
+  if( pOp->p1>=0 ){
+    pC = p->apCsr[pOp->p1];
+    assert( pC!=0 );
+    assert( pOp->p3>=0 );
+    if( pOp->p3<(u32)pC->nHdrParsed ){
+      serialType = pC->aType[pOp->p3];
+      if( serialType==0 ){
+        typeMask = 0x10;   /* SQLITE_NULL */
+      }else if( serialType==7 ){
+        typeMask = 0x02;   /* SQLITE_FLOAT */
+      }else if( serialType<12 ){
+        typeMask = 0x01;   /* SQLITE_INTEGER */
+      }else if( serialType&1 ){
+        typeMask = 0x04;   /* SQLITE_TEXT */
+      }else{
+        typeMask = 0x08;   /* SQLITE_BLOB */
+      }
+    }else{
+      typeMask = 1 << (pOp->p4.i - 1);
+    }
+  }else{
+    assert( memIsValid(&aMem[pOp->p3]) );
+    typeMask = 1 << (sqlite3_value_type((sqlite3_value*)&aMem[pOp->p3])-1);
+  }
+  VdbeBranchTaken( (typeMask & pOp->p5)!=0, 2);
+  if( typeMask & pOp->p5 ){
+    goto jump_to_p2;
+  }
   break;
 }
 

diff --git a/test/memsubsys1.test b/test/memsubsys1.test
index 41bc11526944f8f7ad86ed1acb13ed348d62e4f6..fda35a918bd7fb0fb8ce084d0e475ef17b1501a5 100644 (file)
--- a/test/memsubsys1.test
+++ b/test/memsubsys1.test
@@ -174,7 +174,7 @@ do_test memsubsys1-4.4 {
 } 0
 do_test memsubsys1-4.5 {
   set maxreq [lindex [sqlite3_status SQLITE_STATUS_MALLOC_SIZE 0] 2]
-  expr {$maxreq<7000}
+  expr {$maxreq<9000}
 } 1
 
 db close