Correctly handle NULLs in IN operators. Ticket #2273.

The changes in where.c and in the WhereLevel.aInLoop structure are
not strictly necessary to fix this problem - they just make the code
easier to read.  Only the change in OP_Next/OP_Prev operator of vdbe.c
is required. (CVS 3735)

FossilOrigin-Name: 26348556d824c032851e409ac510cddb55c200bf
This commit is contained in:
drh 2007-03-28 14:30:06 +00:00
parent 930cc5864e
commit 72e8fa42f9
6 changed files with 123 additions and 51 deletions

View File

@ -1,5 +1,5 @@
C Update\scomments\sin\ssqlite3.h.\s\sNo\schanges\sto\scode.\s(CVS\s3734)
D 2007-03-28T13:07:41
C Correctly\shandle\sNULLs\sin\sIN\soperators.\s\sTicket\s#2273.\nThe\schanges\sin\swhere.c\sand\sin\sthe\sWhereLevel.aInLoop\sstructure\sare\nnot\sstrictly\snecessary\sto\sfix\sthis\sproblem\s-\sthey\sjust\smake\sthe\scode\neasier\sto\sread.\s\sOnly\sthe\schange\sin\sOP_Next/OP_Prev\soperator\sof\svdbe.c\nis\srequired.\s(CVS\s3735)
D 2007-03-28T14:30:07
F Makefile.in 1fe3d0b46e40fd684e1e61f8e8056cefed16de9f
F Makefile.linux-gcc 2d8574d1ba75f129aba2019f0b959db380a90935
F README 9c4e2d6706bdcc3efdd773ce752a8cdab4f90028
@ -98,7 +98,7 @@ F src/server.c 087b92a39d883e3fa113cae259d64e4c7438bc96
F src/shell.c 3ae4654560e91220a95738a73d135d91d937cda1
F src/sqlite.h.in 02d1159bc8f7387008df9766c79038fce8a9d3a7
F src/sqlite3ext.h 832e13de075d920e2c76584e2b7af1054bb212df
F src/sqliteInt.h 7a3d16cd517dfce73eeac10963275454d6421f82
F src/sqliteInt.h ace601a40db30d667422c79a7db57e7dabb48038
F src/table.c 6d0da66dde26ee75614ed8f584a1996467088d06
F src/tclsqlite.c cd2b3b86ab07c0e0779f6c6e71e72c6c7dc1e704
F src/test1.c 439bba8da10fbc61c731019cf2894e6057578878
@ -122,7 +122,7 @@ F src/update.c 876b3142b8db9ce6ddf8374a2ced65e576acabc7
F src/utf.c 67ecb1032bc0b42c105e88d65ef9d9f626eb0e1f
F src/util.c 8e8180ee5597f2474c1da311ff3c464b6966c0f1
F src/vacuum.c 8bd895d29e7074e78d4e80f948e35ddc9cf2beef
F src/vdbe.c b68f3a86177fa4d10512a4e276a8961d5c1e2ccf
F src/vdbe.c 9fc4520600f364ec510e7c74733dc2b77430b92a
F src/vdbe.h 0025259af1939fb264a545816c69e4b5b8d52691
F src/vdbeInt.h b2ca85ca8abfbba86e380b06c7673dc81c6784c3
F src/vdbeapi.c 6cff63a5b3a52af04b2bef0f7e27ed7ea6f85183
@ -130,7 +130,7 @@ F src/vdbeaux.c 1a50dce809a2f5c2d32e8096166b59016dab1b45
F src/vdbefifo.c 3ca8049c561d5d67cbcb94dc909ae9bb68c0bf8f
F src/vdbemem.c 58a8be2231b0b13e2e77098debe173e79d22f791
F src/vtab.c 7fbda947e28cbe7adb3ba752a76ca9ef29936750
F src/where.c 0ef9acc43cc696b01a3f1019bc07c8d8e0849999
F src/where.c df68bcbb07add015358a91b8412e2ea19dffdbc6
F tclinstaller.tcl 046e3624671962dc50f0481d7c25b38ef803eb42
F test/aggerror.test a867e273ef9e3d7919f03ef4f0e8c0d2767944f2
F test/all.test 60267b055e82de4fb8b841eabb014bc2f836a4eb
@ -367,7 +367,7 @@ F test/vtab_err.test 224cc80ad700797c48b9cd2c1e0bd7a8517d8609
F test/where.test 1d020f50c77f37b2dbab9766ca959e6e3278ecdb
F test/where2.test 3249d426b3fc7a106713d784e1628307fc308d2e
F test/where3.test 0a30fe9808b0fa01c46d0fcf4fac0bf6cf75bb30
F test/where4.test 3fcf53c5ea7af1db3980b3293c2a45b56605f26a
F test/where4.test b68496500bff496e83e76ae4ffb493b99064eac6
F tool/diffdb.c 7524b1b5df217c20cd0431f6789851a4e0cb191b
F tool/fragck.tcl 5265a95126abcf6ab357f7efa544787e5963f439
F tool/lemon.c d3dff81e31c459dd18784d024a759b7229f66297
@ -442,7 +442,7 @@ F www/tclsqlite.tcl bb0d1357328a42b1993d78573e587c6dcbc964b9
F www/vdbe.tcl 87a31ace769f20d3627a64fa1fade7fed47b90d0
F www/version3.tcl 890248cf7b70e60c383b0e84d77d5132b3ead42b
F www/whentouse.tcl 97e2b5cd296f7d8057e11f44427dea8a4c2db513
P 0f7fdb022ca7c94f7d264192e18b6e2bd1e8cff4
R b9541c43661a32a0d6639e1946203bc0
P 1c2656fdf6176a7365db4e11f4bbf47721da72b4
R ad4f14fb479cf3da73d29eb9f4dc5165
U drh
Z 44f123568a87e2be4a17739c7fa10306
Z 203008a1e44e27fee31001b2c9831dfe

