Prevent the flattening or where-term push-down optimizations from obscuring

misuses of SQL row values that can lead to crashes or assert() failures.

FossilOrigin-Name: 433d16ff3adfede3be53d5b0e0512f37e225591b

diff --git a/manifest b/manifest
index 4c14cd36fb1e17968b6bab644bca429d0365bfe8..509a2a1b7d1c05c7def9eb4b47bf0c5b5d85f777 100644 (file)
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Always\shonor\sthe\ssqlite3.dbOptFlags\sbitmask,\sregardless\sof\scompile-time\noptions.\s\sContinuing\sfix\sfor\sticket\s[da78413751863].
-D 2016-12-07T13:49:11.158
+C Prevent\sthe\sflattening\sor\swhere-term\spush-down\soptimizations\sfrom\sobscuring\nmisuses\sof\sSQL\srow\svalues\sthat\scan\slead\sto\scrashes\sor\sassert()\sfailures.
+D 2016-12-07T15:38:37.162
 F Makefile.in 7639c6a09da11a9c7c6f2630fc981ee588d1072d
 F Makefile.linux-gcc 7bc79876b875010e8c8f9502eb935ca92aa3c434
 F Makefile.msc b8ca53350ae545e3562403d5da2a69cec79308da
@@ -341,7 +341,7 @@ F src/ctime.c a2a52d6e353f459d8ab0f07321f60fafa47d5421
 F src/date.c 57f23f5835c0b8b2be0e71f3b0a38e5960d053ec
 F src/dbstat.c 19ee7a4e89979d4df8e44cfac7a8f905ec89b77d
 F src/delete.c a6881ec5a7d1adc6157dfe1596ead3522db04740
-F src/expr.c b4db982acf30aad864a047bf7676fa85761fc55e
+F src/expr.c 0393a016d990c8c2023b737f5a143cdf308d429f
 F src/fault.c 160a0c015b6c2629d3899ed2daf63d75754a32bb
 F src/fkey.c 4017dc6d8bf894c4818ffbaf392e53c7c68dd761
 F src/func.c 43916c1d8e6da5d107d91d2b212577d4f69a876a
@@ -388,12 +388,12 @@ F src/printf.c f94da4935d1dd25420ac50c6745db1deb35e07c1
 F src/random.c ba2679f80ec82c4190062d756f22d0c358180696
 F src/resolve.c bb070cf5f23611c44ab7e4788803684e385fc3fb
 F src/rowset.c 7b7e7e479212e65b723bf40128c7b36dc5afdfac
-F src/select.c ee68944dfee9d548032bba3fdd5a6355011212c0
+F src/select.c 0e4920c4ab7b5751473a1b457ff7d57f4b54e288
 F src/shell.c a3fc2c719ed6d381895cbdb66a4a9b6a791cb02e
 F src/sqlite.h.in e8e2d108d82647f0a812fdb74accf91c1ec08ddc
 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
 F src/sqlite3ext.h 8648034aa702469afb553231677306cc6492a1ae
-F src/sqliteInt.h 181158754e1bd7837747d2459e54cb0e556f9c82
+F src/sqliteInt.h 33b14fb2b549001f2255565068c20fc8d883321e
 F src/sqliteLimit.h c0373387c287c8d0932510b5547ecde31b5da247
 F src/status.c a9e66593dfb28a9e746cba7153f84d49c1ddc4b1
 F src/table.c 5226df15ab9179b9ed558d89575ea0ce37b03fc9
@@ -1032,7 +1032,7 @@ F test/rollbackfault.test 0e646aeab8840c399cfbfa43daab46fd609cf04a
 F test/rowallock.test 3f88ec6819489d0b2341c7a7528ae17c053ab7cc
 F test/rowhash.test 0bc1d31415e4575d10cacf31e1a66b5cc0f8be81
 F test/rowid.test 5b7509f384f4f6fae1af3c8c104c8ca299fea18d
-F test/rowvalue.test f4dc97fe52d4e206e25b3395588b9497a8eed3ff
+F test/rowvalue.test 3f468a9e370a3ee8b267e58049679eb0df2b9d01
 F test/rowvalue2.test 060d238b7e5639a7c5630cb5e63e311b44efef2b
 F test/rowvalue3.test 3068f508753af69884b12125995f023da0dbb256
 F test/rowvalue4.test 4b556d7de161a0dd8cff095c336e913986398bea
