From d7480403bc6b9d50130465596a7fb4b7a9fe9b30 Mon Sep 17 00:00:00 2001
From: drh <>
Date: Mon, 20 Jun 2022 17:04:44 +0000
Subject: [PATCH] Do not allow an ON clause to references tables to its right
 if there is a RIGHT or LEFT join anywhere in the query.  Other RDBMSes
 prohibit this always, but SQLite must allow ON clauses to reference tables to
 their right for legacy compatibility, unless there is a RIGHT or LEFT join
 someplace in the query, in which case there is no legacy to support.

FossilOrigin-Name: e615dbe02ca949252d1526ed5c48f8ce08159773ea2008ce666484379d0d9854
---
 ext/rtree/rtree1.test | 15 ++++++++++++---
 manifest              | 18 +++++++++---------
 manifest.uuid         |  2 +-
 src/select.c          | 18 ++++++++++--------
 src/whereexpr.c       | 12 +++++++++---
 test/join8.test       |  8 +++++---
 6 files changed, 46 insertions(+), 27 deletions(-)

diff --git a/ext/rtree/rtree1.test b/ext/rtree/rtree1.test
index e5b55a28e4..034155341b 100644
--- a/ext/rtree/rtree1.test
+++ b/ext/rtree/rtree1.test
@@ -741,10 +741,19 @@ do_execsql_test 20.0 {
   CREATE TABLE t0(a INT);
   CREATE TABLE t1(b INT);
   INSERT INTO rt0 VALUES(0, 0, 0);
+}
+do_catchsql_test 20.1 {
   SELECT * FROM t1 JOIN t0 ON x0>a RIGHT JOIN rt0 ON true WHERE +x0 = 0;
-} {}
-do_execsql_test 20.1 {
+} {1 {ON clause references tables to its right}}
+do_catchsql_test 20.2 {
   SELECT * FROM t1 JOIN t0 ON x0>a RIGHT JOIN rt0 ON true WHERE x0 = 0;
-} {}
+} {1 {ON clause references tables to its right}}
+db null -
+do_execsql_test 20.3 {
+  SELECT * FROM t1 JOIN t0 ON true RIGHT JOIN rt0 ON x0>a WHERE +x0 = 0;
+} {- - 0 0.0 0.0}
+do_execsql_test 20.4 {
+  SELECT * FROM t1 JOIN t0 ON true RIGHT JOIN rt0 ON x0>a WHERE x0 = 0;
+} {- - 0 0.0 0.0}
 
 finish_test
diff --git a/manifest b/manifest
index 5496a6747d..a9812cacf6 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C The\sfix\sat\s[cab9b4cccd13bf0a]\swas\sincomplete,\sas\sdemonstrated\sby\n[forum:/forumpost/57bdf2217d|forum\spost\s57bdf2217d].\s\sThis\scheck-in\nshould\scomplete\sthe\sfix.
-D 2022-06-20T12:42:28.345
+C Do\snot\sallow\san\sON\sclause\sto\sreferences\stables\sto\sits\sright\sif\sthere\sis\sa\nRIGHT\sor\sLEFT\sjoin\sanywhere\sin\sthe\squery.\s\sOther\sRDBMSes\sprohibit\sthis\salways,\nbut\sSQLite\smust\sallow\sON\sclauses\sto\sreference\stables\sto\stheir\sright\sfor\slegacy\ncompatibility,\sunless\sthere\sis\sa\sRIGHT\sor\sLEFT\sjoin\ssomeplace\sin\sthe\squery,\nin\swhich\scase\sthere\sis\sno\slegacy\sto\ssupport.
+D 2022-06-20T17:04:44.141
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -416,7 +416,7 @@ F ext/rtree/README 6315c0d73ebf0ec40dedb5aa0e942bc8b54e3761
 F ext/rtree/geopoly.c cc3f89c11abcf114fa60d74709ae8b5bc1eae5a261b30bc1bb7085089c03bfab
 F ext/rtree/rtree.c d7b4b8b81d8d54376a7f81de5be85ec58b37c11604bcf42984a8418b34158d93
 F ext/rtree/rtree.h 4a690463901cb5e6127cf05eb8e642f127012fd5003830dbc974eca5802d9412
-F ext/rtree/rtree1.test 70c092dcf34716b64edc9dbab171790322aa87b89a48f7437b8b4c7b466449c4
+F ext/rtree/rtree1.test d47f58832145fcfed9067bc457ca8664962196c4566c17a1ebd679367db55d11
 F ext/rtree/rtree2.test 9d9deddbb16fd0c30c36e6b4fdc3ee3132d765567f0f9432ee71e1303d32603d
 F ext/rtree/rtree3.test 4ee5d7df86040efe3d8d84f141f2962a7745452200a7cba1db06f86d97050499
 F ext/rtree/rtree4.test 304de65d484540111b896827e4261815e5dca4ce28eeecd58be648cd73452c4b
