diff --git a/manifest b/manifest index 5042d05f59..155b9f9da3 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Minor\scleanup\schanges\son\sthe\sOP_StackDepth\sopcode.\s\sAdded\sthe\nsidedelete\stest\sfor\sadditional\stesting\sof\sticket\s#2832.\s(CVS\s4619) -D 2007-12-12T22:24:13 +C Towards\sgetting\sORDER\sBY\sto\smatch\sagainst\sthe\scorrectin\scolumns.\nThis\sversion\sonly\slooks\sat\sthe\sleft-most\scolumn\sin\sa\scompound\sSELECT.\nThat\sis\sthe\scorrect\sthing\sto\sdo,\sbut\snot\swhat\sSQLite\shas\shistorically\sdone.\s(CVS\s4620) +D 2007-12-13T02:45:31 F Makefile.arm-wince-mingw32ce-gcc ac5f7b2cef0cd850d6f755ba6ee4ab961b1fadf7 F Makefile.in 0590398f62fc2c456ff4c45e9741f5a718b7e2ac F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -129,9 +129,9 @@ F src/pager.h f504f7ae84060fee0416a853e368d3d113c3d6fa F src/parse.y a780b33ef45dd7b3272319cf91e609d6f109a31c F src/pragma.c 0246032dbe681dded8710ac43eaf654eead1434e F src/prepare.c f811fdb6fd4a82cca673a6e1d5b041d6caf567f1 -F src/printf.c c94a2571a828b927c64f5e3ed3584da8a91fcaec +F src/printf.c 5732e393c45be7c09bfca9a786daef017e0066ef F src/random.c 4a22746501bf36b0a088c66e38dde5daba6a35da -F src/select.c b7408624b55f58e5aa8521edb177b26ad72f968b +F src/select.c 63cc67c9a9cc3f32ec2205d2c769d94cd2fc6f60 F src/server.c 087b92a39d883e3fa113cae259d64e4c7438bc96 F src/shell.c c97be281cfc3dcb14902f45e4b16f20038eb83ff F src/sqlite.h.in b16a7127dad4a3e5b1b26b3d64241f3373aa12ea @@ -389,7 +389,7 @@ F test/misc6.test 953cc693924d88e6117aeba16f46f0bf5abede91 F test/misc7.test 3fbd0a9e3dd03331d9d76acd47bc179e1a97e15e F test/misuse.test 30b3a458e5a70c31e74c291937b6c82204c59f33 F test/notnull.test 44d600f916b770def8b095a9962dbe3be5a70d82 -F test/null.test 9503e1f63e959544c006d9f01709c5b5eab67d54 +F test/null.test 93e48033673841240f137fd621317cc73b69eeb8 F test/onefile.test b9cce375fd2a41ee3afa79a0a808954046b74458 F test/openv2.test f5dd6b23e4dce828eb211649b600763c42a668df F test/pager.test 60303481b22b240c18d6dd1b64edcecc2f4b5a97 @@ -600,7 +600,7 @@ F www/tclsqlite.tcl 8be95ee6dba05eabcd27a9d91331c803f2ce2130 F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0 F www/version3.tcl 890248cf7b70e60c383b0e84d77d5132b3ead42b F www/whentouse.tcl fc46eae081251c3c181bd79c5faef8195d7991a5 -P 48947e2b75d39c5be0d08fe1c7b888d9065f9116 -R bb83372b6806afd80973cd2815dddbab +P c0689409320de1532be0c0cae12b4b716f6bffb9 +R 1ec374c4daffe98452f78ad79abdb48f U drh -Z ed8f32215ce7ea227bff236ab22d0da6 +Z 92df68b2adbb4d9afb7d6635a4dfe3d8 diff --git a/manifest.uuid b/manifest.uuid index f097a8031d..3a7d141948 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -c0689409320de1532be0c0cae12b4b716f6bffb9 \ No newline at end of file +bbddf16ac9539c7d48adfc73c5a90eecb8df6865 \ No newline at end of file diff --git a/src/printf.c b/src/printf.c index 2558c07c5e..ab88e23fa5 100644 --- a/src/printf.c +++ b/src/printf.c @@ -74,6 +74,7 @@ #define etSRCLIST 14 /* a pointer to a SrcList */ #define etPOINTER 15 /* The %p conversion */ #define etSQLESCAPE3 16 /* %w -> Strings with '\"' doubled */ +#define etORDINAL 17 /* %r -> 1st, 2nd, 3rd, 4th, etc. English only */ /* @@ -133,6 +134,7 @@ static const et_info fmtinfo[] = { { 'p', 16, 0, etPOINTER, 0, 1 }, { 'T', 0, 2, etTOKEN, 0, 0 }, { 'S', 0, 2, etSRCLIST, 0, 0 }, + { 'r', 10, 3, etORDINAL, 0, 0 }, }; #define etNINFO (sizeof(fmtinfo)/sizeof(fmtinfo[0])) @@ -384,6 +386,7 @@ static void vxprintf( flag_longlong = sizeof(char*)==sizeof(i64); flag_long = sizeof(char*)==sizeof(long int); /* Fall through into the next case */ + case etORDINAL: case etRADIX: if( infop->flags & FLAG_SIGNED ){ i64 v; @@ -410,6 +413,9 @@ static void vxprintf( precision = width-(prefix!=0); } bufpt = &buf[etBUFSIZE-1]; + if( xtype==etORDINAL ){ + bufpt -= 2; + } { register const char *cset; /* Use registers for speed */ register int base; @@ -420,6 +426,13 @@ static void vxprintf( longvalue = longvalue/base; }while( longvalue>0 ); } + if( xtype==etORDINAL ){ + static const char zOrd[] = "thstndrd"; + int x = buf[etBUFSIZE-4] - '0'; + if( x>=4 ) x = 0; + buf[etBUFSIZE-3] = zOrd[x*2]; + buf[etBUFSIZE-2] = zOrd[x*2+1]; + } length = &buf[etBUFSIZE-1]-bufpt; for(idx=precision-length; idx>0; idx--){ *(--bufpt) = '0'; /* Zero pad */ diff --git a/src/select.c b/src/select.c index 49273ed102..6c988f330c 100644 --- a/src/select.c +++ b/src/select.c @@ -12,7 +12,7 @@ ** This file contains C code routines that are called by the parser ** to handle SELECT statements in SQLite. ** -** $Id: select.c,v 1.365 2007/12/10 18:51:48 danielk1977 Exp $ +** $Id: select.c,v 1.366 2007/12/13 02:45:31 drh Exp $ */ #include "sqliteInt.h" @@ -1412,100 +1412,222 @@ static int prepSelectStmt(Parse *pParse, Select *p){ return rc; } -#ifndef SQLITE_OMIT_COMPOUND_SELECT /* -** This routine associates entries in an ORDER BY expression list with -** columns in a result. For each ORDER BY expression, the opcode of -** the top-level node is changed to TK_COLUMN and the iColumn value of -** the top-level node is filled in with column number and the iTable -** value of the top-level node is filled with iTable parameter. +** pE is a pointer to an expression which is a single term in +** ORDER BY or GROUP BY clause. ** -** Any entry that does not match is flagged as an error. The number -** of errors is returned. +** If pE evaluates to an integer constant i, then return i. +** This is an indication to the caller that it should sort +** by the i-th column of the result set. +** +** If pE is a well-formed expression and the SELECT statement +** is not compound, then return 0. This indicates to the +** caller that it should sort by the value of the ORDER BY +** expression. +** +** If the SELECT is compound, then attempt to match pE against +** result set columns in the left-most SELECT statement. Return +** the index i of the matching column, as an indication to the +** caller that it should sort by the i-th column. If there is +** no match, return -1 and leave an error message in pParse. */ -static int matchOrderbyToColumn( - Parse *pParse, /* A place to leave error messages */ - Select *pSelect, /* Match to result columns of this SELECT */ - ExprList *pOrderBy, /* The ORDER BY values to match against columns */ - int iTable /* Insert this value in iTable */ +static int matchOrderByTermToExprList( + Parse *pParse, /* Parsing context for error messages */ + Select *pSelect, /* The SELECT statement with the ORDER BY clause */ + Expr *pE, /* The specific ORDER BY term */ + int idx, /* When ORDER BY term is this */ + int isCompound, /* True if this is a compound SELECT */ + u8 *pHasAgg /* True if expression contains aggregate functions */ ){ - int nErr = 0; - int i, j; - sqlite3 *db = pParse->db; - int nExpr; + int i; /* Loop counter */ + ExprList *pEList; /* The columns of the result set */ + NameContext nc; /* Name context for resolving pE */ - if( pSelect==0 || pOrderBy==0 ) return 1; - if( sqlite3SelectResolve(pParse, pSelect, 0) ){ + + /* If the term is an integer constant, return the value of that + ** constant */ + pEList = pSelect->pEList; + if( sqlite3ExprIsInteger(pE, &i) ){ + if( i<=0 ){ + /* If i is too small, make it too big. That way the calling + ** function still sees a value that is out of range, but does + ** not confuse the column number with 0 or -1 result code. + */ + i = pEList->nExpr+1; + } + return i; + } + + /* If the term is a simple identifier that try to match that identifier + ** against a column name in the result set. + */ + if( pE->op==TK_ID || (pE->op==TK_STRING && pE->token.z[0]!='\'') ){ + sqlite3 *db = pParse->db; + char *zCol = sqlite3NameFromToken(db, &pE->token); + if( db->mallocFailed ){ + return -1; + } + for(i=0; inExpr; i++){ + char *zAs = pEList->a[i].zName; + if( zAs!=0 && sqlite3StrICmp(zAs, zCol)==0 ){ + sqlite3_free(zCol); + return i+1; + } + } + sqlite3_free(zCol); + } + + /* Resolve all names in the ORDER BY term expression + */ + memset(&nc, 0, sizeof(nc)); + nc.pParse = pParse; + nc.pSrcList = pSelect->pSrc; + nc.pEList = pEList; + nc.allowAgg = 1; + nc.nErr = 0; + if( sqlite3ExprResolveNames(&nc, pE) ){ + return -1; + } + if( nc.hasAgg && pHasAgg ){ + *pHasAgg = 1; + } + + /* For a compound SELECT, we need to try to match the ORDER BY + ** expression against an expression in the result set + */ + if( isCompound ){ + for(i=0; inExpr; i++){ + if( sqlite3ExprCompare(pEList->a[i].pExpr, pE) ){ + return i+1; + } + } + sqlite3ErrorMsg(pParse, "%r ORDER BY term does not match any " + "column in the result set of the left-most SELECT", idx); + return -1; + }else{ + return 0; + } +} + + +/* +** Analyze and ORDER BY or GROUP BY clause in a simple SELECT statement. +** Return the number of errors seen. +** +** Every term of the ORDER BY or GROUP BY clause needs to be an +** expression. If any expression is an integer constant, then +** that expression is replaced by the corresponding +** expression from the result set. +*/ +static int processOrderGroupBy( + Parse *pParse, /* Parsing context. Leave error messages here */ + Select *pSelect, /* The SELECT statement containing the clause */ + ExprList *pOrderBy, /* The ORDER BY or GROUP BY clause to be processed */ + int isOrder, /* 1 for ORDER BY. 0 for GROUP BY */ + u8 *pHasAgg /* Set to TRUE if any term contains an aggregate */ +){ + int i; + sqlite3 *db = pParse->db; + ExprList *pEList; + + if( pOrderBy==0 ) return 0; + if( pOrderBy->nExpr>SQLITE_MAX_COLUMN ){ + const char *zType = isOrder ? "ORDER" : "GROUP"; + sqlite3ErrorMsg(pParse, "too many terms in %s BY clause", zType); return 1; } - - nExpr = pSelect->pEList->nExpr; - for(i=0; nErr==0 && inExpr; i++){ + pEList = pSelect->pEList; + if( pEList==0 ){ + return 0; + } + for(i=0; inExpr; i++){ + int iCol; Expr *pE = pOrderBy->a[i].pExpr; - int iCol = -1; - - if( sqlite3ExprIsInteger(pE, &iCol) ){ - if( iCol<=0 || iCol>nExpr ){ - sqlite3ErrorMsg(pParse, - "ORDER BY position %d should be between 1 and %d", - iCol, nExpr); - nErr++; - break; - } - iCol--; - }else{ - Select *p; - for(p=pSelect; p; p=p->pPrior){ - ExprList *pEList = p->pEList; - Expr *pDup = sqlite3ExprDup(db, pE); - - NameContext nc; - - memset(&nc, 0, sizeof(nc)); - nc.pParse = pParse; - nc.pSrcList = p->pSrc; - nc.pEList = pEList; - nc.allowAgg = 1; - nc.nErr = 0; - if( sqlite3ExprResolveNames(&nc, pDup) ){ - sqlite3ErrorClear(pParse); - }else{ - struct ExprList_item *pItem; - for(j=0, pItem=pEList->a; jnExpr; j++, pItem++){ - if( sqlite3ExprCompare(pItem->pExpr, pDup) ){ - if( iCol>=0 && iCol!=j ){ - sqlite3ErrorMsg( - pParse, "ORDER BY term number %d is ambiguous", i+1 - ); - }else{ - iCol = j; - } - } - } - } - sqlite3ExprDelete(pDup); - } - } - + iCol = matchOrderByTermToExprList(pParse, pSelect, pE, i+1, 0, pHasAgg); if( iCol<0 ){ - sqlite3ErrorMsg(pParse, - "ORDER BY term number %d does not match any result column", i+1); - }else{ - pE->op = TK_COLUMN; - pE->iTable = iTable; - pE->iAgg = -1; - pE->iColumn = iCol; - pOrderBy->a[i].done = 1; + return 1; } - - if( pParse->nErr ){ - return pParse->nErr; + if( iCol>pEList->nExpr ){ + const char *zType = isOrder ? "ORDER" : "GROUP"; + sqlite3ErrorMsg(pParse, + "%r %s BY term out of range - should be " + "between 1 and %d", i+1, zType, pEList->nExpr); + return 1; + } + if( iCol>0 ){ + CollSeq *pColl = pE->pColl; + int flags = pE->flags & EP_ExpCollate; + sqlite3ExprDelete(pE); + pE = sqlite3ExprDup(db, pEList->a[iCol-1].pExpr); + pOrderBy->a[i].pExpr = pE; + if( pColl && flags ){ + pE->pColl = pColl; + pE->flags |= flags; + } } } - - return SQLITE_OK; + return 0; +} + +/* +** Analyze and ORDER BY or GROUP BY clause in a SELECT statement. Return +** the number of errors seen. +** +** The processing depends on whether the SELECT is simple or compound. +** For a simple SELECT statement, evry term of the ORDER BY or GROUP BY +** clause needs to be an expression. If any expression is an integer +** constant, then that expression is replaced by the corresponding +** expression from the result set. +** +** For compound SELECT statements, every expression needs to be of +** type TK_COLUMN with a iTable value as given in the 4th parameter. +** If any expression is an integer, that becomes the column number. +** Otherwise, match the expression against result set columns from +** the left-most SELECT. +*/ +static int processCompoundOrderBy( + Parse *pParse, /* Parsing context. Leave error messages here */ + Select *pSelect, /* The SELECT statement containing the ORDER BY */ + int iTable /* Output table for compound SELECT statements */ +){ + int i; + ExprList *pOrderBy; + ExprList *pEList; + + pOrderBy = pSelect->pOrderBy; + if( pOrderBy==0 ) return 0; + if( pOrderBy->nExpr>SQLITE_MAX_COLUMN ){ + sqlite3ErrorMsg(pParse, "too many terms in ORDER BY clause"); + return 1; + } + while( pSelect->pPrior ){ + pSelect = pSelect->pPrior; + } + pEList = pSelect->pEList; + if( pEList==0 ){ + return 1; + } + for(i=0; inExpr; i++){ + int iCol; + Expr *pE = pOrderBy->a[i].pExpr; + iCol = matchOrderByTermToExprList(pParse, pSelect, pE, i+1, 1, 0); + if( iCol<0 ){ + return 1; + } + if( iCol>pEList->nExpr ){ + sqlite3ErrorMsg(pParse, + "%r ORDER BY term out of range - should be " + "between 1 and %d", i+1, pEList->nExpr); + return 1; + } + pE->op = TK_COLUMN; + pE->iTable = iTable; + pE->iAgg = -1; + pE->iColumn = iCol-1; + pE->pTab = 0; + } + return 0; } -#endif /* #ifndef SQLITE_OMIT_COMPOUND_SELECT */ /* ** Get a VDBE for the given parser context. Create a new one if necessary. @@ -1768,7 +1890,7 @@ static int multiSelect( ** intermediate results. */ unionTab = pParse->nTab++; - if( pOrderBy && matchOrderbyToColumn(pParse, p, pOrderBy, unionTab) ){ + if( processCompoundOrderBy(pParse, p, unionTab) ){ rc = 1; goto multi_select_end; } @@ -1865,7 +1987,7 @@ static int multiSelect( */ tab1 = pParse->nTab++; tab2 = pParse->nTab++; - if( pOrderBy && matchOrderbyToColumn(pParse,p,pOrderBy,tab1) ){ + if( processCompoundOrderBy(pParse, p, tab1) ){ rc = 1; goto multi_select_end; } @@ -2579,57 +2701,6 @@ static int simpleMinMaxQuery(Parse *pParse, Select *p, int eDest, int iParm){ return 1; } -/* -** Analyze and ORDER BY or GROUP BY clause in a SELECT statement. Return -** the number of errors seen. -** -** An ORDER BY or GROUP BY is a list of expressions. If any expression -** is an integer constant, then that expression is replaced by the -** corresponding entry in the result set. -*/ -static int processOrderGroupBy( - NameContext *pNC, /* Name context of the SELECT statement. */ - ExprList *pOrderBy, /* The ORDER BY or GROUP BY clause to be processed */ - const char *zType /* Either "ORDER" or "GROUP", as appropriate */ -){ - int i; - ExprList *pEList = pNC->pEList; /* The result set of the SELECT */ - Parse *pParse = pNC->pParse; /* The result set of the SELECT */ - assert( pEList ); - - if( pOrderBy==0 ) return 0; - if( pOrderBy->nExpr>SQLITE_MAX_COLUMN ){ - sqlite3ErrorMsg(pParse, "too many terms in %s BY clause", zType); - return 1; - } - for(i=0; inExpr; i++){ - int iCol; - Expr *pE = pOrderBy->a[i].pExpr; - if( sqlite3ExprIsInteger(pE, &iCol) ){ - if( iCol>0 && iCol<=pEList->nExpr ){ - CollSeq *pColl = pE->pColl; - int flags = pE->flags & EP_ExpCollate; - sqlite3ExprDelete(pE); - pE = sqlite3ExprDup(pParse->db, pEList->a[iCol-1].pExpr); - pOrderBy->a[i].pExpr = pE; - if( pColl && flags ){ - pE->pColl = pColl; - pE->flags |= flags; - } - }else{ - sqlite3ErrorMsg(pParse, - "%s BY column number %d out of range - should be " - "between 1 and %d", zType, iCol, pEList->nExpr); - return 1; - } - } - if( sqlite3ExprResolveNames(pNC, pE) ){ - return 1; - } - } - return 0; -} - /* ** This routine resolves any names used in the result set of the ** supplied SELECT statement. If the SELECT statement being resolved @@ -2723,10 +2794,12 @@ int sqlite3SelectResolve( sqlite3ExprResolveNames(&sNC, p->pHaving) ){ return SQLITE_ERROR; } - if( p->pPrior==0 && processOrderGroupBy(&sNC, p->pOrderBy, "ORDER") ){ - return SQLITE_ERROR; + if( p->pPrior==0 ){ + if( processOrderGroupBy(pParse, p, p->pOrderBy, 0, &sNC.hasAgg) ){ + return SQLITE_ERROR; + } } - if( processOrderGroupBy(&sNC, pGroupBy, "GROUP") ){ + if( processOrderGroupBy(pParse, p, pGroupBy, 0, &sNC.hasAgg) ){ return SQLITE_ERROR; } @@ -2946,6 +3019,15 @@ int sqlite3Select( if( sqlite3AuthCheck(pParse, SQLITE_SELECT, 0, 0, 0) ) return 1; memset(&sAggInfo, 0, sizeof(sAggInfo)); + pOrderBy = p->pOrderBy; + if( IgnorableOrderby(eDest) ){ + p->pOrderBy = 0; + } + if( sqlite3SelectResolve(pParse, p, 0) ){ + goto select_end; + } + p->pOrderBy = pOrderBy; + #ifndef SQLITE_OMIT_COMPOUND_SELECT /* If there is are a sequence of queries, do the earlier ones first. */ @@ -2965,15 +3047,6 @@ int sqlite3Select( } #endif - pOrderBy = p->pOrderBy; - if( IgnorableOrderby(eDest) ){ - p->pOrderBy = 0; - } - if( sqlite3SelectResolve(pParse, p, 0) ){ - goto select_end; - } - p->pOrderBy = pOrderBy; - /* Make local copies of the parameters for this query. */ pTabList = p->pSrc; diff --git a/test/null.test b/test/null.test index 9e2d60167e..889252d01f 100644 --- a/test/null.test +++ b/test/null.test @@ -153,11 +153,26 @@ do_test null-5.1 { # as distinct # ifcapable compound { -do_test null-6.1 { - execsql { - select b from t1 union select c from t1 order by c; - } -} {{} 0 1} + do_test null-6.1 { + execsql { + select b from t1 union select c from t1 order by b; + } + } {{} 0 1} + do_test null-6.2 { + execsql { + select b from t1 union select c from t1 order by 1; + } + } {{} 0 1} + do_test null-6.3 { + execsql { + select b from t1 union select c from t1 order by t1.b; + } + } {{} 0 1} + do_test null-6.4 { + execsql { + select b from t1 union select c from t1 order by main.t1.b; + } + } {{} 0 1} } ;# ifcapable compound # The UNIQUE constraint only applies to non-null values