@@ -1536,7 +1536,7 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 2a81763e68cdf9b8c46389b1e1a87bc2084b53e7
-R ad65390018631e2625a804a98b6533cb
-U drh
-Z 680af68799bfcad1d8fa388c257b6fc8
+P afab166313e0b8ad530df99887437a362398ed02
+R 805d4861d87a142d84fe0d34bae5d292
+U dan
+Z fc6459b5407c4f6a11de33c5ce52ff74

diff --git a/manifest.uuid b/manifest.uuid
index d540a8d3c1774a0ebb2e7a8ea4f5ebd7d3278227..bf18ea7c5c438920ee942816bfc6af3e38ae5bd8 100644 (file)
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-afab166313e0b8ad530df99887437a362398ed02
\ No newline at end of file
+433d16ff3adfede3be53d5b0e0512f37e225591b
\ No newline at end of file

diff --git a/src/expr.c b/src/expr.c
index 881123e314ce350bb5ba9b85c76d2df9c08d4114..a5833a65798357126ce3cdc68dd3e6a165ea5e63 100644 (file)
--- a/src/expr.c
+++ b/src/expr.c
@@ -2355,6 +2355,28 @@ void sqlite3SubselectError(Parse *pParse, int nActual, int nExpect){
 }
 #endif
 
+/*
+** Expression pExpr is a vector that has been used in a context where
+** it is not permitted. If pExpr is a sub-select vector, this routine 
+** loads the Parse object with a message of the form:
+**
+**   "sub-select returns N columns - expected 1"
+**
+** Or, if it is a regular scalar vector:
+**
+**   "row value misused"
+*/   
+void sqlite3VectorErrorMsg(Parse *pParse, Expr *pExpr){
+#ifndef SQLITE_OMIT_SUBQUERY
+  if( pExpr->flags & EP_xIsSelect ){
+    sqlite3SubselectError(pParse, pExpr->x.pSelect->pEList->nExpr, 1);
+  }else
+#endif
+  {
+    sqlite3ErrorMsg(pParse, "row value misused");
+  }
+}
+
 /*
 ** Generate code for scalar subqueries used as a subquery expression, EXISTS,
 ** or IN operators.  Examples:
@@ -2637,11 +2659,7 @@ int sqlite3ExprCheckIN(Parse *pParse, Expr *pIn){
       return 1;
     }
   }else if( nVector!=1 ){
-    if( (pIn->pLeft->flags & EP_xIsSelect) ){
-      sqlite3SubselectError(pParse, nVector, 1);
-    }else{
-      sqlite3ErrorMsg(pParse, "row value misused");
-    }
+    sqlite3VectorErrorMsg(pParse, pIn->pLeft);
     return 1;
   }
   return 0;

diff --git a/src/select.c b/src/select.c
index 26a4619dfdd16597ff81e4b5045768f5dd2d7dcd..71977c5f78badc42daab9d8712c9a85a1657336b 100644 (file)
--- a/src/select.c
+++ b/src/select.c
@@ -3133,8 +3133,8 @@ static int multiSelectOrderBy(
 
 #if !defined(SQLITE_OMIT_SUBQUERY) || !defined(SQLITE_OMIT_VIEW)
 /* Forward Declarations */
