/ Check-in [7fa1faea]
Login

Many hyperlinks are disabled.
Use anonymous login to enable hyperlinks.

Overview
Comment:Improvements to error handling in ALTER TABLE RENAME COLUMN.
Downloads: Tarball | ZIP archive | SQL archive
Timelines: family | ancestors | descendants | both | edit-trigger-wrapper
Files: files | file ages | folders
SHA3-256: 7fa1faeaff30b74b68ee6f4b363d837f21cf313d8262361c901bda884df139a2
User & Date: dan 2018-08-18 17:35:38
Context
2018-08-18
18:01
Have ALTER TABLE RENAME COLUMN also edit trigger and view definitions. check-in: 7908e8a4 user: dan tags: alter-table-rename-column
17:35
Improvements to error handling in ALTER TABLE RENAME COLUMN. Closed-Leaf check-in: 7fa1faea user: dan tags: edit-trigger-wrapper
2018-08-17
18:08
Allow an ALTER TABLE RENAME COLUMN to proceed even if the schema contains a virtual table for which the module is unavailable. check-in: 7b72b236 user: dan tags: edit-trigger-wrapper
Changes
Hide Diffs Side-by-Side Diffs Ignore Whitespace Patch

Changes to src/alter.c.

   960    960         pCtx->pList = pToken;
   961    961         pCtx->nList++;
   962    962         break;
   963    963       }
   964    964     }
   965    965   }
   966    966   
          967  +/*
          968  +** This is a Walker select callback. It does nothing. It is only required
          969  +** because without a dummy callback, sqlite3WalkExpr() and similar do not
          970  +** descend into sub-select statements.
          971  +*/
   967    972   static int renameColumnSelectCb(Walker *pWalker, Select *p){
   968    973     return WRC_Continue;
   969    974   }
   970         -
   971    975   
   972    976   /*
   973    977   ** This is a Walker expression callback.
   974    978   **
   975    979   ** For every TK_COLUMN node in the expression tree, search to see
   976    980   ** if the column being references is the column being renamed by an
   977    981   ** ALTER TABLE statement.  If it is, then attach its associated
................................................................................
   992    996       renameTokenFind(pWalker->pParse, p, (void*)pExpr);
   993    997     }
   994    998     return WRC_Continue;
   995    999   }
   996   1000   
   997   1001   /*
   998   1002   ** The RenameCtx contains a list of tokens that reference a column that
   999         -** is being renamed by an ALTER TABLE statement.  Return the "first"
         1003  +** is being renamed by an ALTER TABLE statement.  Return the "last"
  1000   1004   ** RenameToken in the RenameCtx and remove that RenameToken from the
  1001         -** RenameContext.  "First" means the first RenameToken encountered when
  1002         -** the input SQL from left to right.  Repeated calls to this routine
         1005  +** RenameContext.  "Last" means the last RenameToken encountered when
         1006  +** the input SQL is parsed from left to right.  Repeated calls to this routine
  1003   1007   ** return all column name tokens in the order that they are encountered
  1004   1008   ** in the SQL statement.
  1005   1009   */
  1006   1010   static RenameToken *renameColumnTokenNext(RenameCtx *pCtx){
  1007   1011     RenameToken *pBest = pCtx->pList;
  1008   1012     RenameToken *pToken;
  1009   1013     RenameToken **pp;
................................................................................
  1012   1016       if( pToken->t.z>pBest->t.z ) pBest = pToken;
  1013   1017     }
  1014   1018     for(pp=&pCtx->pList; *pp!=pBest; pp=&(*pp)->pNext);
  1015   1019     *pp = pBest->pNext;
  1016   1020   
  1017   1021     return pBest;
  1018   1022   }
         1023  +
         1024  +/*
         1025  +** An error occured while parsing or otherwise processing a database
         1026  +** object (either pParse->pNewTable, pNewIndex or pNewTrigger) as part of an
         1027  +** ALTER TABLE RENAME COLUMN program. The error message emitted by the
         1028  +** sub-routine is currently stored in pParse->zErrMsg. This function
         1029  +** adds context to the error message and then stores it in pCtx.
         1030  +*/
         1031  +static void renameColumnParseError(sqlite3_context *pCtx, Parse *pParse){
         1032  +  const char *zT;
         1033  +  const char *zN;
         1034  +  char *zErr;
         1035  +  if( pParse->pNewTable ){
         1036  +    zT = pParse->pNewTable->pSelect ? "view" : "table";
         1037  +    zN = pParse->pNewTable->zName;
         1038  +  }else if( pParse->pNewIndex ){
         1039  +    zT = "index";
         1040  +    zN = pParse->pNewIndex->zName;
         1041  +  }else{
         1042  +    assert( pParse->pNewTrigger );
         1043  +    zT = "trigger";
         1044  +    zN = pParse->pNewTrigger->zName;
         1045  +  }
         1046  +  zErr = sqlite3_mprintf("error processing %s %s: %s", zT, zN, pParse->zErrMsg);
         1047  +  sqlite3_result_error(pCtx, zErr, -1);
         1048  +  sqlite3_free(zErr);
         1049  +}
  1019   1050   
  1020   1051   /*
  1021   1052   ** SQL function:
  1022   1053   **
  1023   1054   **     sqlite_rename_column(zSql, iCol, bQuote, zNew, zTable, zOld)
  1024   1055   **
  1025   1056   **   0. zSql:     SQL statement to rewrite
................................................................................
  1079   1110     if( iCol<0 ) return;
  1080   1111     pTab = sqlite3FindTable(db, zTable, zDb);
  1081   1112     if( pTab==0 || iCol>=pTab->nCol ) return;
  1082   1113     zOld = pTab->aCol[iCol].zName;
  1083   1114     memset(&sCtx, 0, sizeof(sCtx));
  1084   1115     sCtx.iCol = ((iCol==pTab->iPKey) ? -1 : iCol);
  1085   1116   
         1117  +  /* Parse the SQL statement passed as the first argument. If no error
         1118  +  ** occurs and the parse does not result in a new table, index or
         1119  +  ** trigger object, the database must be corrupt. */
  1086   1120     memset(&sParse, 0, sizeof(sParse));
  1087   1121     sParse.eParseMode = PARSE_MODE_RENAME_COLUMN;
  1088   1122     sParse.db = db;
  1089   1123     sParse.nQueryLoop = 1;
  1090   1124     rc = sqlite3RunParser(&sParse, zSql, &zErr);
  1091         -  assert( sParse.pNewTable==0 || sParse.pNewIndex==0 );
         1125  +  assert( sParse.zErrMsg==0 );
         1126  +  assert( rc!=SQLITE_OK || zErr==0 );
         1127  +  assert( (!!sParse.pNewTable)+(!!sParse.pNewIndex)+(!!sParse.pNewTrigger)<2 );
         1128  +  sParse.zErrMsg = zErr;
  1092   1129     if( db->mallocFailed ) rc = SQLITE_NOMEM;
  1093   1130     if( rc==SQLITE_OK 
  1094   1131      && sParse.pNewTable==0 && sParse.pNewIndex==0 && sParse.pNewTrigger==0 
  1095   1132     ){
  1096   1133       rc = SQLITE_CORRUPT_BKPT;
  1097   1134     }
  1098   1135   
  1099         -  if( rc==SQLITE_OK ){
  1100         -    zQuot = sqlite3_mprintf("\"%w\"", zNew);
  1101         -    if( zQuot==0 ){
  1102         -      rc = SQLITE_NOMEM;
  1103         -    }else{
  1104         -      nQuot = sqlite3Strlen30(zQuot);
  1105         -    }
  1106         -  }
  1107         -
  1108         -  if( bQuote ){
  1109         -    zNew = zQuot;
  1110         -    nNew = nQuot;
  1111         -  }
  1112         -
  1113   1136   #ifdef SQLITE_DEBUG
         1137  +  /* Ensure that all mappings in the Parse.pRename list really do map to
         1138  +  ** a part of the input string.  */
  1114   1139     assert( sqlite3Strlen30(zSql)==nSql );
  1115   1140     if( rc==SQLITE_OK ){
  1116   1141       RenameToken *pToken;
  1117   1142       for(pToken=sParse.pRename; pToken; pToken=pToken->pNext){
  1118   1143         assert( pToken->t.z>=zSql && &pToken->t.z[pToken->t.n]<=&zSql[nSql] );
  1119   1144       }
  1120   1145     }
  1121   1146   #endif
         1147  +
         1148  +  /* Set zQuot to point to a buffer containing a quoted copy of the 
         1149  +  ** identifier zNew. If the corresponding identifier in the original 
         1150  +  ** ALTER TABLE statement was quoted (bQuote==1), then set zNew to
         1151  +  ** point to zQuot so that all substitutions are made using the
         1152  +  ** quoted version of the new column name.  */
         1153  +  if( rc==SQLITE_OK ){
         1154  +    zQuot = sqlite3_mprintf("\"%w\"", zNew);
         1155  +    if( zQuot==0 ){
         1156  +      rc = SQLITE_NOMEM;
         1157  +    }else{
         1158  +      nQuot = sqlite3Strlen30(zQuot);
         1159  +    }
         1160  +  }
         1161  +  if( bQuote ){
         1162  +    zNew = zQuot;
         1163  +    nNew = nQuot;
         1164  +  }
  1122   1165   
  1123   1166     /* Find tokens that need to be replaced. */
  1124   1167     memset(&sWalker, 0, sizeof(Walker));
  1125   1168     sWalker.pParse = &sParse;
  1126   1169     sWalker.xExprCallback = renameColumnExprCb;
  1127   1170     sWalker.xSelectCallback = renameColumnSelectCb;
  1128   1171     sWalker.u.pRename = &sCtx;
