From f7b0b0ad5fb083c194f0f69e75747dccf71427ed Mon Sep 17 00:00:00 2001 From: dan Date: Mon, 19 Oct 2009 15:52:32 +0000 Subject: [PATCH] When generating WHERE clause terms internally for NATURAL and USING joins, identify the table by its position in the FROM list, not by its name or alias. Fix for [b73fb0bd64]. FossilOrigin-Name: 6fe6371175482d38ac4aeea994c7b20c18b7de01 --- manifest | 22 ++++++------ manifest.uuid | 2 +- src/resolve.c | 21 ++++++++++++ src/select.c | 90 ++++++++++++++++++++++--------------------------- src/sqliteInt.h | 2 +- src/update.c | 5 ++- src/where.c | 2 ++ test/join.test | 65 +++++++++++++++++++++++++++++++++++ 8 files changed, 144 insertions(+), 65 deletions(-) diff --git a/manifest b/manifest index 27abdd5650..27fc10a447 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Use\s64-bit\sarithmetic\sin\sthe\sxRead()\smethod\sof\sasyncRead.\sFix\sfor\s[94c04eaadb]. -D 2009-10-19T07:50:26 +C When\sgenerating\sWHERE\sclause\sterms\sinternally\sfor\sNATURAL\sand\sUSING\sjoins,\sidentify\sthe\stable\sby\sits\sposition\sin\sthe\sFROM\slist,\snot\sby\sits\sname\sor\salias.\sFix\sfor\s[b73fb0bd64]. +D 2009-10-19T15:52:33 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 4ca3f1dd6efa2075bcb27f4dc43eef749877740d F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -158,13 +158,13 @@ F src/pragma.c c25d0d15dd0bbc5ec34e9760629353358705a447 F src/prepare.c 665d52303135833c53b9be03e68533e249e1de54 F src/printf.c 508a1c59433353552b6553cba175eaa7331f8fc1 F src/random.c 676b9d7ac820fe81e6fb2394ac8c10cff7f38628 -F src/resolve.c 941843301f6fda6c6350839c6955a172441a0782 +F src/resolve.c 3ac31c7181fab03732125fdedf7c2091a5c07f1b F src/rowset.c c64dafba1f9fd876836c8db8682966b9d197eb1f -F src/select.c 1d0a13137532321b4364f964e46f057d271691e3 +F src/select.c cbe366a0ce114856e66f5daf0f848d7c48a88298 F src/shell.c 270231b3f587f1f86391b9994fdfcd5d472c3fdf F src/sqlite.h.in 02edbab15c4556dc562f6f6b9b9d322e7719c800 F src/sqlite3ext.h 1db7d63ab5de4b3e6b83dd03d1a4e64fef6d2a17 -F src/sqliteInt.h cd893f92cc5605943ccf6ac46b46713d5f571e1e +F src/sqliteInt.h 3b00a3ce79e60c5a47c342b738c8b75013f3ec84 F src/sqliteLimit.h 38b2fffcd01faeaeaadea71b2b47695a81580c8b F src/status.c 237b193efae0cf6ac3f0817a208de6c6c6ef6d76 F src/table.c cc86ad3d6ad54df7c63a3e807b5783c90411a08d @@ -202,7 +202,7 @@ F src/test_thread.c b8a1ab7ca1a632f18e8a361880d5d65eeea08eac F src/test_wsd.c 3ae5101de6cbfda2720152ab659ea84079719241 F src/tokenize.c af8a56e6a50c5042fc305bfa796275e9bf26ff2b F src/trigger.c 2053afa9952f69cf451bc0e6ea88072701f2925e -F src/update.c 2c8a64237e4fae604468d14380b877d169545b63 +F src/update.c 8e8535f66c32d946199cb1caad19646a97ead3a7 F src/utf.c 99cf927eabb104621ba889ac0dd075fc1657ad30 F src/util.c 59d4e9456bf1fe581f415a783fa0cee6115c8f35 F src/vacuum.c f2347520907ee4ec867c9b804d24456b0fd912a7 @@ -215,7 +215,7 @@ F src/vdbeblob.c 9bfaeab22e261a6a7b6df04e7faaf7d6dfdbef5a F src/vdbemem.c 7055a2941a7802094f4704cedc7a28cc88a23749 F src/vtab.c 3e54fe39374e5feb8b174de32a90e7a21966025d F src/walker.c 1edca756275f158b80f20eb6f104c8d3fcc96a04 -F src/where.c 896ec4cd8c5f8d065d793abcf126856ddb6968b4 +F src/where.c 9e8bb75c39adfd85fa0bbfa803836de9485716a8 F test/aggerror.test a867e273ef9e3d7919f03ef4f0e8c0d2767944f2 F test/alias.test 4529fbc152f190268a15f9384a5651bbbabc9d87 F test/all.test 14165b3e32715b700b5f0cbf8f6e3833dda0be45 @@ -430,7 +430,7 @@ F test/ioerr2.test 1b56cb80d5b0726ee3ba325ca175734541e32955 F test/ioerr3.test d3cec5e1a11ad6d27527d0d38573fbff14c71bdd F test/ioerr4.test fc6eddfec2efc2f1ed217b9eae4c1c1d3516ce86 F test/ioerr5.test 89f69b09a6b5d4f5bbfe58d4231f28236d842dcb -F test/join.test a79084b09d862e7be924abb97d1b1342a0157209 +F test/join.test 8d63cc4d230a7affafa4b6ab0b97c49b8ccb365c F test/join2.test f2171c265e57ee298a27e57e7051d22962f9f324 F test/join3.test 6f0c774ff1ba0489e6c88a3e77b9d3528fb4fda0 F test/join4.test 1a352e4e267114444c29266ce79e941af5885916 @@ -760,7 +760,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff x F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f x -P 8a21fdaf6a89f74b040ea0c6bb996ac1c6fcd369 -R aa642d0be1b406a438b276a4ecef679d +P ca3e41b0574cfd8d971c2be2114e58273a531970 +R cd248a323e6fa543f7b6dc5498e63431 U dan -Z 90329c1ce6ebc56353157655ea929310 +Z ef39b99c6d9e839e5c682ef365e81b03 diff --git a/manifest.uuid b/manifest.uuid index 39481d2caa..a20f681b5c 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -ca3e41b0574cfd8d971c2be2114e58273a531970 \ No newline at end of file +6fe6371175482d38ac4aeea994c7b20c18b7de01 \ No newline at end of file diff --git a/src/resolve.c b/src/resolve.c index 4090bdd6ec..40aab3fac0 100644 --- a/src/resolve.c +++ b/src/resolve.c @@ -394,6 +394,27 @@ lookupname_end: } } +/* +** Allocate and return a pointer to an expression to load the column iCol +** from datasource iSrc datasource in SrcList pSrc. +*/ +Expr *sqlite3CreateColumnExpr(sqlite3 *db, SrcList *pSrc, int iSrc, int iCol){ + Expr *p = sqlite3ExprAlloc(db, TK_COLUMN, 0, 0); + if( p ){ + struct SrcList_item *pItem = &pSrc->a[iSrc]; + p->pTab = pItem->pTab; + p->iTable = pItem->iCursor; + if( p->pTab->iPKey==iCol ){ + p->iColumn = -1; + }else{ + p->iColumn = iCol; + pItem->colUsed |= ((Bitmask)1)<<(iCol>=BMS ? BMS-1 : iCol); + } + ExprSetProperty(p, EP_Resolved); + } + return p; +} + /* ** This routine is callback for sqlite3WalkExpr(). ** diff --git a/src/select.c b/src/select.c index f470befafe..0f6a2b251e 100644 --- a/src/select.c +++ b/src/select.c @@ -192,51 +192,45 @@ static int columnIndex(Table *pTab, const char *zCol){ } /* -** Create an expression node for an identifier with the name of zName -*/ -Expr *sqlite3CreateIdExpr(Parse *pParse, const char *zName){ - return sqlite3Expr(pParse->db, TK_ID, zName); -} - -/* -** Add a term to the WHERE expression in *ppExpr that requires the -** zCol column to be equal in the two tables pTab1 and pTab2. +** This function is used to add terms implied by JOIN syntax to the +** WHERE clause expression of a SELECT statement. The new term, which +** is ANDed with the existing WHERE clause, is of the form: +** +** (tab1.col1 = tab2.col2) +** +** where tab1 is the iSrc'th table in SrcList pSrc and tab2 is the +** (iSrc+1)'th. Column col1 is column iColLeft of tab1, and col2 is +** column iColRight of tab2. */ static void addWhereTerm( - Parse *pParse, /* Parsing context */ - const char *zCol, /* Name of the column */ - const Table *pTab1, /* First table */ - const char *zAlias1, /* Alias for first table. May be NULL */ - const Table *pTab2, /* Second table */ - const char *zAlias2, /* Alias for second table. May be NULL */ - int iRightJoinTable, /* VDBE cursor for the right table */ - Expr **ppExpr, /* Add the equality term to this expression */ - int isOuterJoin /* True if dealing with an OUTER join */ + Parse *pParse, /* Parsing context */ + SrcList *pSrc, /* List of tables in FROM clause */ + int iSrc, /* Index of first table to join in pSrc */ + int iColLeft, /* Index of column in first table */ + int iColRight, /* Index of column in second table */ + int isOuterJoin, /* True if this is an OUTER join */ + Expr **ppWhere /* IN/OUT: The WHERE clause to add to */ ){ - Expr *pE1a, *pE1b, *pE1c; - Expr *pE2a, *pE2b, *pE2c; - Expr *pE; + sqlite3 *db = pParse->db; + Expr *pE1; + Expr *pE2; + Expr *pEq; - pE1a = sqlite3CreateIdExpr(pParse, zCol); - pE2a = sqlite3CreateIdExpr(pParse, zCol); - if( zAlias1==0 ){ - zAlias1 = pTab1->zName; + assert( pSrc->nSrc>(iSrc+1) ); + assert( pSrc->a[iSrc].pTab ); + assert( pSrc->a[iSrc+1].pTab ); + + pE1 = sqlite3CreateColumnExpr(db, pSrc, iSrc, iColLeft); + pE2 = sqlite3CreateColumnExpr(db, pSrc, iSrc+1, iColRight); + + pEq = sqlite3PExpr(pParse, TK_EQ, pE1, pE2, 0); + if( pEq && isOuterJoin ){ + ExprSetProperty(pEq, EP_FromJoin); + assert( !ExprHasAnyProperty(pEq, EP_TokenOnly|EP_Reduced) ); + ExprSetIrreducible(pEq); + pEq->iRightJoinTable = (i16)pE2->iTable; } - pE1b = sqlite3CreateIdExpr(pParse, zAlias1); - if( zAlias2==0 ){ - zAlias2 = pTab2->zName; - } - pE2b = sqlite3CreateIdExpr(pParse, zAlias2); - pE1c = sqlite3PExpr(pParse, TK_DOT, pE1b, pE1a, 0); - pE2c = sqlite3PExpr(pParse, TK_DOT, pE2b, pE2a, 0); - pE = sqlite3PExpr(pParse, TK_EQ, pE1c, pE2c, 0); - if( pE && isOuterJoin ){ - ExprSetProperty(pE, EP_FromJoin); - assert( !ExprHasAnyProperty(pE, EP_TokenOnly|EP_Reduced) ); - ExprSetIrreducible(pE); - pE->iRightJoinTable = (i16)iRightJoinTable; - } - *ppExpr = sqlite3ExprAnd(pParse->db,*ppExpr, pE); + *ppWhere = sqlite3ExprAnd(db, *ppWhere, pEq); } /* @@ -318,11 +312,9 @@ static int sqliteProcessJoin(Parse *pParse, Select *p){ } for(j=0; jnCol; j++){ char *zName = pLeftTab->aCol[j].zName; - if( columnIndex(pRightTab, zName)>=0 ){ - addWhereTerm(pParse, zName, pLeftTab, pLeft->zAlias, - pRightTab, pRight->zAlias, - pRight->iCursor, &p->pWhere, isOuter); - + int iRightCol = columnIndex(pRightTab, zName); + if( iRightCol>=0 ){ + addWhereTerm(pParse, pSrc, i, j, iRightCol, isOuter, &p->pWhere); } } } @@ -355,14 +347,14 @@ static int sqliteProcessJoin(Parse *pParse, Select *p){ IdList *pList = pRight->pUsing; for(j=0; jnId; j++){ char *zName = pList->a[j].zName; - if( columnIndex(pLeftTab, zName)<0 || columnIndex(pRightTab, zName)<0 ){ + int iLeftCol = columnIndex(pLeftTab, zName); + int iRightCol = columnIndex(pRightTab, zName); + if( iLeftCol<0 || iRightCol<0 ){ sqlite3ErrorMsg(pParse, "cannot join using column %s - column " "not present in both tables", zName); return 1; } - addWhereTerm(pParse, zName, pLeftTab, pLeft->zAlias, - pRightTab, pRight->zAlias, - pRight->iCursor, &p->pWhere, isOuter); + addWhereTerm(pParse, pSrc, i, iLeftCol, iRightCol, isOuter, &p->pWhere); } } } diff --git a/src/sqliteInt.h b/src/sqliteInt.h index c2c947d12d..49cf77c23e 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -2643,7 +2643,6 @@ int sqlite3ExprCompare(Expr*, Expr*); void sqlite3ExprAnalyzeAggregates(NameContext*, Expr*); void sqlite3ExprAnalyzeAggList(NameContext*,ExprList*); Vdbe *sqlite3GetVdbe(Parse*); -Expr *sqlite3CreateIdExpr(Parse *, const char*); void sqlite3PrngSaveState(void); void sqlite3PrngRestoreState(void); void sqlite3PrngResetState(void); @@ -2877,6 +2876,7 @@ void sqlite3StrAccumAppend(StrAccum*,const char*,int); char *sqlite3StrAccumFinish(StrAccum*); void sqlite3StrAccumReset(StrAccum*); void sqlite3SelectDestInit(SelectDest*,int,int); +Expr *sqlite3CreateColumnExpr(sqlite3 *, SrcList *, int, int); void sqlite3BackupRestart(sqlite3_backup *); void sqlite3BackupUpdate(sqlite3_backup *, Pgno, const u8 *); diff --git a/src/update.c b/src/update.c index 3703c1b59c..3f5e15ff4f 100644 --- a/src/update.c +++ b/src/update.c @@ -579,8 +579,7 @@ static void updateVirtualTable( /* Construct the SELECT statement that will find the new values for ** all updated rows. */ - pEList = sqlite3ExprListAppend(pParse, 0, - sqlite3CreateIdExpr(pParse, "_rowid_")); + pEList = sqlite3ExprListAppend(pParse, 0, sqlite3Expr(db, TK_ID, "_rowid_")); if( pRowid ){ pEList = sqlite3ExprListAppend(pParse, pEList, sqlite3ExprDup(db, pRowid, 0)); @@ -590,7 +589,7 @@ static void updateVirtualTable( if( aXRef[i]>=0 ){ pExpr = sqlite3ExprDup(db, pChanges->a[aXRef[i]].pExpr, 0); }else{ - pExpr = sqlite3CreateIdExpr(pParse, pTab->aCol[i].zName); + pExpr = sqlite3Expr(db, TK_ID, pTab->aCol[i].zName); } pEList = sqlite3ExprListAppend(pParse, pEList, pExpr); } diff --git a/src/where.c b/src/where.c index b4ad2ccd69..53ec26b841 100644 --- a/src/where.c +++ b/src/where.c @@ -2034,6 +2034,7 @@ static int whereRangeRegion( ** ** If an error occurs, return an error code. Otherwise, SQLITE_OK. */ +#ifdef SQLITE_ENABLE_STAT2 static int valueFromExpr( Parse *pParse, Expr *pExpr, @@ -2050,6 +2051,7 @@ static int valueFromExpr( } return sqlite3ValueFromExpr(pParse->db, pExpr, SQLITE_UTF8, aff, pp); } +#endif /* ** This function is used to estimate the number of rows that will be visited diff --git a/test/join.test b/test/join.test index 7fecbe876e..88ac04f950 100644 --- a/test/join.test +++ b/test/join.test @@ -576,4 +576,69 @@ ifcapable subquery { } ;# ifcapable subquery +#------------------------------------------------------------------------- +# The following tests are to ensure that bug b73fb0bd64 is fixed. +# +do_test join-11.1 { + drop_all_tables + execsql { + CREATE TABLE t1(a INTEGER PRIMARY KEY, b TEXT); + CREATE TABLE t2(a INTEGER PRIMARY KEY, b TEXT); + INSERT INTO t1 VALUES(1,'abc'); + INSERT INTO t1 VALUES(2,'def'); + INSERT INTO t2 VALUES(1,'abc'); + INSERT INTO t2 VALUES(2,'def'); + SELECT * FROM t1 NATURAL JOIN t2; + } +} {1 abc 2 def} + +do_test join-11.2 { + execsql { SELECT a FROM t1 JOIN t1 USING (a)} +} {1 2} +do_test join-11.3 { + execsql { SELECT a FROM t1 JOIN t1 AS t2 USING (a)} +} {1 2} +do_test join-11.3 { + execsql { SELECT * FROM t1 NATURAL JOIN t1 AS t2} +} {1 abc 2 def} +do_test join-11.4 { + execsql { SELECT * FROM t1 NATURAL JOIN t1 } +} {1 abc 2 def} + +do_test join-11.5 { + drop_all_tables + execsql { + CREATE TABLE t1(a COLLATE nocase, b); + CREATE TABLE t2(a, b); + INSERT INTO t1 VALUES('ONE', 1); + INSERT INTO t1 VALUES('two', 2); + INSERT INTO t2 VALUES('one', 1); + INSERT INTO t2 VALUES('two', 2); + } +} {} +do_test join-11.6 { + execsql { SELECT * FROM t1 NATURAL JOIN t2 } +} {ONE 1 two 2} +do_test join-11.7 { + execsql { SELECT * FROM t2 NATURAL JOIN t1 } +} {two 2} + +do_test join-11.8 { + drop_all_tables + execsql { + CREATE TABLE t1(a, b TEXT); + CREATE TABLE t2(b INTEGER, a); + INSERT INTO t1 VALUES('one', '1.0'); + INSERT INTO t1 VALUES('two', '2'); + INSERT INTO t2 VALUES(1, 'one'); + INSERT INTO t2 VALUES(2, 'two'); + } +} {} +do_test join-11.9 { + execsql { SELECT * FROM t1 NATURAL JOIN t2 } +} {one 1.0 two 2} +do_test join-11.10 { + execsql { SELECT * FROM t2 NATURAL JOIN t1 } +} {1 one 2 two} + finish_test