-static void substExprList(sqlite3*, ExprList*, int, ExprList*);
-static void substSelect(sqlite3*, Select *, int, ExprList*, int);
+static void substExprList(Parse*, ExprList*, int, ExprList*);
+static void substSelect(Parse*, Select *, int, ExprList*, int);
 
 /*
 ** Scan through the expression pExpr.  Replace every reference to
@@ -3150,40 +3150,46 @@ static void substSelect(sqlite3*, Select *, int, ExprList*, int);
 ** of the subquery rather the result set of the subquery.
 */
 static Expr *substExpr(
-  sqlite3 *db,        /* Report malloc errors to this connection */
+  Parse *pParse,      /* Report errors here */
   Expr *pExpr,        /* Expr in which substitution occurs */
   int iTable,         /* Table to be substituted */
   ExprList *pEList    /* Substitute expressions */
 ){
+  sqlite3 *db = pParse->db;
   if( pExpr==0 ) return 0;
   if( pExpr->op==TK_COLUMN && pExpr->iTable==iTable ){
     if( pExpr->iColumn<0 ){
       pExpr->op = TK_NULL;
     }else{
       Expr *pNew;
+      Expr *pCopy = pEList->a[pExpr->iColumn].pExpr;
       assert( pEList!=0 && pExpr->iColumn<pEList->nExpr );
       assert( pExpr->pLeft==0 && pExpr->pRight==0 );
-      pNew = sqlite3ExprDup(db, pEList->a[pExpr->iColumn].pExpr, 0);
-      if( pNew && (pExpr->flags & EP_FromJoin) ){
-        pNew->iRightJoinTable = pExpr->iRightJoinTable;
-        pNew->flags |= EP_FromJoin;
+      if( sqlite3ExprIsVector(pCopy) ){
+        sqlite3VectorErrorMsg(pParse, pCopy);
+      }else{
+        pNew = sqlite3ExprDup(db, pCopy, 0);
+        if( pNew && (pExpr->flags & EP_FromJoin) ){
+          pNew->iRightJoinTable = pExpr->iRightJoinTable;
+          pNew->flags |= EP_FromJoin;
+        }
+        sqlite3ExprDelete(db, pExpr);
+        pExpr = pNew;
       }
-      sqlite3ExprDelete(db, pExpr);
-      pExpr = pNew;
     }
   }else{
-    pExpr->pLeft = substExpr(db, pExpr->pLeft, iTable, pEList);
-    pExpr->pRight = substExpr(db, pExpr->pRight, iTable, pEList);
+    pExpr->pLeft = substExpr(pParse, pExpr->pLeft, iTable, pEList);
+    pExpr->pRight = substExpr(pParse, pExpr->pRight, iTable, pEList);
     if( ExprHasProperty(pExpr, EP_xIsSelect) ){
-      substSelect(db, pExpr->x.pSelect, iTable, pEList, 1);
+      substSelect(pParse, pExpr->x.pSelect, iTable, pEList, 1);
     }else{
-      substExprList(db, pExpr->x.pList, iTable, pEList);
+      substExprList(pParse, pExpr->x.pList, iTable, pEList);
     }
   }
   return pExpr;
 }
 static void substExprList(
-  sqlite3 *db,         /* Report malloc errors here */
+  Parse *pParse,       /* Report errors here */
   ExprList *pList,     /* List to scan and in which to make substitutes */
   int iTable,          /* Table to be substituted */
   ExprList *pEList     /* Substitute values */
@@ -3191,11 +3197,11 @@ static void substExprList(
   int i;
   if( pList==0 ) return;
   for(i=0; i<pList->nExpr; i++){
-    pList->a[i].pExpr = substExpr(db, pList->a[i].pExpr, iTable, pEList);
+    pList->a[i].pExpr = substExpr(pParse, pList->a[i].pExpr, iTable, pEList);
   }
 }
 static void substSelect(
-  sqlite3 *db,         /* Report malloc errors here */
+  Parse *pParse,       /* Report errors here */
   Select *p,           /* SELECT statement in which to make substitutions */
   int iTable,          /* Table to be replaced */
   ExprList *pEList,    /* Substitute values */
@@ -3206,17 +3212,17 @@ static void substSelect(
   int i;
   if( !p ) return;
   do{
-    substExprList(db, p->pEList, iTable, pEList);
-    substExprList(db, p->pGroupBy, iTable, pEList);
-    substExprList(db, p->pOrderBy, iTable, pEList);
-    p->pHaving = substExpr(db, p->pHaving, iTable, pEList);
-    p->pWhere = substExpr(db, p->pWhere, iTable, pEList);
+    substExprList(pParse, p->pEList, iTable, pEList);
+    substExprList(pParse, p->pGroupBy, iTable, pEList);
+    substExprList(pParse, p->pOrderBy, iTable, pEList);
+    p->pHaving = substExpr(pParse, p->pHaving, iTable, pEList);
+    p->pWhere = substExpr(pParse, p->pWhere, iTable, pEList);
     pSrc = p->pSrc;
     assert( pSrc!=0 );
     for(i=pSrc->nSrc, pItem=pSrc->a; i>0; i--, pItem++){
-      substSelect(db, pItem->pSelect, iTable, pEList, 1);
+      substSelect(pParse, pItem->pSelect, iTable, pEList, 1);
       if( pItem->fg.isTabFunc ){
-        substExprList(db, pItem->u1.pFuncArg, iTable, pEList);
+        substExprList(pParse, pItem->u1.pFuncArg, iTable, pEList);
       }
     }
   }while( doPrior && (p = p->pPrior)!=0 );
@@ -3741,7 +3747,7 @@ static int flattenSubquery(
     }else{
       pParent->pWhere = sqlite3ExprAnd(db, pWhere, pParent->pWhere);
     }
-    substSelect(db, pParent, iParent, pSub->pEList, 0);
+    substSelect(pParse, pParent, iParent, pSub->pEList, 0);
   
     /* The flattened query is distinct if either the inner or the
     ** outer query is distinct. 
@@ -3815,7 +3821,7 @@ static int flattenSubquery(
 ** terms are duplicated into the subquery.
 */
 static int pushDownWhereTerms(
-  sqlite3 *db,          /* The database connection (for malloc()) */
+  Parse *pParse,        /* Parse context (for malloc() and error reporting) */
   Select *pSubq,        /* The subquery whose WHERE clause is to be augmented */
   Expr *pWhere,         /* The WHERE clause of the outer query */
   int iCursor           /* Cursor number of the subquery */
@@ -3836,16 +3842,16 @@ static int pushDownWhereTerms(
     return 0; /* restriction (3) */
   }
   while( pWhere->op==TK_AND ){
-    nChng += pushDownWhereTerms(db, pSubq, pWhere->pRight, iCursor);
+    nChng += pushDownWhereTerms(pParse, pSubq, pWhere->pRight, iCursor);
     pWhere = pWhere->pLeft;
   }
   if( ExprHasProperty(pWhere,EP_FromJoin) ) return 0; /* restriction 5 */
   if( sqlite3ExprIsTableConstant(pWhere, iCursor) ){
     nChng++;
     while( pSubq ){
-      pNew = sqlite3ExprDup(db, pWhere, 0);
-      pNew = substExpr(db, pNew, iCursor, pSubq->pEList);
-      pSubq->pWhere = sqlite3ExprAnd(db, pSubq->pWhere, pNew);
+      pNew = sqlite3ExprDup(pParse->db, pWhere, 0);
+      pNew = substExpr(pParse, pNew, iCursor, pSubq->pEList);
+      pSubq->pWhere = sqlite3ExprAnd(pParse->db, pSubq->pWhere, pNew);
       pSubq = pSubq->pPrior;
     }
   }
@@ -5002,7 +5008,7 @@ int sqlite3Select(
     ** inside the subquery.  This can help the subquery to run more efficiently.
     */
     if( (pItem->fg.jointype & JT_OUTER)==0
-     && pushDownWhereTerms(db, pSub, p->pWhere, pItem->iCursor)
+     && pushDownWhereTerms(pParse, pSub, p->pWhere, pItem->iCursor)
     ){
 #if SELECTTRACE_ENABLED
       if( sqlite3SelectTrace & 0x100 ){

diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index 38b002a09f5c1f2a61449edf946572d3ae11a23e..4d0af9800587fcde95d5671627c0c16ce82399fc 100644 (file)
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -4286,5 +4286,6 @@ int sqlite3ExprVectorSize(Expr *pExpr);
 int sqlite3ExprIsVector(Expr *pExpr);
 Expr *sqlite3VectorFieldSubexpr(Expr*, int);
 Expr *sqlite3ExprForVectorField(Parse*,Expr*,int);
+void sqlite3VectorErrorMsg(Parse*, Expr*);
 
 #endif /* SQLITEINT_H */

diff --git a/test/rowvalue.test b/test/rowvalue.test
index 0df2e734c6422b7b6f5174c53a92c0138850a4f5..48fa8bc4d756fab03b7c33cd492e0b59b3774047 100644 (file)
--- a/test/rowvalue.test
+++ b/test/rowvalue.test
@@ -291,6 +291,31 @@ do_execsql_test 14.6 {
   SELECT 1 FROM t12 WHERE (1,x) BETWEEN (1,1) AND (3,3)
 } {1 1}
 
+#-------------------------------------------------------------------------
+# Test that errors are not concealed by the SELECT flattening or
+# WHERE-clause push-down optimizations.
+do_execsql_test 14.1 {
+  CREATE TABLE x1(a PRIMARY KEY, b);
+  CREATE TABLE x2(a INTEGER PRIMARY KEY, b);
+}
+
+foreach {tn n sql} {
+  1 0 "SELECT * FROM (SELECT (1, 1) AS c FROM x1) WHERE c=1"
+  2 2 "SELECT * FROM (SELECT 1 AS x, (SELECT 8,9) AS y) WHERE y<1"
+  3 3 "SELECT * FROM (SELECT 1 AS x, (SELECT 8,9,10) AS y) WHERE y<1"
+  4 0 "SELECT * FROM (SELECT (a, b) AS c FROM x1), x2 WHERE c=a"
+  5 0 "SELECT * FROM (SELECT a AS c, (1, 2, 3) FROM x1), x2 WHERE c=a"
+  6 0 "SELECT * FROM (SELECT 1 AS c, (1, 2, 3) FROM x1) WHERE c=1"
+} {
+  if {$n==0} {
+    set err "row value misused"
+  } else {
+    set err "sub-select returns $n columns - expected 1"
+  }
+  do_catchsql_test 14.2.$tn $sql [list 1 $err]
+}
+
+
 finish_test