Error on Upgrade Database Version, another table already exists

[dragermrb]

Sometimes we have some problems updating the database schema.

At some point, one of the temporary tables has not been deleted and when you do the update again the table already exists and you cannot continue.

Error: Open: Error in creating the databaseError: onUpgrade executeStatementProcess failed
    java.lang.Exception: Error: executeStatementProcess  failed
    java.lang.Exception: Error: backupTables failed 
    java.lang.Exception: Error: backupTable failed 
    java.lang.Exception: there is already another table or index with this name: _temp_messages: , while compiling: ALTER TABLE messages RENAME TO _temp_messages;

Is there a way to drop the temporary tables before starting the upgrade schema?

Versions

@capacitor/android : 3.4.0
@capacitor-community/sqlite: 3.4.0

[tobiasmuecksch]

This might be a duplicate of issue #245 which was fixed in 3.4.2

Could you please try the latest version @capacitor-community/sqlite@3.4.2?

[dragermrb]

Hi!

I think it is not the same issue. In my case we are not using ImportFromJson and I don't see any significant changes between versions 3.4.0 and 3.4.2 (https://github.com/capacitor-community/sqlite/compare/v3.4.0...master) for the files:

android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/UtilsDrop.java android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/UtilsUpgrade.java

I think these changes could fix the issue:

File node_modules/@capacitor-community/sqlite/android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/UtilsUpgrade.java

// Lines ~193
// ...
try {
    // Drop existing temp table if exists
    db.runSQL("DROP TABLE IF EXISTS _temp_" + table + ";", new ArrayList<>()); // <========== Add this line

    // get the column's name
    List<String> colNames = getColumnNames(db, table);
    _alterTables.put(table, colNames);
    // ...

File node_modules/@capacitor-community/sqlite/ios/Plugin/Utils/UtilsUpgrade.swift

// Lines ~274
// ...
do {
    // Drop existing temp table if exists
    try mDB.runSQL(sql: "DROP TABLE IF EXISTS _temp_\(tableName);", values: []) // <========== Add this line

    // get the column's name
    columnNames = try getTableColumnNames(mDB: mDB,
                                          tableName: tableName)
    alterTables["\(tableName)"] = columnNames
    // ...

[dragermrb]

I'm going to try version 3.4.2, but we don't know when the error actually occurs and it's hard to reproduce.

[dragermrb]

Hi!

The idea is to drop the temporary table if it exists before renaming the real table.

For example, if we have the messages table, the backupTable() method takes care of the following:

1. Delete the _temp_messages table if it exists (This is the correction that I propose in the previous code)
2. Rename the messages table in _temp_messages

I can make a PR if you prefer

[tobiasmuecksch]

Thanks for your clarification. I'm just confused, that this is necessary, as this should already be handled in this line.

https://github.com/capacitor-community/sqlite/blob/83c7e134f8c5d3c86d9e59c1c3ecbf581f28bbfe/android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/UtilsUpgrade.java#L155

I'm trying to understand under which circumstances the temp tables are not deleted.

[dragermrb]

Hi!

If an error occurs between lines 131-154 (line 145 for example), the next time an upgrade is attempted, this issue will be present.

How can we guarantee that the temporary tables are dropped in case of any error between lines 131-154? Maybe dropping temp tables on finally block like this?

    try {
            // -> backup all existing tables  "tableName" in
            //    "temp_tableName"
            backupTables(db);

            // -> Drop all Indexes
            _uDrop.dropIndexes(db);

            // -> Drop all Triggers
            _uDrop.dropTriggers(db);

            // -> Create new tables from upgrade.statement

            String[] sqlCmdArray = _uSqlite.getStatementsArray(statement);

            JSObject retObj = db.execute(sqlCmdArray);
            changes = retObj.getInteger("changes");
            if (changes < Integer.valueOf(0)) {
                throw new Exception("create new tables failed");
            }

            // -> Create the list of table's common fields
            findCommonColumns(db);

            // -> Update the new table's data from old table's data
            updateNewTablesData(db);
        } catch (Exception e) {
            throw new Exception("Error: executeStatementProcess " + " failed " + e);
        } finally {  // <================ this block
            // -> Drop _temp_tables
            _uDrop.dropTempTables(db, _alterTables);

            // -> Do some cleanup
            _alterTables = new Hashtable<>();
            _commonColumns = new Hashtable<>();
        }

[tobiasmuecksch]

If an error occurs between lines 131-154 (line 145 for example), the next time an upgrade is attempted, this issue will be present.

Have you seen such an error? If yes, could you post it here? It would help to understand what exactly is happening.

[dragermrb]

We have not been able to see the error directly. We have an application with many users and on some occasions we find the initial error of this issue ("another table already exists").

We use rollbar (https://rollbar.com/) to access the error reports, but we don't have the original error, just the "another table already exists" error.

[tobiasmuecksch]

Maybe @jepiqueau has an idea, where this issue could come from?

@dragermrb I think as a temporary workaround, you could run "DROP TABLE IF EXISTS _temp_" + table + ";" in your code, before triggering the upgrade.

[dragermrb]

@dragermrb I think as a temporary workaround, you could run "DROP TABLE IF EXISTS _temp_" + table + ";" in your code, before triggering the upgrade.

How? UtilsUpgrade.onUpgrade() is executed inside db.open() and I can't execute querys before open db.

[tobiasmuecksch]

Ah yes, your are right. Sorry for that.

[jepiqueau]

@dragermrb thanks for using the plugin, if there is _temp_YOURTABLENAME already existing means that one of the previous upgrade did not complete successfully. So adding a finally block like you suggest is correct in Android i will have to see how defer works in swift i never use it. Any how if will have also to add a drop all _temp_* at the beginning of the process. @tobiasmuecksch thanks for trying to help.

[dragermrb]

@jepiqueau I'm not sure if a finally block is a good idea. I can find some situations where it can be problematic (it will work for my app, but not in a general way). For example:

• Upgrade begins
• Rename messages table to _temp_messages
• Some errors occurs
• Finally block delete _temp_message so the user lost all data

I think we need another approach like use transactions to upgrade tables. Something like this:

try {
    db.runSQL("BEGIN TRANSACTION;", new ArrayList<>());

    backupTables(db);
    _uDrop.dropIndexes(db);

    // ... Rest of upgrade flow

    db.runSQL("COMMIT;", new ArrayList<>());
} catch(e){
    db.runSQL("ROLLBACK;", new ArrayList<>());
}

I can make a PR if you like the idea.

On the other hand, there are other ways to deal with the schema upgrade that do not involve renaming tables, but would involve a major version change (4.x). I can explain the idea if you are interested.

[jepiqueau]

@dragermrb @tobiasmuecksch this is fixed in 3.4.3-1, i also add both of you in the contributor's list. Thanks for your contribution

[jepiqueau]

@dragermrb Did you test the 3.4.3-1 and if it works can you close the issue

[dragermrb]

Hi @jepiqueau

It works for my app, thanks for the fast patch.

I will close this issue, but did you view my last comment? https://github.com/capacitor-community/sqlite/issues/263#issuecomment-1105399035

[jepiqueau]

@dragermrb i saw your comment but i am so concentrate on the delete process that i forgot to answer. Yes it would be interesting that you develop your idea.if you think it is a big change i may need some help to implement it . So let see what you have and we will discuss it later. May be @tobiasmuecksch could also have a look at it and comment. The plugin start to be big and it becomes difficult to maintain it on my own.

[tobiasmuecksch]

Sure I will help where I can 😊👍