@@ -570,7 +570,7 @@ F src/printf.c 6166a30417b05c5b2f82e1f183f75faa2926ad60531c0b688a57dbc951441a20
 F src/random.c 097dc8b31b8fba5a9aca1697aeb9fd82078ec91be734c16bffda620ced7ab83c
 F src/resolve.c a4eb3c617027fd049b07432f3b942ea7151fa793a332a11a7d0f58c9539e104f
 F src/rowset.c ba9515a922af32abe1f7d39406b9d35730ed65efab9443dc5702693b60854c92
-F src/select.c e3181898cc05e8a78195f1792f170760df49262d6267dcaa834160900aab7d37
+F src/select.c baadb1b3fd0e87f82f0c416b68a5b8d90d8f4ffb3568c9a86f84a2be7d40261f
 F src/shell.c.in 08e59f1cb9d9b1180aba52861aaada0c95f6ddd210488719684e160a0724c806
 F src/sqlite.h.in 172528c287399a34f188154017b7268bf82c6d5b780902e361958d2318c4e37c
 F src/sqlite3.rc 5121c9e10c3964d5755191c80dd1180c122fc3a8
@@ -660,7 +660,7 @@ F src/walker.c f890a3298418d7cba3b69b8803594fdc484ea241206a8dfa99db6dd36f8cbb3b
 F src/where.c 2db3d41a1fd3b93d86680b4f33530d1f96ce9941771ebee80effba071d9d1e38
 F src/whereInt.h b48ca529ffe293c18cbfa8326af18a09e39910de66fb3e96ef788c7cbf8ef3a7
 F src/wherecode.c 0b09abfcb88c61c6a6984a3e065786631ff35495e9bdf865e6b74ab0a1299c5b
-F src/whereexpr.c 20255cf03e0b765b742301197d165511ff99e95da0d7ee9c8a2ebc1e888dd049
+F src/whereexpr.c 3b5f9f6f9fc07039b242ecf7a66e87392e9c49d62923b3c593c904090bd9345c
 F src/window.c fff1b51757438c664e471d5184634e48dcdf8ea34b640f3b1b0810b1e06de18c
 F test/8_3_names.test ebbb5cd36741350040fd28b432ceadf495be25b2
 F test/affinity2.test ce1aafc86e110685b324e9a763eab4f2a73f737842ec3b687bd965867de90627
@@ -1168,7 +1168,7 @@ F test/join4.test 1a352e4e267114444c29266ce79e941af5885916
 F test/join5.test d22b6cba8fb59ab3f1c82701434c360705eb12d4ce200c449f37b018fc47681a
 F test/join6.test f809c025fa253f9e150c0e9afd4cef8813257bceeb6f46e04041228c9403cc2c
 F test/join7.test 2268dcbb54b724391dda3748ea95c60d960607ffeed67885675998e7117697f6
-F test/join8.test c839d7cd4704b600468fd4d82d92bfe3eac67623c41dbfb99f804cdad7a3846e
+F test/join8.test 6d063ef3fa580ff9e50015107b0e6caf51e7055731749ba6391b8f11c1e00e3f
 F test/join9.test 9056ddd3b0c0f4f9d658f4521038d9a37dc23ead8ca9a505d0b0db2b6a471e05
 F test/joinA.test 7eab225dc1c1ab258a5e62513a4ed7cabbd3db971d59d5d92f4fb6fa14c12f6a
 F test/joinB.test 1b2ba3fc8568b49411787fccbf540570c148e9b6a53a30f80691cb6268098ded
@@ -1978,8 +1978,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 11162446f12ae3af6e4a63bb5c374129b2505f6006f91d4028c7165f05fe9651
-R e2f301053cf79999e11ddef561e846ed
+P fb0a23b6789da8e934562ce9ebd9d58ea13a10fd10dee5cbfc7ac8f394e1aeec
+R 9a31acf363f55129767709f03c1bfdcf
 U drh
-Z 238d98417a47860062f9a4d41505b756
+Z ae2f170f0ee7cf50c320a853890db816
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index b75dd0fdc6..bd8414421c 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-fb0a23b6789da8e934562ce9ebd9d58ea13a10fd10dee5cbfc7ac8f394e1aeec
\ No newline at end of file
+e615dbe02ca949252d1526ed5c48f8ce08159773ea2008ce666484379d0d9854
\ No newline at end of file
diff --git a/src/select.c b/src/select.c
index f062d2f4df..5c73c6bf63 100644
--- a/src/select.c
+++ b/src/select.c
@@ -424,11 +424,13 @@ void sqlite3SetJoinExpr(Expr *p, int iTable, u32 joinFlag){
   } 
 }
 
