From db04571c79364fcfffc2dfd7fc655dc9d1b7d071 Mon Sep 17 00:00:00 2001 From: dan Date: Mon, 21 Mar 2011 19:41:29 +0000 Subject: [PATCH] Remove some unreachable code in sqlite3session.c. Add test cases. FossilOrigin-Name: 39cdfa5324ae91bfbbac733b1e3e2d33ca883340 --- ext/session/session1.test | 1 - ext/session/session2.test | 16 +++- ext/session/session_common.tcl | 6 +- ext/session/sessionfault.test | 52 +++++++++++ ext/session/sqlite3session.c | 164 ++++++++++++++------------------- manifest | 32 +++---- manifest.uuid | 2 +- 7 files changed, 150 insertions(+), 123 deletions(-) diff --git a/ext/session/session1.test b/ext/session/session1.test index fd09b47cc5..1d8b1921b9 100644 --- a/ext/session/session1.test +++ b/ext/session/session1.test @@ -188,7 +188,6 @@ proc do_conflict_test {tn args} { sqlite3session S db main foreach t $O(-tables) { S attach $t } execsql $O(-sql) - set ::xConflict [list] sqlite3changeset_apply db2 [S changeset] xConflict diff --git a/ext/session/session2.test b/ext/session/session2.test index 9065df99a0..984559bb12 100644 --- a/ext/session/session2.test +++ b/ext/session/session2.test @@ -150,6 +150,13 @@ set set_of_tests { INSERT INTO %T1% SELECT a+8, b+8 FROM %T1%; INSERT INTO %T1% SELECT a+256, b+256 FROM %T1%; } + + 16 { + INSERT INTO %T4% VALUES('abc', 'def'); + INSERT INTO %T4% VALUES('def', 'abc'); + } + 17 { UPDATE %T4% SET b = 1 } + 18 { DELETE FROM %T4% WHERE 1 } } test_reset @@ -157,9 +164,10 @@ do_common_sql { CREATE TABLE t1(a PRIMARY KEY, b); CREATE TABLE t2(a, b INTEGER PRIMARY KEY); CREATE TABLE t3(a, b, c, PRIMARY KEY(a, b)); + CREATE TABLE t4(a, b, PRIMARY KEY(b, a)); } -foreach {tn sql} [string map {%T1% t1 %T2% t2 %T3% t3} $set_of_tests] { +foreach {tn sql} [string map {%T1% t1 %T2% t2 %T3% t3 %T4% t4} $set_of_tests] { do_then_apply_sql $sql do_test 2.$tn { compare_db db db2 } {} } @@ -182,21 +190,23 @@ do_test 3.0 { CREATE TABLE aux.t1(a PRIMARY KEY, b); CREATE TABLE aux.t2(a, b INTEGER PRIMARY KEY); CREATE TABLE aux.t3(a, b, c, PRIMARY KEY(a, b)); + CREATE TABLE aux.t4(a, b, PRIMARY KEY(b, a)); } execsql { CREATE TABLE t1(a PRIMARY KEY, b); CREATE TABLE t2(a, b INTEGER PRIMARY KEY); CREATE TABLE t3(a, b, c, PRIMARY KEY(a, b)); + CREATE TABLE t4(a, b, PRIMARY KEY(b, a)); } db2 } {} proc xTrace {args} { puts $args } foreach {tn sql} [ - string map {%T1% aux.t1 %T2% aux.t2 %T3% aux.t3} $set_of_tests + string map {%T1% aux.t1 %T2% aux.t2 %T3% aux.t3 %T4% aux.t4} $set_of_tests ] { do_then_apply_sql $sql aux - do_test 3.$tn { compare_db db3 db2 } {} + do_test 3.$tn { compare_db db2 db3 } {} } catch {db3 close} diff --git a/ext/session/session_common.tcl b/ext/session/session_common.tcl index 1b5ebc5b97..c1a6440396 100644 --- a/ext/session/session_common.tcl +++ b/ext/session/session_common.tcl @@ -83,7 +83,11 @@ proc compare_db {db1 db2} { set lot1 [$db1 eval $sql] set lot2 [$db2 eval $sql] - if {$lot1 != $lot2} { error "databases contain different tables" } + if {$lot1 != $lot2} { + puts $lot1 + puts $lot2 + error "databases contain different tables" + } foreach tbl $lot1 { set col1 [list] diff --git a/ext/session/sessionfault.test b/ext/session/sessionfault.test index d6dd361b00..8f38fe63e8 100644 --- a/ext/session/sessionfault.test +++ b/ext/session/sessionfault.test @@ -82,4 +82,56 @@ do_faultsim_test pagerfault-2 -faults oom-* -prep { faultsim_integrity_check } +catch { db close } +catch { db2 close } +forcedelete test.db2 test.db +sqlite3 db2 test.db2 +sqlite3 db test.db + +proc xConflict {op tbl type args} { + if { $type=="CONFLICT" || $type=="DATA" } { + return "REPLACE" + } + return "OMIT" +} + +do_test 3.0 { + execsql { + PRAGMA encoding = 'utf16'; + CREATE TABLE t1(a PRIMARY KEY, b); + INSERT INTO t1 VALUES(5, 32); + } + execsql { + PRAGMA encoding = 'utf16'; + CREATE TABLE t1(a PRIMARY KEY, b NOT NULL); + INSERT INTO t1 VALUES(1, 2); + INSERT INTO t1 VALUES(2, 4); + INSERT INTO t1 VALUES(4, 16); + } db2 +} {} + +faultsim_save_and_close +db2 close + +do_faultsim_test pagerfault-3 -faults oom-transient -prep { + catch {db2 close} + catch {db close} + faultsim_restore_and_reopen + sqlite3 db2 test.db2 + sqlite3session S db main + S attach t1 + execsql { + INSERT INTO t1 VALUES(1, 45); + INSERT INTO t1 VALUES(2, 55); + INSERT INTO t1 VALUES(3, 55); + UPDATE t1 SET a = 4 WHERE a = 5; + } +} -body { + sqlite3changeset_apply db2 [S changeset] xConflict +} -test { + catch { S delete } + faultsim_test_result {0 {}} {1 SQLITE_NOMEM} + if {$testrc==0} { compare_db db db2 } +} + finish_test diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c index e6d4675b08..3c6a972961 100644 --- a/ext/session/sqlite3session.c +++ b/ext/session/sqlite3session.c @@ -552,11 +552,10 @@ static int sessionTableInfo( int nThis; int i; u8 *pAlloc; - u8 *pFree = 0; - char **azCol; + char **azCol = 0; u8 *abPK; - assert( pazCol || pabPK ); + assert( pazCol && pabPK ); nThis = strlen(zThis); zPragma = sqlite3_mprintf("PRAGMA '%q'.table_info('%q')", zDb, zThis); @@ -584,15 +583,10 @@ static int sessionTableInfo( } } if( rc==SQLITE_OK ){ - pFree = pAlloc; - if( pazCol ){ - azCol = (char **)pAlloc; - pAlloc = (u8 *)&azCol[nCol]; - } - if( pabPK ){ - abPK = (u8 *)pAlloc; - pAlloc = &abPK[nCol]; - } + azCol = (char **)pAlloc; + pAlloc = (u8 *)&azCol[nCol]; + abPK = (u8 *)pAlloc; + pAlloc = &abPK[nCol]; if( pzTab ){ memcpy(pAlloc, zThis, nThis+1); *pzTab = (char *)pAlloc; @@ -604,12 +598,10 @@ static int sessionTableInfo( int nName = sqlite3_column_bytes(pStmt, 1); const unsigned char *zName = sqlite3_column_text(pStmt, 1); if( zName==0 ) break; - if( pazCol ){ - memcpy(pAlloc, zName, nName+1); - azCol[i] = (char *)pAlloc; - pAlloc += nName+1; - } - if( pabPK ) abPK[i] = sqlite3_column_int(pStmt, 5); + memcpy(pAlloc, zName, nName+1); + azCol[i] = (char *)pAlloc; + pAlloc += nName+1; + abPK[i] = sqlite3_column_int(pStmt, 5); i++; } rc = sqlite3_reset(pStmt); @@ -620,13 +612,13 @@ static int sessionTableInfo( ** free any allocation made. An error code will be returned in this case. */ if( rc==SQLITE_OK ){ - if( pazCol ) *pazCol = (const char **)azCol; - if( pabPK ) *pabPK = abPK; + *pazCol = (const char **)azCol; + *pabPK = abPK; }else{ - if( pazCol ) *pazCol = 0; - if( pabPK ) *pabPK = 0; + *pazCol = 0; + *pabPK = 0; if( pzTab ) *pzTab = 0; - sqlite3_free(pFree); + sqlite3_free(azCol); } sqlite3_finalize(pStmt); return rc; @@ -1785,7 +1777,7 @@ struct SessionApplyCtx { ** ** The DELETE statement looks like this: ** -** DELETE FROM x WHERE a = :1 AND c = :3 AND :5 OR (b IS :2 AND d IS :4) +** DELETE FROM x WHERE a = :1 AND c = :3 AND (:5 OR b IS :2 AND d IS :4) ** ** Variable :5 (nCol+1) is a boolean. It should be set to 0 if we require ** matching b and d values, or 1 otherwise. The second case comes up if the @@ -1997,6 +1989,41 @@ static int sessionInsertRow( return rc; } +/* +** Iterator pIter must point to an SQLITE_INSERT entry. This function +** transfers new.* values from the current iterator entry to statement +** pStmt. The table being inserted into has nCol columns. +** +** New.* value $i 0 from the iterator is bound to variable ($i+1) of +** statement pStmt. If parameter abPK is NULL, all values from 0 to (nCol-1) +** are transfered to the statement. Otherwise, if abPK is not NULL, it points +** to an array nCol elements in size. In this case only those values for +** which abPK[$i] is true are read from the iterator and bound to the +** statement. +** +** An SQLite error code is returned if an error occurs. Otherwise, SQLITE_OK. +*/ +static int sessionBindValues( + sqlite3_changeset_iter *pIter, /* Iterator to read values from */ + int(*xIterValue)(sqlite3_changeset_iter *, int, sqlite3_value **), + int nCol, /* Number of columns */ + u8 *abPK, /* If not NULL, bind only if true */ + sqlite3_stmt *pStmt /* Bind values to this statement */ +){ + int i; + int rc = SQLITE_OK; + for(i=0; rc==SQLITE_OK && idb, pIter, p->abPK, p->pSelect); }else{ - rc = SQLITE_DONE; + rc = SQLITE_OK; } if( rc==SQLITE_ROW ){ @@ -2117,7 +2130,7 @@ static int sessionConflictHandler( res = xConflict(pCtx, eType, pIter); pIter->pConflict = 0; rc = sqlite3_reset(p->pSelect); - }else{ + }else if( rc==SQLITE_OK ){ /* No other row with the new.* primary key. */ rc = sqlite3_reset(p->pSelect); if( rc==SQLITE_OK ){ @@ -2192,16 +2205,9 @@ static int sessionApplyOneOp( sqlite3changeset_op(pIter, &zDummy, &nCol, &op); if( op==SQLITE_DELETE ){ - int i; /* Bind values to the DELETE statement. */ - for(i=0; rc==SQLITE_OK && ipDelete, i+1, pVal); - } - } + rc = sessionBindValues(pIter, sqlite3changeset_old, nCol, 0, p->pDelete); if( rc==SQLITE_OK && sqlite3_bind_parameter_count(p->pDelete)>nCol ){ rc = sqlite3_bind_int(p->pDelete, nCol+1, pbRetry==0); } @@ -2254,44 +2260,20 @@ static int sessionApplyOneOp( ); }else if( rc==SQLITE_CONSTRAINT ){ - /* This may be a CONSTRAINT or CONFLICT error. It is a CONFLICT if - ** the only problem is a duplicate PRIMARY KEY, or a CONSTRAINT - ** otherwise. */ - int bPKChange = 0; - - /* Check if the PK has been modified. */ - rc = SQLITE_OK; - for(i=0; iabPK[i] ){ - sqlite3_value *pNew; - rc = sqlite3changeset_new(pIter, i, &pNew); - if( rc==SQLITE_OK && pNew ){ - bPKChange = 1; - break; - } - } - } - - rc = sessionConflictHandler(SQLITE_CHANGESET_CONFLICT, - p, pIter, xConflict, pCtx, (bPKChange ? pbReplace : 0) + /* This is always a CONSTRAINT conflict. */ + rc = sessionConflictHandler( + SQLITE_CHANGESET_CONFLICT, p, pIter, xConflict, pCtx, 0 ); } }else{ - int i; assert( op==SQLITE_INSERT ); - for(i=0; rc==SQLITE_OK && ipInsert, i+1, pVal); - } - } + rc = sessionBindValues(pIter, sqlite3changeset_new, nCol, 0, p->pInsert); if( rc!=SQLITE_OK ) return rc; sqlite3_step(p->pInsert); rc = sqlite3_reset(p->pInsert); - if( rc==SQLITE_CONSTRAINT && xConflict ){ + if( rc==SQLITE_CONSTRAINT ){ rc = sessionConflictHandler( SQLITE_CHANGESET_CONFLICT, p, pIter, xConflict, pCtx, pbReplace ); @@ -2369,21 +2351,11 @@ int sqlite3changeset_apply( } if( bReplace ){ + assert( pIter->op==SQLITE_INSERT ); rc = sqlite3_exec(db, "SAVEPOINT replace_op", 0, 0, 0); if( rc==SQLITE_OK ){ - int i; - for(i=0; i