From: drh <>
Date: Fri, 8 Mar 2024 21:37:18 +0000 (+0000)
Subject: The NOT NULL strength reduction optimization from [de9c86c9e4cdb34f] should
X-Git-Tag: version-3.46.0~148
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=61b77a6fe14d271c89b0c392898b83dc05c658e5;p=thirdparty%2Fsqlite.git

The NOT NULL strength reduction optimization from [de9c86c9e4cdb34f] should
be applied to the WHERE clause only.  Otherwise, the operand of the IS NULL
or IS NOT NULL operator might be a reference to a bare column of an
aggregate table, and we can't tell if it is NULL or not based only on its
NOT NULL attribute.  [forum:/forumpost/440f2a2f17|Forum post 440f2a2f17].

FossilOrigin-Name: 51704feae224eff601db5607f8651da11b3c2ed8a58ffe5b6ee8260cab50695b
---

diff --git a/manifest b/manifest
index fcee6b38c4..c35db0c838 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C One\sof\sthe\sassert()s\sadded\sby\sthe\sprevious\scheck-in\swas\snot\squite\scorrect.\nThis\scommit\sfixes\sit.
-D 2024-03-08T19:03:30.426
+C The\sNOT\sNULL\sstrength\sreduction\soptimization\sfrom\s[de9c86c9e4cdb34f]\sshould\nbe\sapplied\sto\sthe\sWHERE\sclause\sonly.\s\sOtherwise,\sthe\soperand\sof\sthe\sIS\sNULL\nor\sIS\sNOT\sNULL\soperator\smight\sbe\sa\sreference\sto\sa\sbare\scolumn\sof\san\naggregate\stable,\sand\swe\scan't\stell\sif\sit\sis\sNULL\sor\snot\sbased\sonly\son\sits\nNOT\sNULL\sattribute.\s\s[forum:/forumpost/440f2a2f17|Forum\spost\s440f2a2f17].
+D 2024-03-08T21:37:18.921
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -748,14 +748,14 @@ F src/pragma.h e690a356c18e98414d2e870ea791c1be1545a714ba623719deb63f7f226d8bb7
 F src/prepare.c 371f6115cb69286ebc12c6f2d7511279c2e47d9f54f475d46a554d687a3b312c
 F src/printf.c 10e8bad30042f8bd6114a013b4afc229ec8ad255ab27518d7d9f52e8cbc5cd0a
 F src/random.c 606b00941a1d7dd09c381d3279a058d771f406c5213c9932bbd93d5587be4b9c
-F src/resolve.c d77c6160bc8f249c2196fdd3e75f66a1dd70e37aa25c39aedc7b1f93c42b7c6d
+F src/resolve.c ef87e3bc7700bfe761a7bbee2ce6084f1766dc816dd82a3ae77c133eec898432
 F src/rowset.c 8432130e6c344b3401a8874c3cb49fefe6873fec593294de077afea2dce5ec97
 F src/select.c 43fabfc01bf87addd15e39f112f1e2ade15b19594835ab8a9e5bd50839d4e1b1
 F src/shell.c.in 78bbd861cd0128aed67c0136561572ebcf11649be6cea86bee8491576d5958d0
 F src/sqlite.h.in 19a2db3995a699bd7f6dfb423856242bfceb7ec849a93c91d241d19fc28d9f0f
 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
 F src/sqlite3ext.h 3f046c04ea3595d6bfda99b781926b17e672fd6d27da2ba6d8d8fc39981dcb54
-F src/sqliteInt.h 06d757ee6cd22f59593e51a7066327a0690a6cb66dad4f0077ee3297228f5401
+F src/sqliteInt.h 6123ce6ca6a1ef351c3b87189e92c92042728f16c088de56b9b5bc2552d0ae33
 F src/sqliteLimit.h 6878ab64bdeb8c24a1d762d45635e34b96da21132179023338c93f820eee6728
 F src/status.c cb11f8589a6912af2da3bb1ec509a94dd8ef27df4d4c1a97e0bcf2309ece972b
 F src/table.c 0f141b58a16de7e2fbe81c308379e7279f4c6b50eb08efeec5892794a0ba30d1
