From: dan Date: Fri, 22 Aug 2025 16:25:34 +0000 (+0000) Subject: Attempt to detect errors cause by ON clauses that refer to tables to the right of... X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=54c96b21525c1374e6aba8a2b0f527c32feb1a13;p=thirdparty%2Fsqlite.git Attempt to detect errors cause by ON clauses that refer to tables to the right of themselves while resolving names, instead of later on after query-flattening and other operations have complicated things. FossilOrigin-Name: 4bb527d3377f9e0e328c5e9901e5e6d45a90958004db5ff7000baa2ecb3ffeb6 --- diff --git a/manifest b/manifest index 73c5ed4732..2d9014fd79 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sa\sminor\stypo\sin\sa\scomment. -D 2025-08-21T18:47:01.547 +C Attempt\sto\sdetect\serrors\scause\sby\sON\sclauses\sthat\srefer\sto\stables\sto\sthe\sright\sof\sthemselves\swhile\sresolving\snames,\sinstead\sof\slater\son\safter\squery-flattening\sand\sother\soperations\shave\scomplicated\sthings. +D 2025-08-22T16:25:34.129 F .fossil-settings/binary-glob 61195414528fb3ea9693577e1980230d78a1f8b0a54c78cf1b9b24d0a409ed6a x F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea @@ -739,14 +739,14 @@ F src/pragma.c 30b535d0a66348df844ee36f890617b4cf45e9a22dcbc47ec3ca92909c50aaf1 F src/prepare.c 2af0b5c1ec787c8eebd21baa9d79caf4a4dc3a18e76ce2edbf2027d706bca37a F src/printf.c 5f0c957af9699e849d786e8fbaa3baab648ca5612230dc17916434c14bc8698f F src/random.c 606b00941a1d7dd09c381d3279a058d771f406c5213c9932bbd93d5587be4b9c -F src/resolve.c f8d1d011aba0964ff1bdccd049d4d2c2fec217efd90d202a4bb775e926b2c25d +F src/resolve.c f400b2c1fa22ffe779901fba13deace5e483e2724addebaa9242dd1cedc8a56b F src/rowset.c 8432130e6c344b3401a8874c3cb49fefe6873fec593294de077afea2dce5ec97 F src/select.c 78ebf432355e820962a5001277cb43ffe3d82441c6dc9c8f0aeb0b15fbd5dd02 F src/shell.c.in 0636915df0dbac6c780f04959f5d1055f206fb281b2c8fc8b113fe7bfc7d44ef F src/sqlite.h.in ebfc0358de0e18aabee7fa918f2f846894e23bebc74160fbe265c99046ee61b8 F src/sqlite3.rc 015537e6ac1eec6c7050e17b616c2ffe6f70fca241835a84a4f0d5937383c479 F src/sqlite3ext.h 0bfd049bb2088cc44c2ad54f2079d1c6e43091a4e1ce8868779b75f6c1484f1e -F src/sqliteInt.h a54f83985985655d1276e9e356dd6ae19b8d0b62c2abc75cc9e8f402ea141207 +F src/sqliteInt.h b527df450e1b0b7c64a395129fc4fd36684ce81039d22c78c4ed718ca11d0c14 F src/sqliteLimit.h fe70bd8983e5d317a264f2ea97473b359faf3ebb0827877a76813f5cf0cdc364 F src/status.c 0e72e4f6be6ccfde2488eb63210297e75f569f3ce9920f6c3d77590ec6ce5ffd F src/table.c 0f141b58a16de7e2fbe81c308379e7279f4c6b50eb08efeec5892794a0ba30d1 @@ -826,7 +826,7 @@ F src/walker.c d5006d6b005e4ea7302ad390957a8d41ed83faa177e412f89bc5600a7462a014 F src/where.c f2f075bd17065922235632feb368efe92a7f03d42797eb575267574fbf6d4218 F src/whereInt.h 8d94cb116c9e06205c3d5ac87af065fc044f8cf08bfdccd94b6ea1c1308e65da F src/wherecode.c 71c5c6804b7f882dec8ec858758accae02fcfca13df3cc720f1f258e663ec7c5 -F src/whereexpr.c 78c28a8da187816d5d82049f2e343fb39f4a8e30b5bf1bda9b96cecde40ca8bd +F src/whereexpr.c 403a44eeec1a0f0914fccc6a59376b6924bc00ef6728fe6ffce4cf3051b320fc F src/window.c d01227141f622f24fbe36ca105fbe6ef023f9fd98f1ccd65da95f88886565db5 F test/8_3_names.test ebbb5cd36741350040fd28b432ceadf495be25b2 F test/affinity2.test 4d7a34d328e58ca2a2d78fd76c27614a41ca7ddf4312ded9c68c04f430b3b47d @@ -1335,6 +1335,7 @@ F test/joinD.test 2ce62e7353a0702ca5e70008faf319c1d4686aa19fba34275c6d1da0e960be F test/joinE.test d5d182f3812771e2c0d97c9dcf5dbe4c41c8e21c82560e59358731c4a3981d6b F test/joinF.test 53dd66158806823ea680dd7543b5406af151b5aafa5cd06a7f3231cd94938127 F test/joinH.test fd76024ff104baec16417db5cafc0894ad4e0863e70803e63c1bba0322706339 +F test/joinI.test 78cae6f746a78f32d8aedb8cf7b93bcd802c245db0986330844dfb1a6700ebab F test/journal1.test bc61a4228db11bffca118bd358ba4b868524bf080f3532749de6c539656e20fa F test/journal2.test 9dac6b4ba0ca79c3b21446bbae993a462c2397c4 F test/journal3.test e5aeff93a7776cf644dbc48dec277655cff80a1cd24689036abc87869b120ea6 @@ -2169,8 +2170,11 @@ F tool/version-info.c 3b36468a90faf1bbd59c65fd0eb66522d9f941eedd364fabccd7227350 F tool/warnings-clang.sh bbf6a1e685e534c92ec2bfba5b1745f34fb6f0bc2a362850723a9ee87c1b31a7 F tool/warnings.sh 1ad0169b022b280bcaaf94a7fa231591be96b514230ab5c98fbf15cd7df842dd F tool/win/sqlite.vsix deb315d026cc8400325c5863eef847784a219a2f -P 9f15182776b30676c9aae9bcb5d4ad7580359fbdd607c2a9227c9cf2c81a4054 -R e87d0e2ec7db35078d5f7c68106ae850 -U drh -Z a3b0bda8d54e990d1d0f3efd38f275ec +P 9ada44eb6d26532e45cdd2ed8d5707f1734d0177a13b493ff9cf070e0a992522 +R 6308fbfdbe1bb96ade6fc59905bcaa05 +T *branch * on-clause-error-fix +T *sym-on-clause-error-fix * +T -sym-trunk * +U dan +Z 5f576e89fa32a605a15ca63be19fcc55 # Remove this line to create a well-formed Fossil manifest. diff --git a/manifest.uuid b/manifest.uuid index 565294e9a4..c4296e1645 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -9ada44eb6d26532e45cdd2ed8d5707f1734d0177a13b493ff9cf070e0a992522 +4bb527d3377f9e0e328c5e9901e5e6d45a90958004db5ff7000baa2ecb3ffeb6 diff --git a/src/resolve.c b/src/resolve.c index 00d8239401..0ea125e58d 100644 --- a/src/resolve.c +++ b/src/resolve.c @@ -697,7 +697,12 @@ static int lookupName( /* Advance to the next name context. The loop will exit when either ** we have a match (cnt>0) or when we run out of name contexts. */ - if( cnt ) break; + if( cnt ){ + if( pNC->pOn && cnt==1 && pNC->pOn->w.iJoiniTable ){ + sqlite3ErrorMsg(pParse, "ON clause references tables to its right"); + } + break; + } pNC = pNC->pNext; nSubquery++; }while( pNC ); @@ -1454,6 +1459,53 @@ static int resolveExprStep(Walker *pWalker, Expr *pExpr){ return pParse->nErr ? WRC_Abort : WRC_Continue; } +/* +** True if the SrcList passed as the only argument contains a RIGHT JOIN. +*/ +#define hasRightJoin(pSrc) (((pSrc)->a[0].fg.jointype && JT_LTORJ)!=0) + +/* +** Like resolveExprStep(), except this version does some special handling +** of ON expressions to ensure they do not illegally refer to tables +** to the right of them within the FROM clause. +*/ +static int resolveOnStep(Walker *pWalker, Expr *pExpr){ + NameContext *pNC = pWalker->u.pNC; + int res; + + /* SQLite raises an error if an ON clause refers to tables to the right + ** of itself in its join if either: + ** + ** + the join the ON is attached to is an OUTER JOIN, or + ** + the FROM clause in question features at least one FULL or RIGHT join. + ** + ** SQLite versions 3.0-3.38 treated a ON clause on an INNER JOIN as + ** equivalent to part of the WHERE clause, and so it was not an error for + ** them refer to tables to the right of themselves. Starting with 3.39, + ** SQLite will raise an error in the second case enumerated above as well. + ** This makes SQLite more like other systems while preserving legacy + ** behavior. + ** + ** So, if this expression is an ON clause and either of the above are true, + ** then set NameContext.pOn to point to the expression before resolving + ** its descendants. lookupName() will generate an error if any descendant + ** expression refers to a table to the right of this ON clause in the current + ** query. */ + if( pNC->pOn==0 && ( + (ExprHasProperty(pExpr, EP_OuterON)) + || (ExprHasProperty(pExpr, EP_InnerON) && hasRightJoin(pNC->pSrcList)) + )){ + pNC->pOn = pExpr; + res = sqlite3WalkExprNN(pWalker, pExpr); + if( res==WRC_Continue ) res = WRC_Prune; + pNC->pOn = 0; + }else{ + res = resolveExprStep(pWalker, pExpr); + } + + return res; +} + /* ** pEList is a list of expressions which are really the result set of the ** a SELECT statement. pE is a term in an ORDER BY or GROUP BY clause. @@ -1936,6 +1988,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ pItem->fg.isCorrelated = (pOuterNC->nRef>nRef); } } + if( pItem->fg.isOn ) sNC.ncFlags |= NC_On; } if( pOuterNC && ALWAYS(pOuterNC->nNestedSelect>0) ){ pOuterNC->nNestedSelect--; @@ -1944,7 +1997,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ /* Set up the local name-context to pass to sqlite3ResolveExprNames() to ** resolve the result-set expression list. */ - sNC.ncFlags = NC_AllowAgg|NC_AllowWin; + sNC.ncFlags |= (NC_AllowAgg|NC_AllowWin); sNC.pSrcList = p->pSrc; sNC.pNext = pOuterNC; @@ -1985,7 +2038,7 @@ static int resolveSelectStep(Walker *pWalker, Select *p){ } sNC.ncFlags |= NC_Where; if( sqlite3ResolveExprNames(&sNC, p->pWhere) ) return WRC_Abort; - sNC.ncFlags &= ~NC_Where; + sNC.ncFlags &= ~(NC_Where); /* Resolve names in table-valued-function arguments */ for(i=0; ipSrc->nSrc; i++){ @@ -2151,7 +2204,11 @@ int sqlite3ResolveExprNames( savedHasAgg = pNC->ncFlags & (NC_HasAgg|NC_MinMaxAgg|NC_HasWin|NC_OrderAgg); pNC->ncFlags &= ~(NC_HasAgg|NC_MinMaxAgg|NC_HasWin|NC_OrderAgg); w.pParse = pNC->pParse; - w.xExprCallback = resolveExprStep; + if( (pNC->ncFlags & (NC_On|NC_Where))==(NC_On|NC_Where) ){ + w.xExprCallback = resolveOnStep; + }else{ + w.xExprCallback = resolveExprStep; + } w.xSelectCallback = (pNC->ncFlags & NC_NoSelect) ? 0 : resolveSelectStep; w.xSelectCallback2 = 0; w.u.pNC = pNC; diff --git a/src/sqliteInt.h b/src/sqliteInt.h index a09c94ae62..a23f60f912 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -3496,6 +3496,7 @@ struct SrcList { struct NameContext { Parse *pParse; /* The parser */ SrcList *pSrcList; /* One or more tables used to resolve names */ + Expr *pOn; /* Currently inside this ON(...) clause */ union { ExprList *pEList; /* Optional list of result-set columns */ AggInfo *pAggInfo; /* Information about aggregates at this level */ @@ -3541,6 +3542,7 @@ struct NameContext { #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_On 0x200000 /* Requires EP_OuterOn|EP_InnerOn processing */ #define NC_OrderAgg 0x8000000 /* Has an aggregate other than count/min/max */ /* diff --git a/src/whereexpr.c b/src/whereexpr.c index 0d17b0d752..0d99ca85e6 100644 --- a/src/whereexpr.c +++ b/src/whereexpr.c @@ -1170,21 +1170,7 @@ static void exprAnalyze( prereqAll |= x; extraRight = x-1; /* ON clause terms may not be used with an index ** on left table of a LEFT JOIN. Ticket #3015 */ - if( (prereqAll>>1)>=x ){ - sqlite3ErrorMsg(pParse, "ON clause references tables to its right"); - return; - } }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 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 FULL JOIN in the query. This makes SQLite - ** more like other systems, and also preserves legacy. */ - if( ALWAYS(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/joinI.test b/test/joinI.test new file mode 100644 index 0000000000..a422f19470 --- /dev/null +++ b/test/joinI.test @@ -0,0 +1,96 @@ +# 2022 May 17 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#*********************************************************************** +# This file implements regression tests for SQLite library. +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +set testprefix joinI + +do_execsql_test 1.0 { + CREATE TABLE t1(a INT); + CREATE TABLE t2(b INT); + CREATE TABLE t3(c INT); +} + +foreach {tn sql} { + 1 "SELECT * FROM t1 RIGHT JOIN t2 ON t2.b=t3.c CROSS JOIN t3" + 2 "SELECT * FROM t1 RIGHT JOIN t2 ON t2.b=(SELECT t3.c) CROSS JOIN t3" + 3 "SELECT * FROM t1 RIGHT JOIN t2 ON CASE WHEN t2.b THEN t3.c ELSE 1 END CROSS JOIN t3" +} { + do_catchsql_test 1.1.$tn $sql {1 {ON clause references tables to its right}} +} + + +#------------------------------------------------------------------------- +reset_db + +do_execsql_test 2.0 { + CREATE TABLE t0(c0 INT, c1 INT); + CREATE TABLE t1 (c0 INT); + + CREATE VIEW v1(c0) AS SELECT t0.c0 FROM t0 NATURAL RIGHT JOIN t1; + CREATE VIEW v2(c0) AS SELECT 0 FROM v1; + + INSERT INTO t0(c0, c1) VALUES (-1, 0); + INSERT INTO t1(c0) VALUES (NULL); + + SELECT * FROM v1 INNER JOIN (v2 CROSS JOIN t0) ON (t0.c0 < t0.c1); +} {{} 0 -1 0} + +#------------------------------------------------------------------------- +reset_db +do_execsql_test 3.0 { + CREATE TABLE t0(c0, c1); + CREATE TABLE t1(v); + CREATE TABLE t2(w); + CREATE TABLE t3(x); + CREATE TABLE t4(y); + CREATE TABLE t5(z); +} + +do_execsql_test 3.1 { + SELECT 1234 FROM t4 + RIGHT JOIN t5 + CROSS JOIN (t2 CROSS JOIN t0) AS a1 ON (a1.c0 < a1.c1); +} + +do_execsql_test 3.2 { + SELECT 1234 FROM t4 + RIGHT JOIN t5 + CROSS JOIN (t2 CROSS JOIN t1 CROSS JOIN t0) AS a1 ON (a1.c0 < a1.c1); +} + +do_execsql_test 3.3 { + SELECT 5678 FROM t0 RIGHT JOIN t1 ON ( + SELECT 1 FROM t2 RIGHT JOIN t3 ON t2.w + ) CROSS JOIN t4; +} + +do_catchsql_test 3.4 { + SELECT 5678 FROM t0 RIGHT JOIN t1 ON ( + SELECT 1 FROM t2 RIGHT JOIN t3 ON t4.y + ) CROSS JOIN t4; +} {1 {ON clause references tables to its right}} + +do_execsql_test 3.5 { + SELECT 5678 FROM t0 RIGHT JOIN t1 ON ( + SELECT 1 FROM t2 RIGHT JOIN t3 ON 0=t1.v + ) CROSS JOIN t4; +} + +do_catchsql_test 3.6 { + SELECT 5678 FROM t0 RIGHT JOIN t1 ON ( + SELECT 1 FROM t2 RIGHT JOIN t3 ON max(0,t5.z) CROSS JOIN t5 + ) CROSS JOIN t4; +} {1 {ON clause references tables to its right}} + +finish_test