From 4387006c18f99a63fffbed12ce30a0a099999c18 Mon Sep 17 00:00:00 2001 From: drh Date: Thu, 31 Jul 2014 15:44:44 +0000 Subject: Deactivate the DISTINCT in a SELECT on the right-hand side of an IN operator, since it should not make any difference in the output but dues consume extra memory and CPU time. FossilOrigin-Name: f4cb53651b1e352fae7378878b830a902bcd9248 --- src/expr.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 72286dfdf..94647e514 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1624,7 +1624,7 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, int *prNotFound){ } if( eType==0 ){ - /* Could not found an existing table or index to use as the RHS b-tree. + /* Could not find an existing table or index to use as the RHS b-tree. ** We will have to generate an ephemeral table to do the job. */ u32 savedNQueryLoop = pParse->nQueryLoop; @@ -1754,6 +1754,7 @@ int sqlite3CodeSubselect( ** Generate code to write the results of the select into the temporary ** table allocated and opened above. */ + Select *pSelect = pExpr->x.pSelect; SelectDest dest; ExprList *pEList; @@ -1761,13 +1762,15 @@ int sqlite3CodeSubselect( sqlite3SelectDestInit(&dest, SRT_Set, pExpr->iTable); dest.affSdst = (u8)affinity; assert( (pExpr->iTable&0x0000FFFF)==pExpr->iTable ); - pExpr->x.pSelect->iLimit = 0; + pSelect->iLimit = 0; + testcase( pSelect->selFlags & SF_Distinct ); + pSelect->selFlags &= ~SF_Distinct; testcase( pKeyInfo==0 ); /* Caused by OOM in sqlite3KeyInfoAlloc() */ - if( sqlite3Select(pParse, pExpr->x.pSelect, &dest) ){ + if( sqlite3Select(pParse, pSelect, &dest) ){ sqlite3KeyInfoUnref(pKeyInfo); return 0; } - pEList = pExpr->x.pSelect->pEList; + pEList = pSelect->pEList; assert( pKeyInfo!=0 ); /* OOM will cause exit after sqlite3Select() */ assert( pEList!=0 ); assert( pEList->nExpr>0 ); -- cgit v1.2.3 From 37e08081f3da7ceab00ff4db996510f924de931a Mon Sep 17 00:00:00 2001 From: drh Date: Thu, 31 Jul 2014 20:16:08 +0000 Subject: Omit a pointless OP_Null when processing a value-list RHS of an IN operator where the LHS is a rowid. FossilOrigin-Name: 1361450a9dfe9476e8df98f370a3695752252245 --- src/expr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 94647e514..aa55ff7af 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1801,7 +1801,7 @@ int sqlite3CodeSubselect( /* Loop through each expression in . */ r1 = sqlite3GetTempReg(pParse); r2 = sqlite3GetTempReg(pParse); - sqlite3VdbeAddOp2(v, OP_Null, 0, r2); + if( isRowid ) sqlite3VdbeAddOp2(v, OP_Null, 0, r2); for(i=pList->nExpr, pItem=pList->a; i>0; i--, pItem++){ Expr *pE2 = pItem->pExpr; int iValToIns; -- cgit v1.2.3 From 5f1d1d9c870f2377d6907ec05df3c6d38d75cd57 Mon Sep 17 00:00:00 2001 From: drh Date: Thu, 31 Jul 2014 22:59:04 +0000 Subject: Refactoring: Change "pIndex->onError!=OE_None" to use a macro: "IsUniqueIndex(pIndex)". Easier to understand that way. FossilOrigin-Name: e75b26ee357bb3d3c1a539b05d633ebf314726d7 --- src/expr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index aa55ff7af..3b254f3d3 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1604,7 +1604,7 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, int *prNotFound){ for(pIdx=pTab->pIndex; pIdx && eType==0 && affinity_ok; pIdx=pIdx->pNext){ if( (pIdx->aiColumn[0]==iCol) && sqlite3FindCollSeq(db, ENC(db), pIdx->azColl[0], 0)==pReq - && (!mustBeUnique || (pIdx->nKeyCol==1 && pIdx->onError!=OE_None)) + && (!mustBeUnique || (pIdx->nKeyCol==1 && IsUniqueIndex(pIdx))) ){ int iAddr = sqlite3CodeOnce(pParse); VdbeCoverage(v); sqlite3VdbeAddOp3(v, OP_OpenRead, iTab, pIdx->tnum, iDb); -- cgit v1.2.3 From 3a85625d87c2b5d0f4cd504cb4fdc5d8fc5ee8c4 Mon Sep 17 00:00:00 2001 From: drh Date: Fri, 1 Aug 2014 14:46:57 +0000 Subject: Begin making changes to the IN operator in an attempt to make it run faster and to make the code easier to understand. FossilOrigin-Name: ee0fd6aaf94cda1dce3fe752bfe3b0f83e0043f1 --- src/expr.c | 64 +++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 26 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 3b254f3d3..dd7b7c3de 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1484,7 +1484,7 @@ int sqlite3CodeOnce(Parse *pParse){ ** be used either to test for membership in the RHS set or to iterate through ** all members of the RHS set, skipping duplicates. ** -** A cursor is opened on the b-tree object that the RHS of the IN operator +** A cursor is opened on the b-tree object that is the RHS of the IN operator ** and pX->iTable is set to the index of that cursor. ** ** The returned value of this function indicates the b-tree type, as follows: @@ -1494,6 +1494,8 @@ int sqlite3CodeOnce(Parse *pParse){ ** IN_INDEX_INDEX_DESC - The cursor was opened on a descending index. ** IN_INDEX_EPH - The cursor was opened on a specially created and ** populated epheremal table. +** IN_INDEX_NOOP - No cursor was allocated. The in operator must be +** implemented as a sequence of comparisons. ** ** An existing b-tree might be used if the RHS expression pX is a simple ** subquery such as: @@ -1503,23 +1505,37 @@ int sqlite3CodeOnce(Parse *pParse){ ** If the RHS of the IN operator is a list or a more complex subquery, then ** an ephemeral table might need to be generated from the RHS and then ** pX->iTable made to point to the ephermeral table instead of an -** existing table. +** existing table. ** -** If the prNotFound parameter is 0, then the b-tree will be used to iterate -** through the set members, skipping any duplicates. In this case an -** epheremal table must be used unless the selected is guaranteed +** The inFlags parameter must contain exactly one of the bits +** IN_INDEX_MEMBERSHIP or IN_INDEX_LOOP. If inFlags contains +** IN_INDEX_MEMBERSHIP, then the generated table will be used for a +** fast membership test. When the IN_INDEX_LOOP bit is set, the +** IN index will be used to loop over all values of the RHS of the +** IN operator. +** +** When IN_INDEX_LOOP is used (and the b-tree will be used to iterate +** through the set members) then the b-tree must not contain duplicates. +** An epheremal table must be used unless the selected is guaranteed ** to be unique - either because it is an INTEGER PRIMARY KEY or it ** has a UNIQUE constraint or UNIQUE index. ** -** If the prNotFound parameter is not 0, then the b-tree will be used -** for fast set membership tests. In this case an epheremal table must +** When IN_INDEX_MEMBERSHIP is used (and the b-tree will be used +** for fast set membership tests) then an epheremal table must ** be used unless is an INTEGER PRIMARY KEY or an index can ** be found with as its left-most column. ** +** If the IN_INDEX_NOOP_OK and IN_INDEX_MEMBERSHIP are both set and +** if the RHS of the IN operator is a list (not a subquery) then this +** routine might decide that creating an ephemeral b-tree for membership +** testing is too expensive and return IN_INDEX_NOOP. IN that case, the +** calling routine should implement the IN operator using a sequence +** of Eq or Ne comparison operations. +** ** When the b-tree is being used for membership tests, the calling function -** needs to know whether or not the structure contains an SQL NULL -** value in order to correctly evaluate expressions like "X IN (Y, Z)". -** If there is any chance that the (...) might contain a NULL value at +** might need to know whether or not the RHS side of the IN operator +** contains a NULL. If prNotFound is not NULL and +** if there is any chance that the (...) might contain a NULL value at ** runtime, then a register is allocated and the register number written ** to *prNotFound. If there is no chance that the (...) contains a ** NULL value, then *prNotFound is left unchanged. @@ -1540,14 +1556,15 @@ int sqlite3CodeOnce(Parse *pParse){ ** test more often than is necessary. */ #ifndef SQLITE_OMIT_SUBQUERY -int sqlite3FindInIndex(Parse *pParse, Expr *pX, int *prNotFound){ +int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prNotFound){ Select *p; /* SELECT to the right of IN operator */ int eType = 0; /* Type of RHS table. IN_INDEX_* */ int iTab = pParse->nTab++; /* Cursor of the RHS table */ - int mustBeUnique = (prNotFound==0); /* True if RHS must be unique */ + int mustBeUnique; /* True if RHS must be unique */ Vdbe *v = sqlite3GetVdbe(pParse); /* Virtual machine being coded */ assert( pX->op==TK_IN ); + mustBeUnique = (inFlags & IN_INDEX_LOOP)!=0; /* Check to see if an existing table or index can be used to ** satisfy the query. This is preferable to generating a new @@ -1630,14 +1647,14 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, int *prNotFound){ u32 savedNQueryLoop = pParse->nQueryLoop; int rMayHaveNull = 0; eType = IN_INDEX_EPH; - if( prNotFound ){ - *prNotFound = rMayHaveNull = ++pParse->nMem; - sqlite3VdbeAddOp2(v, OP_Null, 0, *prNotFound); - }else{ + if( inFlags & IN_INDEX_LOOP ){ pParse->nQueryLoop = 0; if( pX->pLeft->iColumn<0 && !ExprHasProperty(pX, EP_xIsSelect) ){ eType = IN_INDEX_ROWID; } + }else if( prNotFound ){ + *prNotFound = rMayHaveNull = ++pParse->nMem; + sqlite3VdbeAddOp2(v, OP_Null, 0, rMayHaveNull); } sqlite3CodeSubselect(pParse, pX, rMayHaveNull, eType==IN_INDEX_ROWID); pParse->nQueryLoop = savedNQueryLoop; @@ -1668,15 +1685,9 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, int *prNotFound){ ** ** If rMayHaveNull is non-zero, that means that the operation is an IN ** (not a SELECT or EXISTS) and that the RHS might contains NULLs. -** Furthermore, the IN is in a WHERE clause and that we really want -** to iterate over the RHS of the IN operator in order to quickly locate -** all corresponding LHS elements. All this routine does is initialize -** the register given by rMayHaveNull to NULL. Calling routines will take -** care of changing this register value to non-NULL if the RHS is NULL-free. -** -** If rMayHaveNull is zero, that means that the subquery is being used -** for membership testing only. There is no need to initialize any -** registers to indicate the presence or absence of NULLs on the RHS. +** All this routine does is initialize the register given by rMayHaveNull +** to NULL. Calling routines will take care of changing this register +** value to non-NULL if the RHS is NULL-free. ** ** For a SELECT or EXISTS operator, return the register that holds the ** result. For IN operators or if an error occurs, the return value is 0. @@ -1928,7 +1939,8 @@ static void sqlite3ExprCodeIN( v = pParse->pVdbe; assert( v!=0 ); /* OOM detected prior to this routine */ VdbeNoopComment((v, "begin IN expr")); - eType = sqlite3FindInIndex(pParse, pExpr, &rRhsHasNull); + eType = sqlite3FindInIndex(pParse, pExpr, 0, + destIfFalse==destIfNull ? 0 : &rRhsHasNull); /* Figure out the affinity to use to create a key from the results ** of the expression. affinityStr stores a static string suitable for -- cgit v1.2.3 From e80c9b9ad5e62af9ccd8a4ce8f069c965dde6070 Mon Sep 17 00:00:00 2001 From: drh Date: Fri, 1 Aug 2014 15:34:36 +0000 Subject: The idea of coding IN operator with a short list on the RHS as an OR expression turns out to be helpful. If the list is of length 1 or 2, the OR expression is very slightly faster, but the ephemeral table approach is clearly better for all list lengths greater than 2. Better to keep the code simple. FossilOrigin-Name: e13175d3579e1045165bab091b3b28951d691704 --- src/expr.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index dd7b7c3de..d81882710 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1494,8 +1494,6 @@ int sqlite3CodeOnce(Parse *pParse){ ** IN_INDEX_INDEX_DESC - The cursor was opened on a descending index. ** IN_INDEX_EPH - The cursor was opened on a specially created and ** populated epheremal table. -** IN_INDEX_NOOP - No cursor was allocated. The in operator must be -** implemented as a sequence of comparisons. ** ** An existing b-tree might be used if the RHS expression pX is a simple ** subquery such as: @@ -1525,13 +1523,6 @@ int sqlite3CodeOnce(Parse *pParse){ ** be used unless is an INTEGER PRIMARY KEY or an index can ** be found with as its left-most column. ** -** If the IN_INDEX_NOOP_OK and IN_INDEX_MEMBERSHIP are both set and -** if the RHS of the IN operator is a list (not a subquery) then this -** routine might decide that creating an ephemeral b-tree for membership -** testing is too expensive and return IN_INDEX_NOOP. IN that case, the -** calling routine should implement the IN operator using a sequence -** of Eq or Ne comparison operations. -** ** When the b-tree is being used for membership tests, the calling function ** might need to know whether or not the RHS side of the IN operator ** contains a NULL. If prNotFound is not NULL and @@ -1939,7 +1930,7 @@ static void sqlite3ExprCodeIN( v = pParse->pVdbe; assert( v!=0 ); /* OOM detected prior to this routine */ VdbeNoopComment((v, "begin IN expr")); - eType = sqlite3FindInIndex(pParse, pExpr, 0, + eType = sqlite3FindInIndex(pParse, pExpr, IN_INDEX_MEMBERSHIP, destIfFalse==destIfNull ? 0 : &rRhsHasNull); /* Figure out the affinity to use to create a key from the results @@ -1986,7 +1977,8 @@ static void sqlite3ExprCodeIN( ** contains one or more NULL values, then the result of the ** expression is also NULL. */ - if( rRhsHasNull==0 || destIfFalse==destIfNull ){ + assert( destIfFalse!=destIfNull || rRhsHasNull==0 ); + if( rRhsHasNull==0 ){ /* This branch runs if it is known at compile time that the RHS ** cannot contain NULL values. This happens as the result ** of a "NOT NULL" constraint in the database schema. -- cgit v1.2.3 From e21a6e1dfe2196184bf17e9d9212f3eae0b346e5 Mon Sep 17 00:00:00 2001 From: drh Date: Fri, 1 Aug 2014 18:00:24 +0000 Subject: Remove an unnecessary OP_Null in the IN-operator logic. Attempt to clarify comments explaining the IN-operator code, though it is not clear that the comments are correct even yet - more work to be done. FossilOrigin-Name: c11e55fabbc718cb324ecd3540453c25db98f50c --- src/expr.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index d81882710..591ea9845 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1525,29 +1525,34 @@ int sqlite3CodeOnce(Parse *pParse){ ** ** When the b-tree is being used for membership tests, the calling function ** might need to know whether or not the RHS side of the IN operator -** contains a NULL. If prNotFound is not NULL and +** contains a NULL. If prRhsHasNull is not a NULL pointer and ** if there is any chance that the (...) might contain a NULL value at ** runtime, then a register is allocated and the register number written -** to *prNotFound. If there is no chance that the (...) contains a -** NULL value, then *prNotFound is left unchanged. +** to *prRhsHasNull. If there is no chance that the (...) contains a +** NULL value, then *prRhsHasNull is left unchanged. ** -** If a register is allocated and its location stored in *prNotFound, then -** its initial value is NULL. If the (...) does not remain constant -** for the duration of the query (i.e. the SELECT within the (...) -** is a correlated subquery) then the value of the allocated register is -** reset to NULL each time the subquery is rerun. This allows the -** caller to use vdbe code equivalent to the following: +** If a register is allocated and its location stored in *prRhsHasNull, then +** the value in that register will be: ** -** if( register==NULL ){ -** has_null = -** register = 1 +** 0 if the (...) contains no NULL values +** 1 if the (...) does not contain NULL values +** NULL if we do not yet know if (...) contains NULLs +** +** If the (...) does not remain constant for the duration of the query +** (i.e. the SELECT within the (...) is a correlated subquery) then the +** value of the allocated register is reset to NULL each time the subquery +** is rerun. This allows the caller to use vdbe code equivalent to the +** following: +** +** if( r[*prRhsHasNull] IS NULL ){ +** r[*prRhsHasNull] = ** } ** ** in order to avoid running the ** test more often than is necessary. */ #ifndef SQLITE_OMIT_SUBQUERY -int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prNotFound){ +int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prRhsHasNull){ Select *p; /* SELECT to the right of IN operator */ int eType = 0; /* Type of RHS table. IN_INDEX_* */ int iTab = pParse->nTab++; /* Cursor of the RHS table */ @@ -1621,9 +1626,9 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prNotFound){ assert( IN_INDEX_INDEX_DESC == IN_INDEX_INDEX_ASC+1 ); eType = IN_INDEX_INDEX_ASC + pIdx->aSortOrder[0]; - if( prNotFound && !pTab->aCol[iCol].notNull ){ - *prNotFound = ++pParse->nMem; - sqlite3VdbeAddOp2(v, OP_Null, 0, *prNotFound); + if( prRhsHasNull && !pTab->aCol[iCol].notNull ){ + *prRhsHasNull = ++pParse->nMem; + sqlite3VdbeAddOp2(v, OP_Null, 0, *prRhsHasNull); } sqlite3VdbeJumpHere(v, iAddr); } @@ -1643,9 +1648,8 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prNotFound){ if( pX->pLeft->iColumn<0 && !ExprHasProperty(pX, EP_xIsSelect) ){ eType = IN_INDEX_ROWID; } - }else if( prNotFound ){ - *prNotFound = rMayHaveNull = ++pParse->nMem; - sqlite3VdbeAddOp2(v, OP_Null, 0, rMayHaveNull); + }else if( prRhsHasNull ){ + *prRhsHasNull = rMayHaveNull = ++pParse->nMem; } sqlite3CodeSubselect(pParse, pX, rMayHaveNull, eType==IN_INDEX_ROWID); pParse->nQueryLoop = savedNQueryLoop; -- cgit v1.2.3 From 6be515ebe0c6976352efd82cd6cddebe4b43361f Mon Sep 17 00:00:00 2001 From: drh Date: Fri, 1 Aug 2014 21:00:53 +0000 Subject: Improved detection and handling of NULL values on the RHS of a IN operator. FossilOrigin-Name: 468e730036edac22cfeb9ea3515aa16e6bcd6650 --- src/expr.c | 94 ++++++++++++++++++++++++++------------------------------------ 1 file changed, 40 insertions(+), 54 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 591ea9845..6440dea39 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1475,6 +1475,22 @@ int sqlite3CodeOnce(Parse *pParse){ return sqlite3VdbeAddOp1(v, OP_Once, pParse->nOnce++); } +/* +** Generate code that checks the single-column index table iCur to see if +** contains any NULL entries. Cause the register at regHasNull to be set +** to a non-NULL value if iCur contains no NULLs. Cause register regHasNull +** to be set to NULL if iCur contains one or more NULL values. +*/ +static void sqlite3SetHasNullFlag(Vdbe *v, int iCur, int regHasNull){ + int j1; + sqlite3VdbeAddOp2(v, OP_Integer, 0, regHasNull); + j1 = sqlite3VdbeAddOp1(v, OP_Rewind, iCur); VdbeCoverage(v); + sqlite3VdbeAddOp3(v, OP_Column, iCur, 0, regHasNull); + sqlite3VdbeChangeP5(v, OPFLAG_TYPEOFARG); + VdbeComment((v, "")); + sqlite3VdbeJumpHere(v, j1); +} + /* ** This function is used by the implementation of the IN (...) operator. ** The pX parameter is the expression on the RHS of the IN operator, which @@ -1532,24 +1548,9 @@ int sqlite3CodeOnce(Parse *pParse){ ** NULL value, then *prRhsHasNull is left unchanged. ** ** If a register is allocated and its location stored in *prRhsHasNull, then -** the value in that register will be: -** -** 0 if the (...) contains no NULL values -** 1 if the (...) does not contain NULL values -** NULL if we do not yet know if (...) contains NULLs -** -** If the (...) does not remain constant for the duration of the query -** (i.e. the SELECT within the (...) is a correlated subquery) then the -** value of the allocated register is reset to NULL each time the subquery -** is rerun. This allows the caller to use vdbe code equivalent to the -** following: -** -** if( r[*prRhsHasNull] IS NULL ){ -** r[*prRhsHasNull] = -** } -** -** in order to avoid running the -** test more often than is necessary. +** the value in that register will be NULL if the b-tree contains one or more +** NULL values, and it will be some non-NULL value if the b-tree contains no +** NULL values. */ #ifndef SQLITE_OMIT_SUBQUERY int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prRhsHasNull){ @@ -1628,7 +1629,7 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prRhsHasNull){ if( prRhsHasNull && !pTab->aCol[iCol].notNull ){ *prRhsHasNull = ++pParse->nMem; - sqlite3VdbeAddOp2(v, OP_Null, 0, *prRhsHasNull); + sqlite3SetHasNullFlag(v, iTab, *prRhsHasNull); } sqlite3VdbeJumpHere(v, iAddr); } @@ -1691,10 +1692,10 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prRhsHasNull){ int sqlite3CodeSubselect( Parse *pParse, /* Parsing context */ Expr *pExpr, /* The IN, SELECT, or EXISTS operator */ - int rMayHaveNull, /* Register that records whether NULLs exist in RHS */ + int rHasNullFlag, /* Register that records whether NULLs exist in RHS */ int isRowid /* If true, LHS of IN operator is a rowid */ ){ - int testAddr = -1; /* One-time test address */ + int jmpIfDynamic = -1; /* One-time test address */ int rReg = 0; /* Register storing resulting */ Vdbe *v = sqlite3GetVdbe(pParse); if( NEVER(v==0) ) return 0; @@ -1711,13 +1712,13 @@ int sqlite3CodeSubselect( ** save the results, and reuse the same result on subsequent invocations. */ if( !ExprHasProperty(pExpr, EP_VarSelect) ){ - testAddr = sqlite3CodeOnce(pParse); VdbeCoverage(v); + jmpIfDynamic = sqlite3CodeOnce(pParse); VdbeCoverage(v); } #ifndef SQLITE_OMIT_EXPLAIN if( pParse->explain==2 ){ char *zMsg = sqlite3MPrintf( - pParse->db, "EXECUTE %s%s SUBQUERY %d", testAddr>=0?"":"CORRELATED ", + pParse->db, "EXECUTE %s%s SUBQUERY %d", jmpIfDynamic>=0?"":"CORRELATED ", pExpr->op==TK_IN?"LIST":"SCALAR", pParse->iNextSelectId ); sqlite3VdbeAddOp4(v, OP_Explain, pParse->iSelectId, 0, 0, zMsg, P4_DYNAMIC); @@ -1731,10 +1732,6 @@ int sqlite3CodeSubselect( Expr *pLeft = pExpr->pLeft; /* the LHS of the IN operator */ KeyInfo *pKeyInfo = 0; /* Key information */ - if( rMayHaveNull ){ - sqlite3VdbeAddOp2(v, OP_Null, 0, rMayHaveNull); - } - affinity = sqlite3ExprAffinity(pLeft); /* Whether this is an 'x IN(SELECT...)' or an 'x IN()' @@ -1817,9 +1814,9 @@ int sqlite3CodeSubselect( ** this code only executes once. Because for a non-constant ** expression we need to rerun this code each time. */ - if( testAddr>=0 && !sqlite3ExprIsConstant(pE2) ){ - sqlite3VdbeChangeToNoop(v, testAddr); - testAddr = -1; + if( jmpIfDynamic>=0 && !sqlite3ExprIsConstant(pE2) ){ + sqlite3VdbeChangeToNoop(v, jmpIfDynamic); + jmpIfDynamic = -1; } /* Evaluate the expression and insert it into the temp table */ @@ -1889,8 +1886,12 @@ int sqlite3CodeSubselect( } } - if( testAddr>=0 ){ - sqlite3VdbeJumpHere(v, testAddr); + if( rHasNullFlag ){ + sqlite3SetHasNullFlag(v, pExpr->iTable, rHasNullFlag); + } + + if( jmpIfDynamic>=0 ){ + sqlite3VdbeJumpHere(v, jmpIfDynamic); } sqlite3ExprCachePop(pParse); @@ -1911,7 +1912,7 @@ int sqlite3CodeSubselect( ** if the LHS is NULL or if the LHS is not contained within the RHS and the ** RHS contains one or more NULL values. ** -** This routine generates code will jump to destIfFalse if the LHS is not +** This routine generates code that jumps to destIfFalse if the LHS is not ** contained within the RHS. If due to NULLs we cannot determine if the LHS ** is contained in the RHS then jump to destIfNull. If the LHS is contained ** within the RHS then fall through. @@ -1997,34 +1998,19 @@ static void sqlite3ExprCodeIN( ** the presence of a NULL on the RHS makes a difference in the ** outcome. */ - int j1, j2; + int j1; /* First check to see if the LHS is contained in the RHS. If so, - ** then the presence of NULLs in the RHS does not matter, so jump - ** over all of the code that follows. + ** then the answer is TRUE the presence of NULLs in the RHS does + ** not matter. If the LHS is not contained in the RHS, then the + ** answer is NULL if the RHS contains NULLs and the answer is + ** FALSE if the RHS is NULL-free. */ j1 = sqlite3VdbeAddOp4Int(v, OP_Found, pExpr->iTable, 0, r1, 1); VdbeCoverage(v); - - /* Here we begin generating code that runs if the LHS is not - ** contained within the RHS. Generate additional code that - ** tests the RHS for NULLs. If the RHS contains a NULL then - ** jump to destIfNull. If there are no NULLs in the RHS then - ** jump to destIfFalse. - */ - sqlite3VdbeAddOp2(v, OP_If, rRhsHasNull, destIfNull); VdbeCoverage(v); - sqlite3VdbeAddOp2(v, OP_IfNot, rRhsHasNull, destIfFalse); VdbeCoverage(v); - j2 = sqlite3VdbeAddOp4Int(v, OP_Found, pExpr->iTable, 0, rRhsHasNull, 1); + sqlite3VdbeAddOp2(v, OP_IsNull, rRhsHasNull, destIfNull); VdbeCoverage(v); - sqlite3VdbeAddOp2(v, OP_Integer, 0, rRhsHasNull); sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfFalse); - sqlite3VdbeJumpHere(v, j2); - sqlite3VdbeAddOp2(v, OP_Integer, 1, rRhsHasNull); - sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfNull); - - /* The OP_Found at the top of this branch jumps here when true, - ** causing the overall IN expression evaluation to fall through. - */ sqlite3VdbeJumpHere(v, j1); } } -- cgit v1.2.3 From 4c259e9f40a0580c60455f9f4b7492f1d99749a7 Mon Sep 17 00:00:00 2001 From: drh Date: Fri, 1 Aug 2014 21:12:35 +0000 Subject: A better comment on the generated code for the NULL-in-RHS-of-IN detection logic. FossilOrigin-Name: 9bc1c730a366e75b760b58e7a343d39165b2a469 --- src/expr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 6440dea39..c32affe34 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1476,8 +1476,8 @@ int sqlite3CodeOnce(Parse *pParse){ } /* -** Generate code that checks the single-column index table iCur to see if -** contains any NULL entries. Cause the register at regHasNull to be set +** Generate code that checks the left-most column of index table iCur to see if +** it contains any NULL entries. Cause the register at regHasNull to be set ** to a non-NULL value if iCur contains no NULLs. Cause register regHasNull ** to be set to NULL if iCur contains one or more NULL values. */ @@ -1487,7 +1487,7 @@ static void sqlite3SetHasNullFlag(Vdbe *v, int iCur, int regHasNull){ j1 = sqlite3VdbeAddOp1(v, OP_Rewind, iCur); VdbeCoverage(v); sqlite3VdbeAddOp3(v, OP_Column, iCur, 0, regHasNull); sqlite3VdbeChangeP5(v, OPFLAG_TYPEOFARG); - VdbeComment((v, "")); + VdbeComment((v, "first_entry_in(%d)", iCur)); sqlite3VdbeJumpHere(v, j1); } -- cgit v1.2.3 From bb53ecb1dbd36de81519086bab2c6589640daf49 Mon Sep 17 00:00:00 2001 From: drh Date: Sat, 2 Aug 2014 21:03:33 +0000 Subject: Enhancements to the code generator for the IN operator that result in much faster queries in some cases, for example when the RHS of the IN operator changes for each row of a large table scan. FossilOrigin-Name: 436e884215e2b33ca3fbb555362237b12827c07a --- src/expr.c | 195 ++++++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 141 insertions(+), 54 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index c32affe34..7565d41b9 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1491,6 +1491,24 @@ static void sqlite3SetHasNullFlag(Vdbe *v, int iCur, int regHasNull){ sqlite3VdbeJumpHere(v, j1); } + +#ifndef SQLITE_OMIT_SUBQUERY +/* +** The argument is an IN operator with a list (not a subquery) on the +** right-hand side. Return TRUE if that list is constant. +*/ +static int sqlite3InRhsIsConstant(Expr *pIn){ + Expr *pLHS; + int res; + assert( !ExprHasProperty(pIn, EP_xIsSelect) ); + pLHS = pIn->pLeft; + pIn->pLeft = 0; + res = sqlite3ExprIsConstant(pIn); + pIn->pLeft = pLHS; + return res; +} +#endif + /* ** This function is used by the implementation of the IN (...) operator. ** The pX parameter is the expression on the RHS of the IN operator, which @@ -1510,6 +1528,8 @@ static void sqlite3SetHasNullFlag(Vdbe *v, int iCur, int regHasNull){ ** IN_INDEX_INDEX_DESC - The cursor was opened on a descending index. ** IN_INDEX_EPH - The cursor was opened on a specially created and ** populated epheremal table. +** IN_INDEX_NOOP - No cursor was allocated. The IN operator must be +** implemented as a sequence of comparisons. ** ** An existing b-tree might be used if the RHS expression pX is a simple ** subquery such as: @@ -1539,6 +1559,13 @@ static void sqlite3SetHasNullFlag(Vdbe *v, int iCur, int regHasNull){ ** be used unless is an INTEGER PRIMARY KEY or an index can ** be found with as its left-most column. ** +** If the IN_INDEX_NOOP_OK and IN_INDEX_MEMBERSHIP are both set and +** if the RHS of the IN operator is a list (not a subquery) then this +** routine might decide that creating an ephemeral b-tree for membership +** testing is too expensive and return IN_INDEX_NOOP. In that case, the +** calling routine should implement the IN operator using a sequence +** of Eq or Ne comparison operations. +** ** When the b-tree is being used for membership tests, the calling function ** might need to know whether or not the RHS side of the IN operator ** contains a NULL. If prRhsHasNull is not a NULL pointer and @@ -1637,6 +1664,22 @@ int sqlite3FindInIndex(Parse *pParse, Expr *pX, u32 inFlags, int *prRhsHasNull){ } } + /* If no preexisting index is available for the IN clause + ** and IN_INDEX_NOOP is an allowed reply + ** and the RHS of the IN operator is a list, not a subquery + ** and the RHS is not contant or has two or fewer terms, + ** then it is not worth creating an ephermeral table to evaluate + ** the IN operator so return IN_INDEX_NOOP. + */ + if( eType==0 + && (inFlags & IN_INDEX_NOOP_OK) + && !ExprHasProperty(pX, EP_xIsSelect) + && (!sqlite3InRhsIsConstant(pX) || pX->x.pList->nExpr<=2) + ){ + eType = IN_INDEX_NOOP; + } + + if( eType==0 ){ /* Could not find an existing table or index to use as the RHS b-tree. ** We will have to generate an ephemeral table to do the job. @@ -1935,7 +1978,8 @@ static void sqlite3ExprCodeIN( v = pParse->pVdbe; assert( v!=0 ); /* OOM detected prior to this routine */ VdbeNoopComment((v, "begin IN expr")); - eType = sqlite3FindInIndex(pParse, pExpr, IN_INDEX_MEMBERSHIP, + eType = sqlite3FindInIndex(pParse, pExpr, + IN_INDEX_MEMBERSHIP | IN_INDEX_NOOP_OK, destIfFalse==destIfNull ? 0 : &rRhsHasNull); /* Figure out the affinity to use to create a key from the results @@ -1950,68 +1994,111 @@ static void sqlite3ExprCodeIN( r1 = sqlite3GetTempReg(pParse); sqlite3ExprCode(pParse, pExpr->pLeft, r1); - /* If the LHS is NULL, then the result is either false or NULL depending - ** on whether the RHS is empty or not, respectively. + /* If sqlite3FindInIndex() did not find or create an index that is + ** suitable for evaluating the IN operator, then evaluate using a + ** sequence of comparisons. */ - if( destIfNull==destIfFalse ){ - /* Shortcut for the common case where the false and NULL outcomes are - ** the same. */ + if( eType==IN_INDEX_NOOP ){ + ExprList *pList = pExpr->x.pList; + CollSeq *pColl = sqlite3ExprCollSeq(pParse, pExpr->pLeft); + int labelOk = sqlite3VdbeMakeLabel(v); + int r2, regToFree; + int regCkNull = 0; + int ii; + assert( !ExprHasProperty(pExpr, EP_xIsSelect) ); sqlite3VdbeAddOp2(v, OP_IsNull, r1, destIfNull); VdbeCoverage(v); + if( destIfNull!=destIfFalse ){ + regCkNull = sqlite3GetTempReg(pParse); + sqlite3VdbeAddOp2(v, OP_Integer, 0, regCkNull); + } + for(ii=0; iinExpr; ii++){ + r2 = sqlite3ExprCodeTemp(pParse, pList->a[ii].pExpr, ®ToFree); + if( regCkNull ){ + sqlite3VdbeAddOp3(v, OP_BitAnd, regCkNull, r2, regCkNull); + } + if( iinExpr-1 || destIfNull!=destIfFalse ){ + sqlite3VdbeAddOp4(v, OP_Eq, r1, labelOk, r2, + (void*)pColl, P4_COLLSEQ); VdbeCoverage(v); + sqlite3VdbeChangeP5(v, affinity); + }else{ + assert( destIfNull==destIfFalse ); + sqlite3VdbeAddOp4(v, OP_Ne, r1, destIfFalse, r2, + (void*)pColl, P4_COLLSEQ); VdbeCoverage(v); + sqlite3VdbeChangeP5(v, affinity | SQLITE_JUMPIFNULL); + } + sqlite3ReleaseTempReg(pParse, regToFree); + } + if( regCkNull ){ + sqlite3VdbeAddOp2(v, OP_IsNull, regCkNull, destIfNull); VdbeCoverage(v); + sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfFalse); + } + sqlite3VdbeResolveLabel(v, labelOk); + sqlite3ReleaseTempReg(pParse, regCkNull); }else{ - int addr1 = sqlite3VdbeAddOp1(v, OP_NotNull, r1); VdbeCoverage(v); - sqlite3VdbeAddOp2(v, OP_Rewind, pExpr->iTable, destIfFalse); - VdbeCoverage(v); - sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfNull); - sqlite3VdbeJumpHere(v, addr1); - } - - if( eType==IN_INDEX_ROWID ){ - /* In this case, the RHS is the ROWID of table b-tree - */ - sqlite3VdbeAddOp2(v, OP_MustBeInt, r1, destIfFalse); VdbeCoverage(v); - sqlite3VdbeAddOp3(v, OP_NotExists, pExpr->iTable, destIfFalse, r1); - VdbeCoverage(v); - }else{ - /* In this case, the RHS is an index b-tree. - */ - sqlite3VdbeAddOp4(v, OP_Affinity, r1, 1, 0, &affinity, 1); - - /* If the set membership test fails, then the result of the - ** "x IN (...)" expression must be either 0 or NULL. If the set - ** contains no NULL values, then the result is 0. If the set - ** contains one or more NULL values, then the result of the - ** expression is also NULL. + + /* If the LHS is NULL, then the result is either false or NULL depending + ** on whether the RHS is empty or not, respectively. */ - assert( destIfFalse!=destIfNull || rRhsHasNull==0 ); - if( rRhsHasNull==0 ){ - /* This branch runs if it is known at compile time that the RHS - ** cannot contain NULL values. This happens as the result - ** of a "NOT NULL" constraint in the database schema. - ** - ** Also run this branch if NULL is equivalent to FALSE - ** for this particular IN operator. + if( destIfNull==destIfFalse ){ + /* Shortcut for the common case where the false and NULL outcomes are + ** the same. */ + sqlite3VdbeAddOp2(v, OP_IsNull, r1, destIfNull); VdbeCoverage(v); + }else{ + int addr1 = sqlite3VdbeAddOp1(v, OP_NotNull, r1); VdbeCoverage(v); + sqlite3VdbeAddOp2(v, OP_Rewind, pExpr->iTable, destIfFalse); + VdbeCoverage(v); + sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfNull); + sqlite3VdbeJumpHere(v, addr1); + } + + if( eType==IN_INDEX_ROWID ){ + /* In this case, the RHS is the ROWID of table b-tree */ - sqlite3VdbeAddOp4Int(v, OP_NotFound, pExpr->iTable, destIfFalse, r1, 1); + sqlite3VdbeAddOp2(v, OP_MustBeInt, r1, destIfFalse); VdbeCoverage(v); + sqlite3VdbeAddOp3(v, OP_NotExists, pExpr->iTable, destIfFalse, r1); VdbeCoverage(v); }else{ - /* In this branch, the RHS of the IN might contain a NULL and - ** the presence of a NULL on the RHS makes a difference in the - ** outcome. + /* In this case, the RHS is an index b-tree. */ - int j1; - - /* First check to see if the LHS is contained in the RHS. If so, - ** then the answer is TRUE the presence of NULLs in the RHS does - ** not matter. If the LHS is not contained in the RHS, then the - ** answer is NULL if the RHS contains NULLs and the answer is - ** FALSE if the RHS is NULL-free. + sqlite3VdbeAddOp4(v, OP_Affinity, r1, 1, 0, &affinity, 1); + + /* If the set membership test fails, then the result of the + ** "x IN (...)" expression must be either 0 or NULL. If the set + ** contains no NULL values, then the result is 0. If the set + ** contains one or more NULL values, then the result of the + ** expression is also NULL. */ - j1 = sqlite3VdbeAddOp4Int(v, OP_Found, pExpr->iTable, 0, r1, 1); - VdbeCoverage(v); - sqlite3VdbeAddOp2(v, OP_IsNull, rRhsHasNull, destIfNull); - VdbeCoverage(v); - sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfFalse); - sqlite3VdbeJumpHere(v, j1); + assert( destIfFalse!=destIfNull || rRhsHasNull==0 ); + if( rRhsHasNull==0 ){ + /* This branch runs if it is known at compile time that the RHS + ** cannot contain NULL values. This happens as the result + ** of a "NOT NULL" constraint in the database schema. + ** + ** Also run this branch if NULL is equivalent to FALSE + ** for this particular IN operator. + */ + sqlite3VdbeAddOp4Int(v, OP_NotFound, pExpr->iTable, destIfFalse, r1, 1); + VdbeCoverage(v); + }else{ + /* In this branch, the RHS of the IN might contain a NULL and + ** the presence of a NULL on the RHS makes a difference in the + ** outcome. + */ + int j1; + + /* First check to see if the LHS is contained in the RHS. If so, + ** then the answer is TRUE the presence of NULLs in the RHS does + ** not matter. If the LHS is not contained in the RHS, then the + ** answer is NULL if the RHS contains NULLs and the answer is + ** FALSE if the RHS is NULL-free. + */ + j1 = sqlite3VdbeAddOp4Int(v, OP_Found, pExpr->iTable, 0, r1, 1); + VdbeCoverage(v); + sqlite3VdbeAddOp2(v, OP_IsNull, rRhsHasNull, destIfNull); + VdbeCoverage(v); + sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfFalse); + sqlite3VdbeJumpHere(v, j1); + } } } sqlite3ReleaseTempReg(pParse, r1); -- cgit v1.2.3 From a976979b6e1c01dbdfaeaddee5b3c80fc924ef94 Mon Sep 17 00:00:00 2001 From: drh Date: Mon, 4 Aug 2014 16:39:39 +0000 Subject: Refinements to the enhanced IN-operator logic. FossilOrigin-Name: 92ba2821468ecbfac2469161d81c873de67b2243 --- src/expr.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 7565d41b9..86787a9c3 100644 --- a/src/expr.c +++ b/src/expr.c @@ -2006,14 +2006,13 @@ static void sqlite3ExprCodeIN( int regCkNull = 0; int ii; assert( !ExprHasProperty(pExpr, EP_xIsSelect) ); - sqlite3VdbeAddOp2(v, OP_IsNull, r1, destIfNull); VdbeCoverage(v); if( destIfNull!=destIfFalse ){ regCkNull = sqlite3GetTempReg(pParse); - sqlite3VdbeAddOp2(v, OP_Integer, 0, regCkNull); + sqlite3VdbeAddOp3(v, OP_BitAnd, r1, r1, regCkNull); } for(ii=0; iinExpr; ii++){ r2 = sqlite3ExprCodeTemp(pParse, pList->a[ii].pExpr, ®ToFree); - if( regCkNull ){ + if( regCkNull && sqlite3ExprCanBeNull(pList->a[ii].pExpr) ){ sqlite3VdbeAddOp3(v, OP_BitAnd, regCkNull, r2, regCkNull); } if( iinExpr-1 || destIfNull!=destIfFalse ){ @@ -2722,7 +2721,7 @@ int sqlite3ExprCodeTarget(Parse *pParse, Expr *pExpr, int target){ addr = sqlite3VdbeAddOp1(v, op, r1); VdbeCoverageIf(v, op==TK_ISNULL); VdbeCoverageIf(v, op==TK_NOTNULL); - sqlite3VdbeAddOp2(v, OP_AddImm, target, -1); + sqlite3VdbeAddOp2(v, OP_Integer, 0, target); sqlite3VdbeJumpHere(v, addr); break; } -- cgit v1.2.3 From 7248a8b2b95a99d7fb7af7e5a3481f9cfcb95e3e Mon Sep 17 00:00:00 2001 From: drh Date: Mon, 4 Aug 2014 18:50:54 +0000 Subject: Further enhancements to IN-operator processing. FossilOrigin-Name: 7fdf26da1d2f40b80f9e44ff6f5af22ace8f95f3 --- src/expr.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 86787a9c3..6816d560d 100644 --- a/src/expr.c +++ b/src/expr.c @@ -1368,6 +1368,9 @@ int sqlite3ExprCanBeNull(const Expr *p){ case TK_FLOAT: case TK_BLOB: return 0; + case TK_COLUMN: + assert( p->pTab!=0 ); + return p->iColumn>=0 && p->pTab->aCol[p->iColumn].notNull==0; default: return 1; } @@ -2038,16 +2041,18 @@ static void sqlite3ExprCodeIN( /* If the LHS is NULL, then the result is either false or NULL depending ** on whether the RHS is empty or not, respectively. */ - if( destIfNull==destIfFalse ){ - /* Shortcut for the common case where the false and NULL outcomes are - ** the same. */ - sqlite3VdbeAddOp2(v, OP_IsNull, r1, destIfNull); VdbeCoverage(v); - }else{ - int addr1 = sqlite3VdbeAddOp1(v, OP_NotNull, r1); VdbeCoverage(v); - sqlite3VdbeAddOp2(v, OP_Rewind, pExpr->iTable, destIfFalse); - VdbeCoverage(v); - sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfNull); - sqlite3VdbeJumpHere(v, addr1); + if( sqlite3ExprCanBeNull(pExpr->pLeft) ){ + if( destIfNull==destIfFalse ){ + /* Shortcut for the common case where the false and NULL outcomes are + ** the same. */ + sqlite3VdbeAddOp2(v, OP_IsNull, r1, destIfNull); VdbeCoverage(v); + }else{ + int addr1 = sqlite3VdbeAddOp1(v, OP_NotNull, r1); VdbeCoverage(v); + sqlite3VdbeAddOp2(v, OP_Rewind, pExpr->iTable, destIfFalse); + VdbeCoverage(v); + sqlite3VdbeAddOp2(v, OP_Goto, 0, destIfNull); + sqlite3VdbeJumpHere(v, addr1); + } } if( eType==IN_INDEX_ROWID ){ -- cgit v1.2.3 From 4336b0e64a42bbf3f7a97511805a300797c681fd Mon Sep 17 00:00:00 2001 From: drh Date: Tue, 5 Aug 2014 00:53:51 +0000 Subject: Improved VdbeCoverage() macros. A few minor simplifications to generated VDBE code. FossilOrigin-Name: 01f60027ad1841051fa493a646141445f8971357 --- src/expr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 6816d560d..e6ac84db9 100644 --- a/src/expr.c +++ b/src/expr.c @@ -2020,7 +2020,9 @@ static void sqlite3ExprCodeIN( } if( iinExpr-1 || destIfNull!=destIfFalse ){ sqlite3VdbeAddOp4(v, OP_Eq, r1, labelOk, r2, - (void*)pColl, P4_COLLSEQ); VdbeCoverage(v); + (void*)pColl, P4_COLLSEQ); + VdbeCoverageIf(v, iinExpr-1); + VdbeCoverageIf(v, ii==pList->nExpr-1); sqlite3VdbeChangeP5(v, affinity); }else{ assert( destIfNull==destIfFalse ); -- cgit v1.2.3 From 1cfc9aa993ee0210d71b6c58c81f2a20c5590b7e Mon Sep 17 00:00:00 2001 From: drh Date: Tue, 5 Aug 2014 21:31:08 +0000 Subject: Ensure that aggregate functions are not used when evaluating a default value for a table column. Candidate fix for ticket [3a88d85f36704eebe134f7]. FossilOrigin-Name: 29ba812825bf06ef230f2480bba0579653f0a52d --- src/expr.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index e6ac84db9..9eb893d26 100644 --- a/src/expr.c +++ b/src/expr.c @@ -2768,6 +2768,11 @@ int sqlite3ExprCodeTarget(Parse *pParse, Expr *pExpr, int target){ sqlite3ErrorMsg(pParse, "unknown function: %.*s()", nId, zId); break; } + if( pDef->xFunc==0 ){ + sqlite3ErrorMsg(pParse, "misuse of aggregate function: %.*s()", + nId, zId); + break; + } /* Attempt a direct implementation of the built-in COALESCE() and ** IFNULL() functions. This avoids unnecessary evalation of -- cgit v1.2.3 From 0c4de2d96de879f19e4ff8f952a82713c9a9c18b Mon Sep 17 00:00:00 2001 From: drh Date: Wed, 6 Aug 2014 00:29:06 +0000 Subject: A simpler fix for ticket [3a88d85f36704eebe1] - one that uses less code. The error message is not quite as good, but as this error has apparently not previously occurred in over 8 years of heavy use, that is not seen as a serious problem. FossilOrigin-Name: 0ad1ed8ef0b5fb5d8db44479373b2b93d8fcfd66 --- src/expr.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'src/expr.c') diff --git a/src/expr.c b/src/expr.c index 9eb893d26..0d2292e94 100644 --- a/src/expr.c +++ b/src/expr.c @@ -2764,15 +2764,10 @@ int sqlite3ExprCodeTarget(Parse *pParse, Expr *pExpr, int target){ zId = pExpr->u.zToken; nId = sqlite3Strlen30(zId); pDef = sqlite3FindFunction(db, zId, nId, nFarg, enc, 0); - if( pDef==0 ){ + if( pDef==0 || pDef->xFunc==0 ){ sqlite3ErrorMsg(pParse, "unknown function: %.*s()", nId, zId); break; } - if( pDef->xFunc==0 ){ - sqlite3ErrorMsg(pParse, "misuse of aggregate function: %.*s()", - nId, zId); - break; - } /* Attempt a direct implementation of the built-in COALESCE() and ** IFNULL() functions. This avoids unnecessary evalation of -- cgit v1.2.3