................................................................................
  1133   1176       Select *pSelect = sParse.pNewTable->pSelect;
  1134   1177       if( pSelect ){
  1135   1178         sParse.rc = SQLITE_OK;
  1136   1179         sqlite3SelectPrep(&sParse, sParse.pNewTable->pSelect, 0);
  1137   1180         rc = (db->mallocFailed ? SQLITE_NOMEM : sParse.rc);
  1138   1181         if( rc==SQLITE_OK ){
  1139   1182           sqlite3WalkSelect(&sWalker, pSelect);
  1140         -      }else if( rc==SQLITE_ERROR ){
  1141         -        /* Failed to resolve all symbols in the view. This is not an 
  1142         -        ** error, but it will not be edited. */
  1143         -        sqlite3DbFree(db, sParse.zErrMsg);
  1144         -        sParse.zErrMsg = 0;
  1145         -        rc = SQLITE_OK;
  1146   1183         }
  1147   1184         if( rc!=SQLITE_OK ) goto renameColumnFunc_done;
  1148   1185       }else{
  1149   1186         /* A regular table */
  1150   1187         int bFKOnly = sqlite3_stricmp(zTable, sParse.pNewTable->zName);
  1151   1188         FKey *pFKey;
  1152   1189         assert( sParse.pNewTable->pSelect==0 );
................................................................................
  1290   1327           sqlite3WalkExprList(&sWalker, pUpsert->pUpsertSet);
  1291   1328           sqlite3WalkExpr(&sWalker, pUpsert->pUpsertWhere);
  1292   1329           sqlite3WalkExpr(&sWalker, pUpsert->pUpsertTargetWhere);
  1293   1330         }
  1294   1331       }
  1295   1332     }
  1296   1333   
         1334  +  /* At this point sCtx.pList contains a list of RenameToken objects
         1335  +  ** corresponding to all tokens in the input SQL that must be replaced
         1336  +  ** with the new column name. All that remains is to construct and
         1337  +  ** return the edited SQL string. */
  1297   1338     assert( rc==SQLITE_OK );
  1298   1339     assert( nQuot>=nNew );
  1299   1340     zOut = sqlite3DbMallocZero(db, nSql + sCtx.nList*nQuot + 1);
  1300   1341     if( zOut ){
  1301   1342       int nOut = nSql;
  1302   1343       memcpy(zOut, zSql, nSql);
  1303   1344       while( sCtx.pList ){
................................................................................
  1330   1371       sqlite3DbFree(db, zOut);
  1331   1372     }else{
  1332   1373       rc = SQLITE_NOMEM;
  1333   1374     }
  1334   1375   
  1335   1376   renameColumnFunc_done:
  1336   1377     if( rc!=SQLITE_OK ){
  1337         -    if( zErr ){
  1338         -      sqlite3_result_error(context, zErr, -1);
         1378  +    if( sParse.zErrMsg ){
         1379  +      renameColumnParseError(context, &sParse);
  1339   1380       }else{
  1340   1381         sqlite3_result_error_code(context, rc);
  1341   1382       }
  1342   1383     }
  1343   1384   
  1344   1385     if( sParse.pVdbe ){
  1345   1386       sqlite3VdbeFinalize(sParse.pVdbe);
  1346   1387     }
  1347   1388     sqlite3DeleteTable(db, sParse.pNewTable);
  1348   1389     if( sParse.pNewIndex ) sqlite3FreeIndex(db, sParse.pNewIndex);
  1349   1390     sqlite3DeleteTrigger(db, sParse.pNewTrigger);
  1350   1391     renameTokenFree(db, sParse.pRename);
  1351   1392     renameTokenFree(db, sCtx.pList);
         1393  +  sqlite3DbFree(db, sParse.zErrMsg);
  1352   1394     sqlite3ParserReset(&sParse);
  1353   1395     sqlite3_free(zQuot);
  1354   1396   }
  1355   1397   
  1356   1398   /*
  1357   1399   ** Register built-in functions used to help implement ALTER TABLE
  1358   1400   */

Changes to test/altercol.test.

    21     21   set testprefix altercol
    22     22   
    23     23   # If SQLITE_OMIT_ALTERTABLE is defined, omit this file.
    24     24   ifcapable !altertable {
    25     25     finish_test
    26     26     return
    27     27   }
           28  +
           29  +# Drop all the tables and views in the 'main' database of database connect
           30  +# [db]. Sort the objects by name before dropping them.
           31  +#
           32  +proc drop_all_tables_and_views {db} {
           33  +  set SQL {
           34  +    SELECT name, type FROM sqlite_master 
           35  +    WHERE type IN ('table', 'view') AND name NOT LIKE 'sqlite_%'
           36  +    ORDER BY 1
           37  +  }
           38  +  foreach {z t} [db eval $SQL] {
           39  +    db eval "DROP $t $z"
           40  +  }
           41  +}
    28     42   
    29     43   foreach {tn before after} {
    30     44     1 {CREATE TABLE t1(a INTEGER, b TEXT, c BLOB)}
    31     45       {CREATE TABLE t1(a INTEGER, d TEXT, c BLOB)}
    32     46   
    33     47     2 {CREATE TABLE t1(a INTEGER, x TEXT, "b" BLOB)}
    34     48       {CREATE TABLE t1(a INTEGER, x TEXT, "d" BLOB)}
................................................................................
   320    334   }
   321    335   
   322    336   do_execsql_test 8.4.4 {
   323    337     ALTER TABLE b2 RENAME x TO hello;
   324    338     SELECT sql FROM sqlite_master WHERE name='xxx';
   325    339   } {{CREATE VIEW xxx AS SELECT a FROM b1 UNION SELECT hello FROM b2 ORDER BY 1 COLLATE nocase}}
   326    340   
   327         -do_execsql_test 8.4.5 {
          341  +do_catchsql_test 8.4.5 {
   328    342     CREATE VIEW zzz AS SELECT george, ringo FROM b1;
   329    343     ALTER TABLE b1 RENAME a TO aaa;
   330         -  SELECT sql FROM sqlite_master WHERE name = 'zzz'
   331         -} {{CREATE VIEW zzz AS SELECT george, ringo FROM b1}}
          344  +} {1 {error processing view zzz: no such column: george}}
   332    345   
   333    346   #-------------------------------------------------------------------------
   334    347   # More triggers.
   335    348   #
   336    349   proc do_rename_column_test {tn old new lSchema} {
   337         -
   338    350     for {set i 0} {$i < 2} {incr i} {
   339         -    # DROP all tables and views in database.
   340         -    set sql "SELECT name FROM sqlite_master WHERE type='table' ORDER BY 1"
   341         -    foreach nm [db eval $sql] { db eval "DROP TABLE $nm" }
   342         -    set sql "SELECT name FROM sqlite_master WHERE type='view' ORDER BY 1"
   343         -    foreach nm [db eval $sql] { db eval "DROP VIEW $nm" }
          351  +    drop_all_tables_and_views db
   344    352   
   345    353       set lSorted [list]
   346    354       foreach sql $lSchema { 
   347    355         execsql $sql 
   348    356         lappend lSorted [string trim $sql]
   349    357       }
   350    358       set lSorted [lsort $lSorted]
   351    359   
   352    360       do_execsql_test $tn.$i.1 {
   353    361         SELECT sql FROM sqlite_master WHERE sql!='' ORDER BY 1
   354    362       } $lSorted
   355    363   
   356         -    if {0 && $i==1} {
          364  +    if {$i==1} {
   357    365         db close
   358    366         sqlite3 db test.db
   359    367       }
   360    368   
   361    369       do_execsql_test $tn.$i.2 "ALTER TABLE t1 RENAME $old TO $new"
   362    370   
   363    371       do_execsql_test $tn.$i.3 {
................................................................................
   428    436       { CREATE VIRTUAL TABLE e1 USING echo(t1) }
   429    437     }
   430    438   } {
   431    439     register_echo_module db
   432    440     do_rename_column_test 10.$tn $old $new $lSchema
   433    441   }
   434    442   
          443  +#--------------------------------------------------------------------------
          444  +# Test that if a view or trigger refers to a virtual table for which the
          445  +# module is not available, RENAME COLUMN cannot proceed.
          446  +#
          447  +reset_db
          448  +register_echo_module db
          449  +do_execsql_test 11.0 {
          450  +  CREATE TABLE x1(a, b, c);
          451  +  CREATE VIRTUAL TABLE e1 USING echo(x1);
          452  +}
          453  +db close
          454  +sqlite3 db test.db
          455  +
          456  +do_execsql_test 11.1 {
          457  +  ALTER TABLE x1 RENAME b TO bbb;
          458  +  SELECT sql FROM sqlite_master;
          459  +} { {CREATE TABLE x1(a, bbb, c)} {CREATE VIRTUAL TABLE e1 USING echo(x1)} }
          460  +
          461  +do_execsql_test 11.2 {
          462  +  CREATE VIEW v1 AS SELECT e1.*, x1.c FROM e1, x1;
          463  +}
          464  +
          465  +do_catchsql_test 11.3 {
          466  +  ALTER TABLE x1 RENAME c TO ccc;
          467  +} {1 {error processing view v1: no such module: echo}}
   435    468   
   436    469   finish_test