1

I have an insert statement and it takes my app about 25 seconds to insert ~1000 rows. I'm trying to use Transactions to speed up my processing time but I'm having trouble implementing it.

Here is my method for inserting in my DatabaseHandler:

public boolean insertQuestions(String gid, String qid, String qname) {
    SQLiteDatabase db = this.getWritableDatabase();
    db.beginTransaction();
    try {
        ContentValues contentValues = new ContentValues();
        contentValues.put(KEY_GID, gid);
        contentValues.put(KEY_QID, qid);
        contentValues.put(KEY_QNAME, qname);

        long result = db.insert(TABLE_QUESTIONS, null, contentValues);
        db.setTransactionSuccessful();
        if (result == -1) {
            db.close();
            return false;
        } else {
            db.close();
            return true;
        }
    } finally {
        db.endTransaction(); //Error is here
    }
}

I'm getting the following error when I try to run the above method:

08-28 15:50:40.961  20539-20539/? E/AndroidRuntime﹕ FATAL EXCEPTION: main
Process: com.murrion.navigationdrawer, PID: 20539
java.lang.IllegalStateException: attempt to re-open an already-closed object: SQLiteDatabase: /storage/emulated/0/testapp3.db
        at android.database.sqlite.SQLiteClosable.acquireReference(SQLiteClosable.java:55)
        at android.database.sqlite.SQLiteDatabase.endTransaction(SQLiteDatabase.java:522)
        at com.murrion.navigationdrawer.utils.DatabaseHandler.insertQuestions(DatabaseHandler.java:197)

The statement is being run in an Asynctask method in my Activity.

mhorgan
  • 886
  • 2
  • 11
  • 32
  • I think that you are trying to execute an order in db when db is closed few lines before. You can't call db.endTransaction(); after call db.close(); Try to put the if/else after db.endTransaction() and tell us. Hope it helps – jekeyeke Aug 28 '15 at 15:12
  • Sorry, my phrasing was bad. It's in a loop, so it's being executed ~1000 times. Very inefficient..I know – mhorgan Aug 28 '15 at 15:29

3 Answers3

5

You have already closed the db before ending the transaction, instead close it after ending.

public boolean insertQuestions(String gid, String qid, String qname) {
    SQLiteDatabase db = this.getWritableDatabase();

    boolean wasSuccess = true;
    try {
        db.beginTransaction();
        ContentValues contentValues = new ContentValues();
        contentValues.put(KEY_GID, gid);
        contentValues.put(KEY_QID, qid);
        contentValues.put(KEY_QNAME, qname);

        long result = db.insert(TABLE_QUESTIONS, null, contentValues);

        if (result == -1) {
            wasSuccess = false;
        } else {
            db.setTransactionSuccessful();
        }
    } finally {
        db.endTransaction(); 
        db.close();
    }
    return wasSuccess;
}
petey
  • 16,914
  • 6
  • 65
  • 97
  • I made the changes, code worked without error. But my insertion still takes ~25 seconds? Could this be a limitation of my tablet (Galaxy Tab 3 10.1")? When I run this on a Genymotion Emulator, it takes ~8 seconds. – mhorgan Aug 28 '15 at 15:23
  • 2
    @Mark117 You should really make an insert per row. And try to **indicize** your fields! – Phantômaxx Aug 28 '15 at 15:29
  • 3
    @Mark117 if you really want to speed up your inserts, ContentValues + db.insert + transactions will not help as much as prepared statements. MY answer only fixes your crash issue. Please see this answer (Section Inser Valuess) as speeding up insert has already been (amazingly) answered http://stackoverflow.com/a/29797229/794088 (dont forget to upvote =) should you find anything helpful) – petey Aug 28 '15 at 15:55
4

Two problems

  1. You are only doing one insert per transaction. You need to do ALL the inserts in the same transaction.
  2. You are closing the database before ending the transaction.

You can make a Question class that encapsulates the data of a single question. In that case your method might look like this:

public boolean insertQuestions(List<Question> questions) {
    SQLiteDatabase db = this.getWritableDatabase();
    db.beginTransaction();
    try {
        ContentValues contentValues = new ContentValues();
        for (Question question : Questions) {
            contentValues.put(KEY_GID, question.gid);
            contentValues.put(KEY_QID, question.qid);
            contentValues.put(KEY_QNAME, question.qname);
            db.insert(TABLE_QUESTIONS, null, contentValues);
        }
        db.setTransactionSuccessful();
    } finally {
        db.endTransaction();
    }
}
Karakuri
  • 38,365
  • 12
  • 84
  • 104
  • I think this is what I have to do. I am currently executing the same insertQuestions method ~1000 times inside a for loop. Passing the list through seems like the way to solve this problem. Thanks – mhorgan Aug 28 '15 at 15:41
1

I think you are inserting question using insertQuestions() one by one. So beginTransaction() and endTransaction() called many more times which is inefficient.

There is some changes needed to speed up transactions:

You should have to first insert all questions to array then use like:

db.beginTransaction();
for (entry : listOfQuestions) {
    db.insertQuestions("","","");
}
db.setTransactionSuccessful();
db.endTransaction();

Some modifications in your method.

public boolean insertQuestions(String gid, String qid, String qname) {
    SQLiteDatabase db = this.getWritableDatabase();

    boolean wasSuccess = true;
    try {
        db.beginTransaction();
        ContentValues contentValues = new ContentValues();
        contentValues.put(KEY_GID, gid);
        contentValues.put(KEY_QID, qid);
        contentValues.put(KEY_QNAME, qname);

        long result = db.insert(TABLE_QUESTIONS, null, contentValues);

        if (result == -1) {
            wasSuccess = false;
        } else {
            db.setTransactionSuccessful();
        }
    } finally {
        db.endTransaction(); 
        db.close();
    }
    return wasSuccess;
}

I hope it helps!

Rajesh Jadav
  • 12,801
  • 5
  • 53
  • 78