-/* Undo the work of sqlite3SetJoinExpr(). In the expression p, convert every
-** term that is marked with EP_OuterON and w.iJoin==iTable into
-** an ordinary term that omits the EP_OuterON mark.
+/* Undo the work of sqlite3SetJoinExpr().  This is used when a LEFT JOIN
+** is simplified into an ordinary JOIN, and when an ON expression is
+** "pushed down" into the WHERE clause of a subquery.
 **
-** This happens when a LEFT JOIN is simplified into an ordinary JOIN.
+** Convert every term that is marked with EP_OuterON and w.iJoin==iTable into
+** an ordinary term that omits the EP_OuterON mark.  Or if iTable<0, then
+** just clear every EP_OuterON and EP_InnerON mark from the expression tree.
 **
 ** If nullable is true, that means that Expr p might evaluate to NULL even
 ** if it is a reference to a NOT NULL column.  This can happen, for example,
@@ -438,10 +440,9 @@ void sqlite3SetJoinExpr(Expr *p, int iTable, u32 joinFlag){
 */
 static void unsetJoinExpr(Expr *p, int iTable, int nullable){
   while( p ){
-    if( ExprHasProperty(p, EP_OuterON)
-     && (iTable<0 || p->w.iJoin==iTable) ){
-      ExprClearProperty(p, EP_OuterON);
-      ExprSetProperty(p, EP_InnerON);
+    if( iTable<0 || (ExprHasProperty(p, EP_OuterON) && p->w.iJoin==iTable) ){
+      ExprClearProperty(p, EP_OuterON|EP_InnerON);
+      if( iTable>=0 ) ExprSetProperty(p, EP_InnerON);
     }
     if( p->op==TK_COLUMN && p->iTable==iTable && !nullable ){
       ExprClearProperty(p, EP_CanBeNull);
@@ -6771,6 +6772,7 @@ int sqlite3Select(
       SELECTTRACE(0x100,pParse,p,
                 ("LEFT-JOIN simplifies to JOIN on term %d\n",i));
       pItem->fg.jointype &= ~(JT_LEFT|JT_OUTER);
+      assert( pItem->iCursor>=0 );
       unsetJoinExpr(p->pWhere, pItem->iCursor,
                     pTabList->a[0].fg.jointype & JT_LTORJ);
     }
diff --git a/src/whereexpr.c b/src/whereexpr.c
index 878e69f633..52da05ddde 100644
--- a/src/whereexpr.c
+++ b/src/whereexpr.c
@@ -1121,9 +1121,15 @@ static void exprAnalyze(
       }
     }else if( (prereqAll>>1)>=x ){
       /* The ON clause of an INNER JOIN references a table to its right.
-      ** Most other SQL database engines raise an error.  But all versions
-      ** of SQLite going back to 3.0.0 have just put the ON clause constraint
-      ** into the WHERE clause and carried on. */
+      ** Most other SQL database engines raise an error.  But SQLite versions
+      ** 3.0 through 3.38 just put the ON clause constraint into the WHERE
+      ** clause and carried on.   Beginning with 3.39, raise an error only
+      ** if there is a RIGHT or LEFT JOIN in the query.  This makes SQLite
+      ** more like other systems, and also preserves legacy. */
+      if( pSrc->nSrc>0 && (pSrc->a[0].fg.jointype & JT_LTORJ)!=0 ){
+        sqlite3ErrorMsg(pParse, "ON clause references tables to its right");
+        return;
+      }
       ExprClearProperty(pExpr, EP_InnerON);
     }
   }
diff --git a/test/join8.test b/test/join8.test
index c39b773d18..46bf38aa6d 100644
--- a/test/join8.test
+++ b/test/join8.test
@@ -684,11 +684,13 @@ do_execsql_test join8-22030 {
   CREATE TABLE t3(c INTEGER PRIMARY KEY, d INT);
   CREATE INDEX t3d ON t3(d);
   INSERT INTO t3 VALUES(0, 0);
+}
+do_catchsql_test join8-22031 {
   SELECT * FROM t1 JOIN t2 ON d>b RIGHT JOIN t3 ON true WHERE +d = 0;
-} {}
-do_execsql_test join8-22040 {
+} {1 {ON clause references tables to its right}}
+do_catchsql_test join8-22040 {
   SELECT * FROM t1 JOIN t2 ON d>b RIGHT JOIN t3 ON true WHERE d = 0;
-} {}
+} {1 {ON clause references tables to its right}}
 
 
 # 2022-06-10