From: dan Date: Tue, 2 Jul 2019 20:10:02 +0000 (+0000) Subject: Have ALTER TABLE detect and error out for the case where renaming a column changes... X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c1158219473f58915b58f1b37eeaaa8f401de1ca;p=thirdparty%2Fsqlite.git Have ALTER TABLE detect and error out for the case where renaming a column changes a the interpretation of a double-quoted identifier in the database schema from an SQL literal to a column reference. FossilOrigin-Name: 5dbb0734aff25e47a58a64abe0e96636cbf8a7a5006df0cfe10af8629af9133c --- diff --git a/manifest b/manifest index a320ae1e91..cabbcab55b 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Use\sthe\sOP_Sequence\sopcode\sfor\sgenerating\sunique\srowid\svalues\sfor\san\nautoindex\son\sa\sco-routine\simplementation\sof\sa\ssubquery. -D 2019-06-28T07:08:13.998 +C Have\sALTER\sTABLE\sdetect\sand\serror\sout\sfor\sthe\scase\swhere\srenaming\sa\scolumn\schanges\sa\sthe\sinterpretation\sof\sa\sdouble-quoted\sidentifier\sin\sthe\sdatabase\sschema\sfrom\san\sSQL\sliteral\sto\sa\scolumn\sreference. +D 2019-07-02T20:10:02.773 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724 @@ -456,7 +456,7 @@ F spec.template 86a4a43b99ebb3e75e6b9a735d5fd293a24e90ca F sqlite.pc.in 42b7bf0d02e08b9e77734a47798d1a55a9e0716b F sqlite3.1 fc7ad8990fc8409983309bb80de8c811a7506786 F sqlite3.pc.in 48fed132e7cb71ab676105d2a4dc77127d8c1f3a -F src/alter.c c1b5e5639b88dcc146db326315f2dea4f7f1c599e524eeb421d544927a0b1e86 +F src/alter.c bb45a2df945060a3125009d3d5fa7458b8fd9b123eb7128475aa3db3c688e48c F src/analyze.c 58db66344a5c58dcabb57f26696f6f2993956c830446da40b444051d2fdaf644 F src/attach.c 78e986baee90cb7b83fb9eafa79c22581a8ada14030fd633b0683c95cf11213c F src/auth.c 0fac71038875693a937e506bceb492c5f136dd7b1249fbd4ae70b4e8da14f9df @@ -595,10 +595,10 @@ F src/utf.c 2f0fac345c7660d5c5bd3df9e9d8d33d4c27f366bcfb09e07443064d751a0507 F src/util.c aef606a78b85d042138a841babbc0f98471b19b9a340b962e8fae307bc8cf3da F src/vacuum.c 82dcec9e7b1afa980288718ad11bc499651c722d7b9f32933c4d694d91cb6ebf F src/vdbe.c aaa36d1ac7d55baf007e9c03ee7c826834a51dfe7a56ba4c386318695dd87c99 -F src/vdbe.h 712bca562eaed1c25506b9faf9680bdc75fc42e2f4a1cd518d883fa79c7a4237 +F src/vdbe.h 69a9f51eb37e3548068df660a1a5b9a40ef0ad70c338079da765c54a813ac56f F src/vdbeInt.h 3ba14553508d66f58753952d6dd287dce4ec735de02c6440858b4891aed51c17 F src/vdbeapi.c f9161e5c77f512fbb80091ce8af621d19c9556bda5e734cffaac1198407400da -F src/vdbeaux.c 3a803d75875031309204df90977059b12ffb706d16b4baa5e2d99f4353962582 +F src/vdbeaux.c e078c888b2c3b2f44895f9aad690e23110fc21e88ff5b611aa27f0b11206da71 F src/vdbeblob.c f5c70f973ea3a9e915d1693278a5f890dc78594300cf4d54e64f2b0917c94191 F src/vdbemem.c 9ee3c0373bfc05dc8bf5307a4a92be6bea3055928c4846fdced7e708993b2d6d F src/vdbesort.c 66592d478dbb46f19aed0b42222325eadb84deb40a90eebe25c6e7c1d8468f47 @@ -620,13 +620,14 @@ F test/aggerror.test a867e273ef9e3d7919f03ef4f0e8c0d2767944f2 F test/aggnested.test 18b00de006597e960a6b27ccec51474ac66cf1070a87c1933e5694dc02190ef1 F test/alias.test 4529fbc152f190268a15f9384a5651bbbabc9d87 F test/all.test 2ecb8bbd52416642e41c9081182a8df05d42c75637afd4488aace78cc4b69e13 -F test/alter.test 93dee7c0ff9106fbd53a8bbf519107904b884050a99c4565412c58c37d68c802 +F test/alter.test 673c5e68f5d0603e943be6f94d8444aac9756de9ab9d33d7c3ceaa548e946997 F test/alter2.test a966ccfcddf9ce0a4e0e6ff1aca9e6e7948e0e242cd7e43fc091948521807687 F test/alter3.test 4d79934d812eaeacc6f22781a080f8cfe012fdc3 F test/alter4.test 7e93a21fe131e1dfeb317e90056856f96b10381fc7fe3a05e765569a23400433 F test/alterauth.test 63442ba61ceb0c1eeb63aac1f4f5cebfa509d352276059d27106ae256bafc959 -F test/alterauth2.test c0a1ddf5b93d93cb0d15ba7acaf0c5c6fb515bbe861ede75b2d3fabad33b6499 -F test/altercol.test 54374d2ba18af25bb24e23acf18a60270d4ec120b7ec0558078b59d5aa1a31ad +F test/alterauth2.test 8cb2f1b99127292e58f306779bee5f6491833cf6691da3e36bf532e26090fcca +F test/altercol.test 545a7f2a7375fb5c3f1fcdfad6645b2a21a4aef5a8d4810e689fa3fa75e9e2fe +F test/altercol2.test 4f301f93ee9fb7655ceaa5bb758c9549b56eab3aae94a18fc8e7a07f31e23013 F test/alterlegacy.test 82022721ce0de29cedc9a7af63bc9fcc078b0ee000f8283b4b6ea9c3eab2f44b F test/altermalloc.test 167a47de41b5c638f5f5c6efb59784002b196fff70f98d9b4ed3cd74a3fb80c9 F test/altermalloc2.test fa7b1c1139ea39b8dec407cf1feb032ca8e0076bd429574969b619175ad0174b @@ -1830,7 +1831,10 @@ F vsixtest/vsixtest.tcl 6a9a6ab600c25a91a7acc6293828957a386a8a93 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0 -P 5fd20e09a522b62a529cf4d76fbdf0a09426f67ffa30430cac6b81ebf32ba43e -R d8e38b1a6647f90f85dd30ba60740431 -U drh -Z 069759ad3085e2b5f17cc34ec369471a +P eab4297577e4d325fed4757867fc77860de7448998d86f098c8a50272e17d35e +R 345332b27d681f48c6f6235823b1591a +T *branch * tkt9b78184b-alt +T *sym-tkt9b78184b-alt * +T -sym-trunk * +U dan +Z 4f5c772cb1a84553bf3caf8b00b061e5 diff --git a/manifest.uuid b/manifest.uuid index 82a0a5af46..ae5e68f100 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -eab4297577e4d325fed4757867fc77860de7448998d86f098c8a50272e17d35e \ No newline at end of file +5dbb0734aff25e47a58a64abe0e96636cbf8a7a5006df0cfe10af8629af9133c \ No newline at end of file diff --git a/src/alter.c b/src/alter.c index 3957edefa0..c1bdb6e1c9 100644 --- a/src/alter.c +++ b/src/alter.c @@ -527,6 +527,8 @@ void sqlite3AlterRenameColumn( const char *zDb; /* Name of schema containing the table */ int iSchema; /* Index of the schema */ int bQuote; /* True to quote the new name */ + Vdbe *v; /* VDBE handle */ + int regRefCnt; /* Register for number of refs to column */ /* Locate the table to be altered */ pTab = sqlite3LocateTableItem(pParse, 0, &pSrc->a[0]); @@ -560,10 +562,15 @@ void sqlite3AlterRenameColumn( goto exit_rename_column; } - /* Do the rename operation using a recursive UPDATE statement that - ** uses the sqlite_rename_column() SQL function to compute the new - ** CREATE statement text for the sqlite_master table. - */ + /* Allocate a register to count the number of rewritten column refs. */ + v = sqlite3GetVdbe(pParse); + if( v==0 ) goto exit_rename_column; + regRefCnt = ++pParse->nMem; + sqlite3VdbeAddOp2(v, OP_Integer, 0, regRefCnt); + + /* Do the rename operation using an UPDATE statement that uses the + ** sqlite_rename_column() SQL function to compute the new CREATE + ** statement text for the sqlite_master table. */ sqlite3MayAbort(pParse); zNew = sqlite3NameFromToken(db, pNew); if( !zNew ) goto exit_rename_column; @@ -571,26 +578,65 @@ void sqlite3AlterRenameColumn( bQuote = sqlite3Isquote(pNew->z[0]); sqlite3NestedParse(pParse, "UPDATE \"%w\".%s SET " - "sql = sqlite_rename_column(sql, type, name, %Q, %Q, %d, %Q, %d, %d) " + "sql = sqlite_rename_column(sql, type, name, %Q, %Q, %d, %Q, %d, %d, %d) " "WHERE name NOT LIKE 'sqliteX_%%' ESCAPE 'X' " " AND (type != 'index' OR tbl_name = %Q)" " AND sql NOT LIKE 'create virtual%%'", zDb, MASTER_NAME, - zDb, pTab->zName, iCol, zNew, bQuote, iSchema==1, + zDb, pTab->zName, iCol, zNew, bQuote, iSchema==1, regRefCnt, pTab->zName ); - sqlite3NestedParse(pParse, - "UPDATE temp.%s SET " - "sql = sqlite_rename_column(sql, type, name, %Q, %Q, %d, %Q, %d, 1) " - "WHERE type IN ('trigger', 'view')", - MASTER_NAME, - zDb, pTab->zName, iCol, zNew, bQuote - ); + if( iSchema!=1 ){ + sqlite3NestedParse(pParse, + "UPDATE temp.%s SET " + "sql = sqlite_rename_column(sql, type, name, %Q, %Q, %d, %Q, %d, 1, %d)" + "WHERE type IN ('trigger', 'view')", + MASTER_NAME, + zDb, pTab->zName, iCol, zNew, bQuote, regRefCnt + ); + } /* Drop and reload the database schema. */ renameReloadSchema(pParse, iSchema); - renameTestSchema(pParse, zDb, iSchema==1); + + /* Now arrange for the sqlite_rename_column() function to be called a + ** second time on the already rewritten schema. This serves two purposes: + ** (a) to check that the schema can still be parsed, and (b) to decrement + ** register regRefCnt once for each reference to the renamed column in the + ** schema (the calls above incremented regRefCnt once for each such reference + ** prior to renaming). If register regRefCnt is not left set to 0, then + ** renaming the column has left the schema with either more or fewer + ** references to the column than there was at the beginning of the + ** operation. This is an error, so return an error message and SQLITE_ABORT + ** the statement. */ + sqlite3NestedParse(pParse, + "SELECT 1 " + "FROM \"%w\".%s " + "WHERE name NOT LIKE 'sqliteX_%%' ESCAPE 'X'" + " AND sql NOT LIKE 'create virtual%%'" + " AND (type != 'index' OR tbl_name = %Q)" + " AND sqlite_rename_column(sql,type,name,%Q,%Q,%d,%Q,%d,%d,%d)=NULL", + zDb, MASTER_NAME, + pTab->zName, + zDb, pTab->zName, iCol, zNew, bQuote, iSchema==1, regRefCnt*-1 + ); + if( iSchema!=1 ){ + sqlite3NestedParse(pParse, + "SELECT 1 " + "FROM temp.%s " + "WHERE type IN ('trigger', 'view')" + " AND sqlite_rename_column(sql,type,name,%Q,%Q,%d,%Q,%d,1,%d)=NULL", + MASTER_NAME, + zDb, pTab->zName, iCol, zNew, bQuote, regRefCnt*-1 + ); + } + + sqlite3VdbeAddOp2(v, OP_IfNot, regRefCnt, sqlite3VdbeCurrentAddr(v)+2); + sqlite3VdbeAddOp2(v, OP_Halt, SQLITE_ERROR, OE_Abort); + sqlite3VdbeChangeP4(v, -1, + "schema contains ambiguous double-quoted string", P4_STATIC + ); exit_rename_column: sqlite3SrcListDelete(db, pSrc); @@ -1236,6 +1282,9 @@ static void renameParseCleanup(Parse *pParse){ ** 6. zNew: New column name ** 7. bQuote: Non-zero if the new column name should be quoted. ** 8. bTemp: True if zSql comes from temp schema +** 9. iReg: Register to increment for each rewritten column ref. Or, +** if this parameter is negative, -1 times the id of a register +** to decrement for each rewritten column ref. ** ** Do a column rename operation on the CREATE statement given in zSql. ** The iCol-th column (left-most is 0) of table zTable is renamed from zCol @@ -1259,6 +1308,7 @@ static void renameColumnFunc( const char *zNew = (const char*)sqlite3_value_text(argv[6]); int bQuote = sqlite3_value_int(argv[7]); int bTemp = sqlite3_value_int(argv[8]); + int regRefCnt = sqlite3_value_int(argv[9]); const char *zOld; int rc; Parse sParse; @@ -1367,7 +1417,6 @@ static void renameColumnFunc( } } - /* Find tokens to edit in UPDATE OF clause */ if( sParse.pTriggerTab==pTab ){ renameColumnIdlistNames(&sParse, &sCtx,sParse.pNewTrigger->pColumns,zOld); @@ -1378,12 +1427,16 @@ static void renameColumnFunc( } assert( rc==SQLITE_OK ); + if( regRefCnt!=0 ){ + int mul = (regRefCnt<0 ? -1 : 1); + sqlite3VdbeIncrReg(context, regRefCnt*mul, sCtx.nList*mul); + } rc = renameEditSql(context, &sCtx, zSql, zNew, bQuote); renameColumnFunc_done: if( rc!=SQLITE_OK ){ if( sParse.zErrMsg ){ - renameColumnParseError(context, 0, argv[1], argv[2], &sParse); + renameColumnParseError(context, regRefCnt<0, argv[1], argv[2], &sParse); }else{ sqlite3_result_error_code(context, rc); } @@ -1659,7 +1712,7 @@ static void renameTableTest( */ void sqlite3AlterFunctions(void){ static FuncDef aAlterTableFuncs[] = { - INTERNAL_FUNCTION(sqlite_rename_column, 9, renameColumnFunc), + INTERNAL_FUNCTION(sqlite_rename_column, 10, renameColumnFunc), INTERNAL_FUNCTION(sqlite_rename_table, 7, renameTableFunc), INTERNAL_FUNCTION(sqlite_rename_test, 5, renameTableTest), }; diff --git a/src/vdbe.h b/src/vdbe.h index 041a91c51f..0e27c6b4d2 100644 --- a/src/vdbe.h +++ b/src/vdbe.h @@ -283,6 +283,7 @@ void sqlite3VdbeLinkSubProgram(Vdbe *, SubProgram *); #endif int sqlite3NotPureFunc(sqlite3_context*); +void sqlite3VdbeIncrReg(sqlite3_context*, int, int); /* Use SQLITE_ENABLE_COMMENTS to enable generation of extra comments on ** each VDBE opcode. diff --git a/src/vdbeaux.c b/src/vdbeaux.c index 8dd2b421aa..5ff8779e7f 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -4910,6 +4910,16 @@ int sqlite3NotPureFunc(sqlite3_context *pCtx){ return 1; } +/* +** Increment the value of register iReg, which is guaranteed to be an +** integer, in the VM associated with context object pCtx by iVal. +*/ +void sqlite3VdbeIncrReg(sqlite3_context *pCtx, int iReg, int iVal){ + Mem *pMem = &pCtx->pVdbe->aMem[iReg]; + assert( pMem->flags==MEM_Int ); + pMem->u.i += iVal; +} + #ifndef SQLITE_OMIT_VIRTUALTABLE /* ** Transfer error message text from an sqlite3_vtab.zErrMsg (text stored diff --git a/test/alter.test b/test/alter.test index d20e5ebccd..1c942e867b 100644 --- a/test/alter.test +++ b/test/alter.test @@ -686,7 +686,7 @@ do_test alter-8.2 { # sqlite3_test_control SQLITE_TESTCTRL_INTERNAL_FUNCTIONS 1 do_test alter-9.1 { - execsql {SELECT SQLITE_RENAME_COLUMN(0,0,0,0,0,0,0,0,0)} + execsql {SELECT SQLITE_RENAME_COLUMN(0,0,0,0,0,0,0,0,0,0)} } {{}} foreach {tn sql} { 1 { SELECT SQLITE_RENAME_TABLE(0,0,0,0,0,0,0) } diff --git a/test/alterauth2.test b/test/alterauth2.test index bd589cda1d..f00c519fb7 100644 --- a/test/alterauth2.test +++ b/test/alterauth2.test @@ -82,7 +82,6 @@ do_auth_test 1.2 { {SQLITE_ALTER_TABLE main t2 {} {}} {SQLITE_FUNCTION {} like {} {}} {SQLITE_FUNCTION {} sqlite_rename_column {} {}} - {SQLITE_FUNCTION {} sqlite_rename_test {} {}} {SQLITE_READ sqlite_master name main {}} {SQLITE_READ sqlite_master sql main {}} {SQLITE_READ sqlite_master tbl_name main {}} diff --git a/test/altercol.test b/test/altercol.test index d71a9b06e4..94b86123f6 100644 --- a/test/altercol.test +++ b/test/altercol.test @@ -638,7 +638,7 @@ do_execsql_test 14.1 { do_execsql_test 14.2 { SELECT - sqlite_rename_column(sql, type, object, db, tbl, icol, znew, bquote, 0) + sqlite_rename_column(sql, type, object, db, tbl, icol, znew, bquote, 0, 0) FROM ddd; } {{} {} {} {}} sqlite3_test_control SQLITE_TESTCTRL_INTERNAL_FUNCTIONS 0 @@ -648,7 +648,7 @@ sqlite3_test_control SQLITE_TESTCTRL_INTERNAL_FUNCTIONS 0 # ordinary SQL. # do_catchsql_test 14.3 { - SELECT sqlite_rename_column(0,0,0,0,0,0,0,0,0); + SELECT sqlite_rename_column(0,0,0,0,0,0,0,0,0,0); } {1 {no such function: sqlite_rename_column}} #------------------------------------------------------------------------- diff --git a/test/altercol2.test b/test/altercol2.test new file mode 100644 index 0000000000..5ae6e77c87 --- /dev/null +++ b/test/altercol2.test @@ -0,0 +1,49 @@ +# 2019 July 3 +# +# The author disclaims copyright to this source code. In place of +# a legal notice, here is a blessing: +# +# May you do good and not evil. +# May you find forgiveness for yourself and forgive others. +# May you share freely, never taking more than you give. +# +#************************************************************************* +# This file implements regression tests for SQLite library. The +# focus of this script is testing that SQLite can handle a subtle +# file format change that may be used in the future to implement +# "ALTER TABLE ... RENAME COLUMN ... TO". +# + +set testdir [file dirname $argv0] +source $testdir/tester.tcl +set testprefix altercol2 + +# If SQLITE_OMIT_ALTERTABLE is defined, omit this file. +ifcapable !altertable { + finish_test + return +} + +sqlite3_db_config db DQS_DDL 1 + +#------------------------------------------------------------------------- +# Check that the ALTER TABLE statement used in bug [9b78184b] is now +# an error (because it effectively transforms a constant string into a +# column ref). +# +do_execsql_test 1.0 { + CREATE TABLE t0(c1, c2); + INSERT INTO t0(c1, c2) VALUES ('a', 1); + CREATE INDEX i0 ON t0("C3"); +} + +do_catchsql_test 1.1 { + ALTER TABLE t0 RENAME COLUMN c1 TO c3; +} {1 {schema contains ambiguous double-quoted string}} + +do_execsql_test 1.2 { + ALTER TABLE t0 RENAME COLUMN c1 TO c4; + SELECT DISTINCT * FROM t0; +} {a 1} + +finish_test