View File

@ -1 +1 @@
1c2656fdf6176a7365db4e11f4bbf47721da72b4
26348556d824c032851e409ac510cddb55c200bf

View File

@ -11,7 +11,7 @@
*************************************************************************
** Internal interface definitions for SQLite.
**
** @(#) $Id: sqliteInt.h,v 1.545 2007/03/27 13:36:37 drh Exp $
** @(#) $Id: sqliteInt.h,v 1.546 2007/03/28 14:30:07 drh Exp $
*/
#ifndef _SQLITEINT_H_
#define _SQLITEINT_H_
@ -1160,12 +1160,16 @@ struct WhereLevel {
int iTabCur; /* The VDBE cursor used to access the table */
int iIdxCur; /* The VDBE cursor used to acesss pIdx */
int brk; /* Jump here to break out of the loop */
int nxt; /* Jump here to start the next IN combination */
int cont; /* Jump here to continue with the next loop cycle */
int top; /* First instruction of interior of the loop */
int op, p1, p2; /* Opcode used to terminate the loop */
int nEq; /* Number of == or IN constraints on this loop */
int nIn; /* Number of IN operators constraining this loop */
int *aInLoop; /* Loop terminators for IN operators */
struct InLoop {
int iCur; /* The VDBE cursor used by this IN operator */
int topAddr; /* Top of the IN loop */
} *aInLoop; /* Information about each nested IN operator */
sqlite3_index_info *pBestIdx; /* Index information for this level */
/* The following field is really not part of the current level. But

View File

@ -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.594 2007/03/27 14:44:51 drh Exp $
** $Id: vdbe.c,v 1.595 2007/03/28 14:30:07 drh Exp $
*/
#include "sqliteInt.h"
#include "os.h"
@ -3677,7 +3677,9 @@ case OP_Next: { /* no-push */
CHECK_FOR_INTERRUPT;
assert( pOp->p1>=0 && pOp->p1<p->nCursor );
pC = p->apCsr[pOp->p1];
assert( pC!=0 );
if( pC==0 ){
break; /* See ticket #2273 */
}
if( (pCrsr = pC->pCursor)!=0 ){
int res;
if( pC->nullRow ){

View File

@ -16,7 +16,7 @@
** so is applicable. Because this module is responsible for selecting
** indices, you might also think of this module as the "query optimizer".
**
** $Id: where.c,v 1.241 2007/03/27 13:36:37 drh Exp $
** $Id: where.c,v 1.242 2007/03/28 14:30:09 drh Exp $
*/
#include "sqliteInt.h"
@ -1687,7 +1687,6 @@ static void buildIndexProbe(
static void codeEqualityTerm(
Parse *pParse, /* The parsing context */
WhereTerm *pTerm, /* The term of the WHERE clause to be coded */
int brk, /* Jump here to abandon the loop */
WhereLevel *pLevel /* When level of the FROM clause we are working on */
){
Expr *pX = pTerm->pExpr;
@ -1699,21 +1698,25 @@ static void codeEqualityTerm(
#ifndef SQLITE_OMIT_SUBQUERY
}else{
int iTab;
int *aIn;
struct InLoop *pIn;
assert( pX->op==TK_IN );
sqlite3CodeSubselect(pParse, pX);
iTab = pX->iTable;
sqlite3VdbeAddOp(v, OP_Rewind, iTab, 0);
VdbeComment((v, "# %.*s", pX->span.n, pX->span.z));
if( pLevel->nIn==0 ){
pLevel->nxt = sqlite3VdbeMakeLabel(v);
}
pLevel->nIn++;
pLevel->aInLoop = sqliteReallocOrFree(pLevel->aInLoop,
sizeof(pLevel->aInLoop[0])*2*pLevel->nIn);
aIn = pLevel->aInLoop;
if( aIn ){
aIn += pLevel->nIn*2 - 2;
aIn[0] = iTab;
aIn[1] = sqlite3VdbeAddOp(v, OP_Column, iTab, 0);
sizeof(pLevel->aInLoop[0])*pLevel->nIn);
pIn = pLevel->aInLoop;
if( pIn ){
pIn += pLevel->nIn - 1;
pIn->iCur = iTab;
pIn->topAddr = sqlite3VdbeAddOp(v, OP_Column, iTab, 0);
sqlite3VdbeAddOp(v, OP_IsNull, -1, 0);
}else{
pLevel->nIn = 0;
}
@ -1749,8 +1752,7 @@ static void codeAllEqualityTerms(
Parse *pParse, /* Parsing context */
WhereLevel *pLevel, /* Which nested loop of the FROM we are coding */
WhereClause *pWC, /* The WHERE clause */
Bitmask notReady, /* Which parts of FROM have not yet been coded */
int brk /* Jump here to end the loop */
Bitmask notReady /* Which parts of FROM have not yet been coded */
){
int nEq = pLevel->nEq; /* The number of == or IN constraints to code */
int termsInMem = 0; /* If true, store value in mem[] cells */
@ -1779,9 +1781,9 @@ static void codeAllEqualityTerms(
pTerm = findTerm(pWC, iCur, k, notReady, pLevel->flags, pIdx);
if( pTerm==0 ) break;
assert( (pTerm->flags & TERM_CODED)==0 );
codeEqualityTerm(pParse, pTerm, brk, pLevel);
if( (pTerm->eOperator & WO_ISNULL)==0 ){
sqlite3VdbeAddOp(v, OP_IsNull, termsInMem ? -1 : -(j+1), brk);
codeEqualityTerm(pParse, pTerm, pLevel);
if( (pTerm->eOperator & (WO_ISNULL|WO_IN))==0 ){
sqlite3VdbeAddOp(v, OP_IsNull, termsInMem ? -1 : -(j+1), pLevel->brk);
}
if( termsInMem ){
sqlite3VdbeAddOp(v, OP_MemStore, pLevel->iMem+j+1, 1);
@ -2184,6 +2186,7 @@ WhereInfo *sqlite3WhereBegin(
int j;
int iCur = pTabItem->iCursor; /* The VDBE cursor for the table */
Index *pIdx; /* The index we will be using */
int nxt; /* Where to jump to continue with the next IN case */
int iIdxCur; /* The VDBE cursor for the index */
int omitTable; /* True if we use the index only */
int bRev; /* True if we need to scan in reverse order */
@ -2199,8 +2202,13 @@ WhereInfo *sqlite3WhereBegin(
** for the current loop. Jump to brk to break out of a loop.
** Jump to cont to go immediately to the next iteration of the
** loop.
**
** When there is an IN operator, we also have a "nxt" label that
** means to continue with the next IN value combination. When
** there are no IN operators in the constraints, the "nxt" label
** is the same as "brk".
*/
brk = pLevel->brk = sqlite3VdbeMakeLabel(v);
brk = pLevel->brk = pLevel->nxt = sqlite3VdbeMakeLabel(v);
cont = pLevel->cont = sqlite3VdbeMakeLabel(v);
/* If this is the right table of a LEFT OUTER JOIN, allocate and
@ -2266,9 +2274,10 @@ WhereInfo *sqlite3WhereBegin(
assert( pTerm->pExpr!=0 );
assert( pTerm->leftCursor==iCur );
assert( omitTable==0 );
codeEqualityTerm(pParse, pTerm, brk, pLevel);
sqlite3VdbeAddOp(v, OP_MustBeInt, 1, brk);
sqlite3VdbeAddOp(v, OP_NotExists, iCur, brk);
codeEqualityTerm(pParse, pTerm, pLevel);
nxt = pLevel->nxt;
sqlite3VdbeAddOp(v, OP_MustBeInt, 1, nxt);
sqlite3VdbeAddOp(v, OP_NotExists, iCur, nxt);
VdbeComment((v, "pk"));
pLevel->op = OP_Noop;
}else if( pLevel->flags & WHERE_ROWID_RANGE ){
@ -2347,7 +2356,7 @@ WhereInfo *sqlite3WhereBegin(
/* Generate code to evaluate all constraint terms using == or IN
** and level the values of those terms on the stack.
*/
codeAllEqualityTerms(pParse, pLevel, &wc, notReady, brk);
codeAllEqualityTerms(pParse, pLevel, &wc, notReady);
/* Duplicate the equality term values because they will all be
** used twice: once to make the termination key and once to make the
@ -2378,6 +2387,7 @@ WhereInfo *sqlite3WhereBegin(
** 2002-Dec-04: On a reverse-order scan, the so-called "termination"
** key computed here really ends up being the start key.
*/
nxt = pLevel->nxt;
if( topLimit ){
Expr *pX;
int k = pIdx->aiColumn[j];
@ -2386,7 +2396,7 @@ WhereInfo *sqlite3WhereBegin(
pX = pTerm->pExpr;
assert( (pTerm->flags & TERM_CODED)==0 );
sqlite3ExprCode(pParse, pX->pRight);
sqlite3VdbeAddOp(v, OP_IsNull, -(nEq+1), brk);
sqlite3VdbeAddOp(v, OP_IsNull, -(nEq+1), nxt);
topEq = pTerm->eOperator & (WO_LE|WO_GE);
disableTerm(pLevel, pTerm);
testOp = OP_IdxGE;
@ -2400,7 +2410,7 @@ WhereInfo *sqlite3WhereBegin(
buildIndexProbe(v, nCol, pIdx);
if( bRev ){
int op = topEq ? OP_MoveLe : OP_MoveLt;
sqlite3VdbeAddOp(v, op, iIdxCur, brk);
sqlite3VdbeAddOp(v, op, iIdxCur, nxt);
}else{
sqlite3VdbeAddOp(v, OP_MemStore, pLevel->iMem, 1);
}
@ -2425,7 +2435,7 @@ WhereInfo *sqlite3WhereBegin(
pX = pTerm->pExpr;
assert( (pTerm->flags & TERM_CODED)==0 );
sqlite3ExprCode(pParse, pX->pRight);
sqlite3VdbeAddOp(v, OP_IsNull, -(nEq+1), brk);
sqlite3VdbeAddOp(v, OP_IsNull, -(nEq+1), nxt);
btmEq = pTerm->eOperator & (WO_LE|WO_GE);
disableTerm(pLevel, pTerm);
}else{
@ -2440,7 +2450,7 @@ WhereInfo *sqlite3WhereBegin(
testOp = OP_IdxLT;
}else{
int op = btmEq ? OP_MoveGe : OP_MoveGt;
sqlite3VdbeAddOp(v, op, iIdxCur, brk);
sqlite3VdbeAddOp(v, op, iIdxCur, nxt);
}
}else if( bRev ){
testOp = OP_Noop;
@ -2455,7 +2465,7 @@ WhereInfo *sqlite3WhereBegin(
start = sqlite3VdbeCurrentAddr(v);
if( testOp!=OP_Noop ){
sqlite3VdbeAddOp(v, OP_MemLoad, pLevel->iMem, 0);
sqlite3VdbeAddOp(v, testOp, iIdxCur, brk);
sqlite3VdbeAddOp(v, testOp, iIdxCur, nxt);
if( (topEq && !bRev) || (!btmEq && bRev) ){
sqlite3VdbeChangeP3(v, -1, "+", P3_STATIC);
}
@ -2484,7 +2494,8 @@ WhereInfo *sqlite3WhereBegin(
/* Generate code to evaluate all constraint terms using == or IN
** and leave the values of those terms on the stack.
*/
codeAllEqualityTerms(pParse, pLevel, &wc, notReady, brk);
codeAllEqualityTerms(pParse, pLevel, &wc, notReady);
nxt = pLevel->nxt;
/* Generate a single key that will be used to both start and terminate
** the search
@ -2493,21 +2504,21 @@ WhereInfo *sqlite3WhereBegin(
sqlite3VdbeAddOp(v, OP_MemStore, pLevel->iMem, 0);
/* Generate code (1) to move to the first matching element of the table.
** Then generate code (2) that jumps to "brk" after the cursor is past
** Then generate code (2) that jumps to "nxt" after the cursor is past
** the last matching element of the table. The code (1) is executed
** once to initialize the search, the code (2) is executed before each
** iteration of the scan to see if the scan has finished. */
if( bRev ){
/* Scan in reverse order */
sqlite3VdbeAddOp(v, OP_MoveLe, iIdxCur, brk);
sqlite3VdbeAddOp(v, OP_MoveLe, iIdxCur, nxt);
start = sqlite3VdbeAddOp(v, OP_MemLoad, pLevel->iMem, 0);
sqlite3VdbeAddOp(v, OP_IdxLT, iIdxCur, brk);
sqlite3VdbeAddOp(v, OP_IdxLT, iIdxCur, nxt);
pLevel->op = OP_Prev;
}else{
/* Scan in the forward order */
sqlite3VdbeAddOp(v, OP_MoveGe, iIdxCur, brk);
sqlite3VdbeAddOp(v, OP_MoveGe, iIdxCur, nxt);
start = sqlite3VdbeAddOp(v, OP_MemLoad, pLevel->iMem, 0);
sqlite3VdbeOp3(v, OP_IdxGE, iIdxCur, brk, "+", P3_STATIC);
sqlite3VdbeOp3(v, OP_IdxGE, iIdxCur, nxt, "+", P3_STATIC);
pLevel->op = OP_Next;
}
if( !omitTable ){
@ -2640,16 +2651,18 @@ void sqlite3WhereEnd(WhereInfo *pWInfo){
if( pLevel->op!=OP_Noop ){
sqlite3VdbeAddOp(v, pLevel->op, pLevel->p1, pLevel->p2);
}
sqlite3VdbeResolveLabel(v, pLevel->brk);
if( pLevel->nIn ){
int *a;
struct InLoop *pIn;
int j;
for(j=pLevel->nIn, a=&pLevel->aInLoop[j*2-2]; j>0; j--, a-=2){
sqlite3VdbeAddOp(v, OP_Next, a[0], a[1]);
sqlite3VdbeJumpHere(v, a[1]-1);
sqlite3VdbeResolveLabel(v, pLevel->nxt);
for(j=pLevel->nIn, pIn=&pLevel->aInLoop[j-1]; j>0; j--, pIn--){
sqlite3VdbeJumpHere(v, pIn->topAddr+1);
sqlite3VdbeAddOp(v, OP_Next, pIn->iCur, pIn->topAddr);
sqlite3VdbeJumpHere(v, pIn->topAddr-1);
}
sqliteFree(pLevel->aInLoop);
}
sqlite3VdbeResolveLabel(v, pLevel->brk);
if( pLevel->iLeftJoin ){
int addr;
addr = sqlite3VdbeAddOp(v, OP_IfMemPos, pLevel->iLeftJoin, 0);

View File

@ -15,7 +15,7 @@
# that IS NULL phrases are correctly optimized. But you can never
# have too many tests, so some other tests are thrown in as well.
#
# $Id: where4.test,v 1.2 2007/01/25 16:56:08 drh Exp $
# $Id: where4.test,v 1.3 2007/03/28 14:30:09 drh Exp $
set testdir [file dirname $argv0]
source $testdir/tester.tcl
@ -173,7 +173,60 @@ do_test where4-4.4 {
WHERE t1.col1 IS NULL;
}
} {}
# Ticket #2273. Problems with IN operators and NULLs.
#
do_test where4-5.1 {
execsql {
CREATE TABLE t4(x,y,z,PRIMARY KEY(x,y));
}
execsql {
SELECT *
FROM t2 LEFT JOIN t4 b1
LEFT JOIN t4 b2 ON b2.x=b1.x AND b2.y IN (b1.y);
}
} {1 {} {} {} {} {} {} 2 {} {} {} {} {} {} 3 {} {} {} {} {} {}}
do_test where4-5.2 {
execsql {
INSERT INTO t4 VALUES(1,1,11);
INSERT INTO t4 VALUES(1,2,12);
INSERT INTO t4 VALUES(1,3,13);
INSERT INTO t4 VALUES(2,2,22);
SELECT rowid FROM t4 WHERE x IN (1,9,2,5) AND y IN (1,3,NULL,2) AND z!=13;
}
} {1 2 4}
do_test where4-5.3 {
execsql {
SELECT rowid FROM t4 WHERE x IN (1,9,NULL,2) AND y IN (1,3,2) AND z!=13;
}
} {1 2 4}
do_test where4-6.1 {
execsql {
CREATE TABLE t5(a,b,c,d,e,f,UNIQUE(a,b,c,d,e,f));
INSERT INTO t5 VALUES(1,1,1,1,1,11111);
INSERT INTO t5 VALUES(2,2,2,2,2,22222);
INSERT INTO t5 VALUES(1,2,3,4,5,12345);
INSERT INTO t5 VALUES(2,3,4,5,6,23456);
}
execsql {
SELECT rowid FROM t5
WHERE a IN (1,9,2) AND b=2 AND c IN (1,2,3,4) AND d>0
}
} {3 2}
do_test where4-6.2 {
execsql {
SELECT rowid FROM t5
WHERE a IN (1,NULL,2) AND b=2 AND c IN (1,2,3,4) AND d>0
}
} {3 2}
do_test where4-7.1 {
execsql {
CREATE TABLE t6(y,z,PRIMARY KEY(y,z));
}
execsql {
SELECT * FROM t6 WHERE y=NULL AND z IN ('hello');
}
} {}
integrity_check {where4-99.0}