Add testcase() macros and improve comments in the LEFT JOIN strength reduction

optimization.

FossilOrigin-Name: 5613457714dd74d6da8e387132a0d8e64980ba4a921a9f53773540b02c0ccec6

3 files changed, 29 insertions, 9 deletions

diff --git a/manifest b/manifest
index d36a1c431..f4a8a0e93 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Yet\sanother\sfault\sin\sthe\ssqlite3ExprImpliesNotNull()\sroutine,\scausing\serrors\nin\sthe\sLEFT\sJOIN\sstrength\sreduction\soptimization\sof\scheck-in\n[dd568c27b1d76563].
-D 2018-03-24T15:47:31.300
+C Add\stestcase()\smacros\sand\simprove\scomments\sin\sthe\sLEFT\sJOIN\sstrength\sreduction\noptimization.
+D 2018-03-24T18:01:51.354
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F Makefile.in 7016fc56c6b9bfe5daac4f34be8be38d8c0b5fab79ccbfb764d3b23bf1c6fff3
@@ -443,7 +443,7 @@ F src/date.c ebe1dc7c8a347117bb02570f1a931c62dd78f4a2b1b516f4837d45b7d6426957
 F src/dbpage.c 8db4c97f630e7d83f884ea75caf1ffd0988c160e9d530194d93721c80821e0f6
 F src/dbstat.c 7a4ba8518b6369ef3600c49cf9c918ad979acba610b2aebef1b656d649b96720
 F src/delete.c 20c8788451dc737a967c87ea53ad43544d617f5b57d32ccce8bd52a0daf9e89b
-F src/expr.c 82d8b12a15fa197c4697f2a5a78cfb3abfc90d8058b4c36a8494f0a3d55b8e31
+F src/expr.c 51500461dcd4d0873a938bf188d03076d6712d70ff00a0f0d65621edf4e521c6
 F src/fault.c 460f3e55994363812d9d60844b2a6de88826e007
 F src/fkey.c d617daf66b5515e2b42c1405b2b4984c30ca50fb705ab164271a9bf66c69e331
 F src/func.c 94f42cba2cc1c34aeaa441022ba0170ec3fec4bba54db4e0ded085c6dc0fdc51
@@ -1717,7 +1717,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 2e06906e0959e1256fc2feb9dc2c3a9d1e9c19c4ba59585fe52df9af174ab443
-R 60924bc906d95198af7e174bd12ddecc
+P e88cf3d4df64097ebc19aae464b88d0faf9b7d4c30d057042b582d78327e7ad3
+R b8ae290c3d3eb7e9edb6abf4de8dcd8d
 U drh
-Z 1363428edd934c95156928d5375763e1
+Z 374891567f103c4731a42ae12a065dd4
diff --git a/manifest.uuid b/manifest.uuid
index e70fae593..0eb2afd35 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-e88cf3d4df64097ebc19aae464b88d0faf9b7d4c30d057042b582d78327e7ad3
\ No newline at end of file
+5613457714dd74d6da8e387132a0d8e64980ba4a921a9f53773540b02c0ccec6
\ No newline at end of file
diff --git a/src/expr.c b/src/expr.c
index bb26b3d04..14bd06a86 100644
--- a/src/expr.c
+++ b/src/expr.c
@@ -5007,6 +5007,15 @@ int sqlite3ExprImpliesExpr(Parse *pParse, Expr *pE1, Expr *pE2, int iTab){
 ** have a non-NULL column, then set pWalker->eCode to 1 and abort.
 */
 static int impliesNotNullRow(Walker *pWalker, Expr *pExpr){
+  /* This routine is only called for WHERE clause expressions and so it
+  ** cannot have any TK_AGG_COLUMN entries because those are only found
+  ** in HAVING clauses.  We can get a TK_AGG_FUNCTION in a WHERE clause,
+  ** but that is an illegal construct and the query will be rejected at
+  ** a later stage of processing, so the TK_AGG_FUNCTION case does not
+  ** need to be considered here. */
+  assert( pExpr->op!=TK_AGG_COLUMN );
+  testcase( pExpr->op==TK_AGG_FUNCTION );
+
   if( ExprHasProperty(pExpr, EP_FromJoin) ) return WRC_Prune;
   switch( pExpr->op ){
     case TK_ISNULL:
@@ -5015,10 +5024,14 @@ static int impliesNotNullRow(Walker *pWalker, Expr *pExpr){
     case TK_CASE:
     case TK_IN:
     case TK_FUNCTION:
-    case TK_AGG_FUNCTION:
+      testcase( pExpr->op==TK_ISNULL );
+      testcase( pExpr->op==TK_IS );
+      testcase( pExpr->op==TK_OR );
+      testcase( pExpr->op==TK_CASE );
+      testcase( pExpr->op==TK_IN );
+      testcase( pExpr->op==TK_FUNCTION );
       return WRC_Prune;
     case TK_COLUMN:
-    case TK_AGG_COLUMN:
       if( pWalker->u.iCur==pExpr->iTable ){
         pWalker->eCode = 1;
         return WRC_Abort;
@@ -5035,6 +5048,13 @@ static int impliesNotNullRow(Walker *pWalker, Expr *pExpr){
 ** if expression p will always be NULL or false if every column of iTab
 ** is NULL.
 **
+** False negatives are acceptable.  In other words, it is ok to return
+** zero even if expression p will never be true of every column of iTab
+** is NULL.  A false negative is merely a missed optimization opportunity.
+**
+** False positives are not allowed, however.  A false positive may result
+** in an incorrect answer.
+**
 ** Terms of p that are marked with EP_FromJoin (and hence that come from
 ** the ON or USING clauses of LEFT JOINS) are excluded from the analysis.
 **