@@ -1455,7 +1455,7 @@ F test/notify1.test 669b2b743618efdc18ca4b02f45423d5d2304abf
 F test/notify2.test 2ecabaa1305083856b7c39cf32816b612740c161
 F test/notify3.test 796c7b7157f55c93b4e672b724e9c923a6fc6aa72ac419379a623e2350472e22
 F test/notnull.test a37b663d5bb728d66fc182016613fb8e4a0a4bbf3d75b8876a7527f7d4ed3f18
-F test/notnull2.test 1ee4acbd614d3cf5f1c4a52f5af7fc771b82352f1a51a86afeaa02c9df1d82ef
+F test/notnull2.test 2ac7b4e04917148c7a1a9ed36df20150175ce942f07f5714375b29acbaca7106
 F test/notnullfault.test fc4bb7845582a2b3db376001ef49118393b1b11abe0d24adb03db057ee2b73d5
 F test/null.test b7ff206a1c60fe01aa2abd33ef9ea83c93727d993ca8a613de86e925c9f2bc6f
 F test/nulls1.test 7a5e4346ee4285034100b4cd20e6784f16a9d6c927e44ecdf10034086bbee9c9
@@ -2177,8 +2177,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 166d1e5d26ef88e995f44182144891f60bd51c1aa585b4a148f01a920b2a8eea
-R a10b9cfdedf65df80cd98f59556bbeca
+P d401358329f5a70f9a0b9b033609a4db2af89b83c6b40242be0c76f3d6474def
+R 05ce52a0a931825416b225aa397af1ac
 U drh
