Have ALTER TABLE detect and error out for the case where renaming a column changes...

FossilOrigin-Name: 5dbb0734aff25e47a58a64abe0e96636cbf8a7a5006df0cfe10af8629af9133c

diff --git a/manifest b/manifest
index a320ae1e9159174faf93d47657cf3192c18602bc..cabbcab55b98e99859f1ce59260da3a627a378ee 100644 (file)
--- 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 82a0a5af46c763a84850ae1bfa6825daaa6e7e30..ae5e68f100a9964f51ff782ac2e7671a4f1503fb 100644 (file)
--- 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 3957edefa08dd009995b1b07dd56a8a2280494f0..c1bdb6e1c9d663e745a00076613e6d9462ec5efd 100644 (file)
--- 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 041a91c51fe93e2dbbfd73d69a9bd9b3c9930806..0e27c6b4d2d173626aee383958c20f06eb6c6e23 100644 (file)
--- 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 8dd2b421aa28e9ab3e43012d1a1fc6f3e9691e35..5ff8779e7f56c0849c93e0c107b47fde9022bdd7 100644 (file)
--- 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 d20e5ebccdea4cd5f26d244b04b5378c46d7ee2f..1c942e867bfde708465309b2fbcf64fe4289c265 100644 (file)
--- 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 bd589cda1d1c6357ed9f2bff307961d2ea5a8737..f00c519fb7dc3834351da5a2e46b60c8762ff9d5 100644 (file)
--- 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 d71a9b06e42bf643cf1c0819ffbdf98dbcce10e0..94b86123f6f05afde057c3a8a683ba09d23a2b04 100644 (file)
--- 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 (file)
index 0000000..5ae6e77
--- /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