From ffbc30884c0123f9b3eb3a0eb5953b5a78cef465 Mon Sep 17 00:00:00 2001 From: drh Date: Fri, 21 May 2004 01:29:06 +0000 Subject: [PATCH] Sorting bug fixes. Now only 17 tests fail. (CVS 1422) FossilOrigin-Name: 0736b7e8401f587f8b412602d029ef9bd69425f6 --- manifest | 22 +++++++++++----------- manifest.uuid | 2 +- src/build.c | 3 +-- src/select.c | 38 ++++++++++++++++++++++++++++++-------- src/sqliteInt.h | 4 ++-- src/vdbe.c | 26 ++++++++++++++++---------- src/vdbe.h | 11 ++++++++++- src/vdbeaux.c | 47 +++++++++++++++++++++++++++-------------------- 8 files changed, 98 insertions(+), 55 deletions(-) diff --git a/manifest b/manifest index 49b72458ea..4ce5c60c60 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fix\sa\sbug\sthat\sprevented\ssorting\sby\sindex.\s\sDown\sto\s162\sfailed\stests.\s(CVS\s1421) -D 2004-05-20T23:37:55 +C Sorting\sbug\sfixes.\s\sNow\sonly\s17\stests\sfail.\s(CVS\s1422) +D 2004-05-21T01:29:06 F Makefile.in ab7b0d5118e2da97bac66be8684a1034e3500f5a F Makefile.linux-gcc b86a99c493a5bfb402d1d9178dcdc4bd4b32f906 F README f1de682fbbd94899d50aca13d387d1b3fd3be2dd @@ -27,7 +27,7 @@ F src/auth.c 5c2f0bea4729c98c2be3b69d6b466fc51448fe79 F src/btree.c 68f8e0f6271afd31551abf0b48de9667c5f7368b F src/btree.h b65140b5ae891f30d2a39e64b9f0343225553545 F src/btree_rb.c 9d7973e266ee6f9c61ce592f68742ce9cd5b10e5 -F src/build.c ec02b35d542d647ab22f31387733759ee0538826 +F src/build.c 1d71deac55e44e9b2443a2f8f55ca03773555613 F src/copy.c 4d2038602fd0549d80c59bda27d96f13ea9b5e29 F src/date.c 0eb0a89960bb45c7f7e768748605a7a97b0c8064 F src/delete.c 2e1dda38345416a1ea1c0a6468589a7472334dac @@ -47,10 +47,10 @@ F src/parse.y 567718866b94d58a6c7681cc45ba7987771d583a F src/pragma.c aeeba7dc5bc32a6f0980e6516cb2a48a50973fab F src/printf.c ef750e8e2398ca7e8b58be991075f08c6a7f0e53 F src/random.c eff68e3f257e05e81eae6c4d50a51eb88beb4ff3 -F src/select.c 97c78398a825553f58139abaa0dd122c6834baca +F src/select.c d77f773165a2690e6e58c0a94dc21b28270695c2 F src/shell.c 0c4662e13bfbfd3d13b066c5859cc97ad2f95d21 F src/sqlite.h.in 8c000826a517ac7302dc2e1c483e71cd06eaf0de -F src/sqliteInt.h cdde94b620596f39d7b7a4835a8e9ff8d42ed1ec +F src/sqliteInt.h 3f4848eb5b7cd7818943e9f4b924a8601f7ef06a F src/table.c af14284fa36c8d41f6829e3f2819dce07d3e2de2 F src/tclsqlite.c fbf0fac73624ae246551a6c671f1de0235b5faa1 F src/test1.c 5ba6352c8d63eae9eb98e6ae5bfe24a448b3bcb7 @@ -64,10 +64,10 @@ F src/update.c 1a5e9182596f3ea8c7a141e308a3d2a7e5689fee F src/utf.c c27c4f1120f7aaef00cd6942b3d9e3f4ca4fe0e4 F src/util.c 5cbeb452da09cfc7248de9948c15b14d840723f7 F src/vacuum.c c134702e023db8778e6be59ac0ea7b02315b5476 -F src/vdbe.c 09ba3911b8cab84604fa2019cfc252f175b74938 -F src/vdbe.h 5bf4ad99fcb5eee0fb72239775e85ef0d70911cf +F src/vdbe.c d1b6fdfc94b530c90db62aa29777e9154611d53f +F src/vdbe.h d6f66896137af3e313d44553618228d882a2cf85 F src/vdbeInt.h cea492c1fcd85fb78f031e274d1844885d5222e2 -F src/vdbeaux.c 4446afcd568d4002cf2020691d38cdf2c799bc9b +F src/vdbeaux.c 51f7d0cc6c515111b11576e2d82f4637156075cd F src/where.c efe5d25fe18cd7381722457898cd863e84097a0c F test/all.test 569a92a8ee88f5300c057cc4a8f50fbbc69a3242 F test/attach.test cb9b884344e6cfa5e165965d5b1adea679a24c83 @@ -195,7 +195,7 @@ F www/sqlite.tcl 3c83b08cf9f18aa2d69453ff441a36c40e431604 F www/tclsqlite.tcl b9271d44dcf147a93c98f8ecf28c927307abd6da F www/vdbe.tcl 9b9095d4495f37697fd1935d10e14c6015e80aa1 F www/whentouse.tcl a8335bce47cc2fddb07f19052cb0cb4d9129a8e4 -P a6cb09d7af537726acc87b9133f68c81e839e047 -R 77b7dd16da245944750dc57d44fdb5de +P b032b646b72a03e828d732ac22192f992904d79f +R 855c0de9c9913c06ac48c05901c20ce2 U drh -Z 33f52970085179fc3765fcfd7b86df26 +Z cf8b0e36973f7277ebb2343b2ce02ab9 diff --git a/manifest.uuid b/manifest.uuid index 68a60c0370..e83806edae 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -b032b646b72a03e828d732ac22192f992904d79f \ No newline at end of file +0736b7e8401f587f8b412602d029ef9bd69425f6 \ No newline at end of file diff --git a/src/build.c b/src/build.c index 84cd83026c..284c437185 100644 --- a/src/build.c +++ b/src/build.c @@ -23,7 +23,7 @@ ** ROLLBACK ** PRAGMA ** -** $Id: build.c,v 1.190 2004/05/20 22:16:29 drh Exp $ +** $Id: build.c,v 1.191 2004/05/21 01:29:06 drh Exp $ */ #include "sqliteInt.h" #include @@ -789,7 +789,6 @@ CollSeq *sqlite3ChangeCollatingFunction( return 0; } pColl->zName = (char*)&pColl[1]; - pColl->reverseOrder = 0; memcpy(pColl->zName, zName, nName+1); sqlite3HashInsert(&db->aCollSeq, pColl->zName, nName, pColl); } diff --git a/src/select.c b/src/select.c index 7565804c7d..d3504c55f6 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.171 2004/05/20 22:16:29 drh Exp $ +** $Id: select.c,v 1.172 2004/05/21 01:29:06 drh Exp $ */ #include "sqliteInt.h" @@ -318,11 +318,14 @@ static void sqliteAggregateInfoReset(Parse *pParse){ ** be used by the OP_Sort opcode. */ static void pushOntoSorter(Parse *pParse, Vdbe *v, ExprList *pOrderBy){ - char *zSortOrder; int i; +#if 0 + char *zSortOrder; zSortOrder = sqliteMalloc( pOrderBy->nExpr + 1 ); if( zSortOrder==0 ) return; +#endif for(i=0; inExpr; i++){ +#if 0 int order = pOrderBy->a[i].sortOrder; int c; if( order==SQLITE_SO_ASC ){ @@ -331,10 +334,14 @@ static void pushOntoSorter(Parse *pParse, Vdbe *v, ExprList *pOrderBy){ c = 'D'; } zSortOrder[i] = c; +#endif sqlite3ExprCode(pParse, pOrderBy->a[i].pExpr); } +#if 0 zSortOrder[pOrderBy->nExpr] = 0; sqlite3VdbeOp3(v, OP_SortMakeKey, pOrderBy->nExpr, 0, zSortOrder, P3_DYNAMIC); +#endif + sqlite3VdbeAddOp(v, OP_MakeKey, pOrderBy->nExpr, 0); sqlite3VdbeAddOp(v, OP_SortPut, 0, 0); } @@ -541,6 +548,7 @@ static int selectInnerLoop( ** routine generates the code needed to do that. */ static void generateSortTail( + Parse *pParse, /* The parsing context */ Select *p, /* The SELECT statement */ Vdbe *v, /* Generate code into this VDBE */ int nColumn, /* Number of columns of data */ @@ -550,8 +558,23 @@ static void generateSortTail( int end1 = sqlite3VdbeMakeLabel(v); int end2 = sqlite3VdbeMakeLabel(v); int addr; + KeyInfo *pInfo; + ExprList *pOrderBy; + int nCol, i; + sqlite *db = pParse->db; + if( eDest==SRT_Sorter ) return; - sqlite3VdbeAddOp(v, OP_Sort, 0, 0); + pOrderBy = p->pOrderBy; + nCol = pOrderBy->nExpr; + pInfo = sqliteMalloc( sizeof(*pInfo) + nCol*(sizeof(CollSeq*)+1) ); + if( pInfo==0 ) return; + pInfo->aSortOrder = (char*)&pInfo->aColl[nCol]; + pInfo->nField = nCol; + for(i=0; iaColl[i] = db->pDfltColl; + pInfo->aSortOrder[i] = pOrderBy->a[i].sortOrder; + } + sqlite3VdbeOp3(v, OP_Sort, 0, 0, (char*)pInfo, P3_KEYINFO_HANDOFF); addr = sqlite3VdbeAddOp(v, OP_SortNext, 0, end1); if( p->iOffset>=0 ){ sqlite3VdbeAddOp(v, OP_MemIncr, p->iOffset, addr+4); @@ -1214,8 +1237,7 @@ static void openTempIndex(Parse *pParse, Select *p, int iTab, int keyAsData){ for(i=0; iaColl[i] = db->pDfltColl; } - sqlite3VdbeOp3(v, OP_OpenTemp, iTab, 0, (char*)pKeyInfo, P3_KEYINFO); - sqliteFree(pKeyInfo); + sqlite3VdbeOp3(v, OP_OpenTemp, iTab, 0, (char*)pKeyInfo, P3_KEYINFO_HANDOFF); if( keyAsData ){ sqlite3VdbeAddOp(v, OP_KeyAsData, iTab, 1); } @@ -1437,7 +1459,7 @@ static int multiSelect( sqlite3VdbeResolveLabel(v, iBreak); sqlite3VdbeAddOp(v, OP_Close, unionTab, 0); if( p->pOrderBy ){ - generateSortTail(p, v, p->pEList->nExpr, eDest, iParm); + generateSortTail(pParse, p, v, p->pEList->nExpr, eDest, iParm); } } break; @@ -1510,7 +1532,7 @@ static int multiSelect( sqlite3VdbeAddOp(v, OP_Close, tab2, 0); sqlite3VdbeAddOp(v, OP_Close, tab1, 0); if( p->pOrderBy ){ - generateSortTail(p, v, p->pEList->nExpr, eDest, iParm); + generateSortTail(pParse, p, v, p->pEList->nExpr, eDest, iParm); } break; } @@ -2449,7 +2471,7 @@ int sqlite3Select( ** and send them to the callback one by one. */ if( pOrderBy ){ - generateSortTail(p, v, pEList->nExpr, eDest, iParm); + generateSortTail(pParse, p, v, pEList->nExpr, eDest, iParm); } /* If this was a subquery, we have now converted the subquery into a diff --git a/src/sqliteInt.h b/src/sqliteInt.h index 3c3d9aa705..9257953827 100644 --- a/src/sqliteInt.h +++ b/src/sqliteInt.h @@ -11,7 +11,7 @@ ************************************************************************* ** Internal interface definitions for SQLite. ** -** @(#) $Id: sqliteInt.h,v 1.241 2004/05/20 22:16:30 drh Exp $ +** @(#) $Id: sqliteInt.h,v 1.242 2004/05/21 01:29:06 drh Exp $ */ #include "config.h" #include "sqlite.h" @@ -487,7 +487,6 @@ struct Column { */ struct CollSeq { char *zName; /* Name of the collating sequence */ - u8 reverseOrder; /* Compare in reverse order. Used by OP_Sort only */ void *pUser; /* First argument to xCmp() */ int (*xCmp)(void*,int,const void*,int,const void*); /* Comparison function */ }; @@ -647,6 +646,7 @@ struct FKey { struct KeyInfo { u8 incrKey; /* Increase 2nd key by epsilon before comparison */ int nField; /* Number of entries in aColl[] */ + u8 *aSortOrder; /* If defined an aSortOrder[i] is true, sort DESC */ CollSeq *aColl[1]; /* Collating sequence for each term of the key */ }; diff --git a/src/vdbe.c b/src/vdbe.c index daa1528f84..aeea0c416a 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -43,7 +43,7 @@ ** in this file for details. If in doubt, do not deviate from existing ** commenting and indentation practices when changing or adding code. ** -** $Id: vdbe.c,v 1.309 2004/05/20 22:16:30 drh Exp $ +** $Id: vdbe.c,v 1.310 2004/05/21 01:29:06 drh Exp $ */ #include "sqliteInt.h" #include "os.h" @@ -368,13 +368,15 @@ static void hardRealify(Mem *pStack){ ** ** In the case of a tie, left sorts in front of right. */ -static Sorter *Merge(Sorter *pLeft, Sorter *pRight){ +static Sorter *Merge(Sorter *pLeft, Sorter *pRight, KeyInfo *pKeyInfo){ Sorter sHead; Sorter *pTail; pTail = &sHead; pTail->pNext = 0; while( pLeft && pRight ){ - int c = sqlite3SortCompare(pLeft->zKey, pRight->zKey); + int c = sqlite3VdbeKeyCompare(pKeyInfo, pLeft->nKey, pLeft->zKey, + pRight->nKey, pRight->zKey); + /* int c = sqlite3SortCompare(pLeft->zKey, pRight->zKey); */ if( c<=0 ){ pTail->pNext = pLeft; pLeft = pLeft->pNext; @@ -4294,13 +4296,15 @@ case OP_SortMakeKey: { break; } -/* Opcode: Sort * * * +/* Opcode: Sort * * P3 ** ** Sort all elements on the sorter. The algorithm is a -** mergesort. +** mergesort. The P3 argument is a pointer to a KeyInfo structure +** that describes the keys to be sorted. */ case OP_Sort: { int i; + KeyInfo *pKeyInfo = (KeyInfo*)pOp->p3; Sorter *pElem; Sorter *apSorter[NSORT]; for(i=0; i=NSORT-1 ){ - apSorter[NSORT-1] = Merge(apSorter[NSORT-1],pElem); + apSorter[NSORT-1] = Merge(apSorter[NSORT-1],pElem, pKeyInfo); } } pElem = 0; for(i=0; ipSort = pElem; break; @@ -5070,9 +5074,11 @@ default: { } zBuf[2] = '['; k = 3; - for(j=0; j<20 && j>4]; + zBuf[k++] = "0123456789ABCDEF"[c&0xf]; if( c>=0x20 && c<0x7f ){ zBuf[k++] = c; }else{ diff --git a/src/vdbe.h b/src/vdbe.h index c368ccda46..236b584dcd 100644 --- a/src/vdbe.h +++ b/src/vdbe.h @@ -15,7 +15,7 @@ ** or VDBE. The VDBE implements an abstract machine that runs a ** simple program to access and modify the underlying database. ** -** $Id: vdbe.h,v 1.79 2004/05/20 22:16:30 drh Exp $ +** $Id: vdbe.h,v 1.80 2004/05/21 01:29:06 drh Exp $ */ #ifndef _SQLITE_VDBE_H_ #define _SQLITE_VDBE_H_ @@ -71,6 +71,15 @@ typedef struct VdbeOpList VdbeOpList; #define P3_COLLSEQ (-4) /* P3 is a pointer to a CollSeq structure */ #define P3_KEYINFO (-5) /* P3 is a pointer to a KeyInfo structure */ +/* When adding a P3 argument using P3_KEYINFO, a copy of the KeyInfo structure +** is made. That copy is freed when the Vdbe is finalized. But if the +** argument is P3_KEYINFO_HANDOFF, the passed in pointer is used. It still +** gets freed when the Vdbe is finalized so it still should be obtained +** from a single sqliteMalloc(). But no copy is made and the calling +** function should *not* try to free the KeyInfo. +*/ +#define P3_KEYINFO_HANDOFF (-6) + /* ** The following macro converts a relative address in the p2 field ** of a VdbeOp structure into a negative number so that diff --git a/src/vdbeaux.c b/src/vdbeaux.c index d92c067cc4..6831459e4e 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -312,6 +312,9 @@ void sqlite3VdbeChangeP3(Vdbe *p, int addr, const char *zP3, int n){ }else{ pOp->p3type = P3_NOTUSED; } + }else if( n==P3_KEYINFO_HANDOFF ){ + pOp->p3 = (char*)zP3; + pOp->p3type = P3_KEYINFO; }else if( n<0 ){ pOp->p3 = (char*)zP3; pOp->p3type = n; @@ -564,7 +567,7 @@ static char *displayP3(Op *pOp, char *zTemp, int nTemp){ break; } zTemp[i++] = ','; - if( pColl->reverseOrder ){ + if( pKeyInfo->aSortOrder && pKeyInfo->aSortOrder[j] ){ zTemp[i++] = '-'; } strcpy(&zTemp[i], pColl->zName); @@ -582,8 +585,7 @@ static char *displayP3(Op *pOp, char *zTemp, int nTemp){ } case P3_COLLSEQ: { CollSeq *pColl = (CollSeq*)pOp->p3; - sprintf(zTemp, "collseq(%s%.20s)", - pColl->reverseOrder ? "-" : "", pColl->zName); + sprintf(zTemp, "collseq(%.20s)", pColl->zName); zP3 = zTemp; break; } @@ -1531,11 +1533,6 @@ int sqlite3MemCompare(const Mem *pMem1, const Mem *pMem2, const CollSeq *pColl){ /* Interchange pMem1 and pMem2 if the collating sequence specifies ** DESC order. */ - if( pColl && pColl->reverseOrder ){ - const Mem *pTemp = pMem1; - pMem1 = pMem2; - pMem2 = pTemp; - } f1 = pMem1->flags; f2 = pMem2->flags; combined_flags = f1|f2; @@ -1632,6 +1629,7 @@ int sqlite3VdbeKeyCompare( int offset1 = 0; int offset2 = 0; int i = 0; + int rc = 0; const unsigned char *aKey1 = (const unsigned char *)pKey1; const unsigned char *aKey2 = (const unsigned char *)pKey2; @@ -1641,7 +1639,6 @@ int sqlite3VdbeKeyCompare( Mem mem2; u64 serial_type1; u64 serial_type2; - int rc; /* Read the serial types for the next element in each key. */ offset1 += sqlite3GetVarint(&aKey1[offset1], &serial_type1); @@ -1656,7 +1653,14 @@ int sqlite3VdbeKeyCompare( assert( !serial_type1 && !serial_type2 ); sqlite3GetVarint(&aKey1[offset1], &serial_type1); sqlite3GetVarint(&aKey2[offset2], &serial_type2); - return ( (i64)serial_type1 - (i64)serial_type2 ); + if( serial_type1 < serial_type2 ){ + rc = -1; + }else if( serial_type1 > serial_type2 ){ + rc = +1; + }else{ + rc = 0; + } + return rc; } assert( inField ); @@ -1677,7 +1681,7 @@ int sqlite3VdbeKeyCompare( sqliteFree(mem2.z); } if( rc!=0 ){ - return rc; + break; } i++; } @@ -1686,19 +1690,22 @@ int sqlite3VdbeKeyCompare( ** were equal. If the incrKey flag is true, then the second key is ** treated as larger. */ - if( pKeyInfo->incrKey ){ - assert( offset2==nKey2 ); - return -1; + if( rc==0 ){ + if( pKeyInfo->incrKey ){ + assert( offset2==nKey2 ); + rc = -1; + }else if( offset1aSortOrder && inField && pKeyInfo->aSortOrder[i] ){ + rc = -rc; } - return 0; + return rc; } /*