-Z 203e497a1090dbea6a05556e060a1144
+Z ce9603354c09773b286e10e1291765af
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index a61603c541..8202619d3e 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-d401358329f5a70f9a0b9b033609a4db2af89b83c6b40242be0c76f3d6474def
\ No newline at end of file
+51704feae224eff601db5607f8651da11b3c2ed8a58ffe5b6ee8260cab50695b
\ No newline at end of file
diff --git a/src/resolve.c b/src/resolve.c
index 5d0801e82e..c2957a870a 100644
--- a/src/resolve.c
+++ b/src/resolve.c
@@ -971,6 +971,19 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){
     ** resolved.  This prevents "column" from being counted as having been
     ** referenced, which might prevent a SELECT from being erroneously
     ** marked as correlated.
+    **
+    ** 2024-03-28: Beware of aggregates.  A bare column of aggregated table
+    ** can still evaluate to NULL even though it is marked as NOT NULL.
+    ** Example:
+    **
+    **       CREATE TABLE t1(a INT NOT NULL);
+    **       SELECT a, a IS NULL, a IS NOT NULL, count(*) FROM t1;
+    **
+    ** The "a IS NULL" and "a IS NOT NULL" expressions cannot be optimized
+    ** here because at the time this case is hit, we do not yet know whether
+    ** or not t1 is being aggregated.  We have to assume the worst and omit
+    ** the optimization.  The only time it is safe to apply this optimization
+    ** is within the WHERE clause.
     */
     case TK_NOTNULL:
     case TK_ISNULL: {
@@ -981,19 +994,36 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){
         anRef[i] = p->nRef;
       }
       sqlite3WalkExpr(pWalker, pExpr->pLeft);
-      if( 0==sqlite3ExprCanBeNull(pExpr->pLeft) && !IN_RENAME_OBJECT ){
-        testcase( ExprHasProperty(pExpr, EP_OuterON) );
-        assert( !ExprHasProperty(pExpr, EP_IntValue) );
-        pExpr->u.iValue = (pExpr->op==TK_NOTNULL);
-        pExpr->flags |= EP_IntValue;
-        pExpr->op = TK_INTEGER;
+      if( IN_RENAME_OBJECT ) return WRC_Prune;
+      if( sqlite3ExprCanBeNull(pExpr->pLeft) ){
+        /* The expression can be NULL.  So the optimization does not apply */
+        return WRC_Prune;
+      }
 
-        for(i=0, p=pNC; p && i<ArraySize(anRef); p=p->pNext, i++){
-          p->nRef = anRef[i];
+      for(i=0, p=pNC; p; p=p->pNext, i++){
+        if( (p->ncFlags & NC_Where)==0 ){
+          return WRC_Prune;  /* Not in a WHERE clause.  Unsafe to optimize. */
         }
-        sqlite3ExprDelete(pParse->db, pExpr->pLeft);
-        pExpr->pLeft = 0;
       }
+      testcase( ExprHasProperty(pExpr, EP_OuterON) );
+      assert( !ExprHasProperty(pExpr, EP_IntValue) );
+#if TREETRACE_ENABLED
+      if( sqlite3TreeTrace & 0x80000 ){
+        sqlite3DebugPrintf(
+           "NOT NULL strength reduction converts the following to %d:\n",
+           pExpr->op==TK_NOTNULL
+        );
+        sqlite3ShowExpr(pExpr);
+      }
+#endif /* TREETRACE_ENABLED */
+      pExpr->u.iValue = (pExpr->op==TK_NOTNULL);
+      pExpr->flags |= EP_IntValue;
+      pExpr->op = TK_INTEGER;
+      for(i=0, p=pNC; p && i<ArraySize(anRef); p=p->pNext, i++){
+        p->nRef = anRef[i];
+      }
+      sqlite3ExprDelete(pParse->db, pExpr->pLeft);
+      pExpr->pLeft = 0;
       return WRC_Prune;
     }
 
@@ -1891,7 +1921,9 @@ static int resolveSelectStep(Walker *pWalker, Select *p){
       }
       if( sqlite3ResolveExprNames(&sNC, p->pHaving) ) return WRC_Abort;
     }
+    sNC.ncFlags |= NC_Where;
     if( sqlite3ResolveExprNames(&sNC, p->pWhere) ) return WRC_Abort;
+    sNC.ncFlags &= ~NC_Where;
 
     /* Resolve names in table-valued-function arguments */
     for(i=0; i<p->pSrc->nSrc; i++){
diff --git a/src/sqliteInt.h b/src/sqliteInt.h
index 91cf173040..e11b5b3a4e 100644
--- a/src/sqliteInt.h
+++ b/src/sqliteInt.h
@@ -1125,6 +1125,7 @@ extern u32 sqlite3TreeTrace;
 **   0x00010000     Beginning of DELETE/INSERT/UPDATE processing
 **   0x00020000     Transform DISTINCT into GROUP BY
 **   0x00040000     SELECT tree dump after all code has been generated
+**   0x00080000     NOT NULL strength reduction
 */
 
 /*
@@ -3453,6 +3454,7 @@ struct NameContext {
 #define NC_InAggFunc 0x020000 /* True if analyzing arguments to an agg func */
 #define NC_FromDDL   0x040000 /* SQL text comes from sqlite_schema */
 #define NC_NoSelect  0x080000 /* Do not descend into sub-selects */
+#define NC_Where     0x100000 /* Processing WHERE clause of a SELECT */
 #define NC_OrderAgg 0x8000000 /* Has an aggregate other than count/min/max */
 
 /*
diff --git a/test/notnull2.test b/test/notnull2.test
index 7f68086810..09161efbdb 100644
--- a/test/notnull2.test
+++ b/test/notnull2.test
@@ -59,14 +59,14 @@ do_vmstep_test 1.4.2 {
 
 do_vmstep_test 1.5.1 {
   SELECT count(*) FROM t2 WHERE EXISTS(
-    SELECT t2.d IS NULL FROM t1 WHERE t1.a=450
+    SELECT 1 FROM t1 WHERE t1.a=450 AND t2.d IS NULL
   )
-} 10000 {1000}
+} 7000 {0}
 do_vmstep_test 1.5.2 {
   SELECT count(*) FROM t2 WHERE EXISTS(
-    SELECT t2.c IS NULL FROM t1 WHERE t1.a=450
+    SELECT 1 FROM t1 WHERE t1.a=450 AND t2.c IS NULL
   )
-} +100000 {1000}
+} +8000 {0}
 
 #-------------------------------------------------------------------------
 reset_db
@@ -111,4 +111,12 @@ do_execsql_test 4.1 {
   SELECT * FROM (SELECT 3 AS c FROM t1) AS t3 LEFT JOIN t2 ON c IS NULL;
 } {3 {}}
 
+# 2024-03-08 https://sqlite.org/forum/forumpost/440f2a2f17
+#
+reset_db
+do_execsql_test 5.0 {
+  CREATE TABLE t1(a INT NOT NULL);
+  SELECT a IS NULL, a IS NOT NULL, count(*) FROM t1;
+} {1 0 0}
+
 finish_test