sqlcipher / android-database-sqlcipher

Android SQLite API based on SQLCipher
https://www.zetetic.net/sqlcipher/sqlcipher-for-android/
Other
2.73k stars 564 forks source link

changes done using ALTER table query with db.execSQL() are not reflecting when using db.query() #605

Closed muthus55 closed 1 year ago

muthus55 commented 1 year ago

Steps to Reproduce:

  1. Create an Android application with SQLCipher integration.

  2. Create a database and add some tables into it.

  3. First query any table using the SQLiteDatabase.query(String table, String[] columns, String selection, String[] selectionArgs, String groupBy, String having, String orderBy) method with null projections

  4. This will return a Cursor object. If you inspect the cursor all the columns and rows will be available

  5. Now alter the table queried in step 3 and add a new column to it using SQLiteDatabase.execSQL()

  6. Use the same query used in step 3. and dump the cursor object. The newly added column in step 5 will not be available in this query.

Example:

  1. Let us a assume a Table "Students" which contains 2 columns "name" & "id"
  2. First query the database using the SQLiteDatabase.query(String table, String[] columns, String selection, String[] selectionArgs, String groupBy, String having, String orderBy) method with null projections method call -> { SQLiteDatabase.query("Students",null,null,null,null,null,null) } if the cursor from query method is dumped we can see 2 columns "name" & "id".
  3. Now alter the "Students" table and add a new column to it. Let us assume the new column to "age". This alter query execution is done by using SQLiteDatabase.execSQL() method call -> { SQLiteDatabase.execSQL(" ALTER TABLE STUDENTS ADD age TEXT NULL") }
  4. Now query the "Students" table again with the same query used in step 2 method call -> { SQLiteDatabase.query("Students",null,null,null,null,null,null) }
  5. Dump the cursor object you can see that the newly created column "age" will not be available in the cursor object.

This issue occurs due to caching the prepared statement. The caching is done in "SQLiteDatabase.mCompiledQueries" object. If a table is queried and the SQL statement is cached in "SQLiteDatabase.mCompiledQueries" object we use the same compiled prepared statement. Whenever a SQLiteCursor is initialed we use the SQLiteProgram object and this SQLiteProgram object will try to check if any cached queries available in mCompiledQueries and reuses it. This cached prepared statement will not reflect the newly added columns done using SQLiteDatabase.execSQL();

developernotes commented 1 year ago

Hello @muthus55,

Thank you for your interest in SQLCipher for Android. This behavior is present in both the new and legacy version of SQLCipher for Android, but it is also the same behavior when executed against the standard SQLite implementation bundled with Android when compiled and targeting the latest SDK version, currently API 33.

Because of this, we would prefer not to deviate from the current behavior for consistency. For your specific scenario, after you perform your ALTER statement, you can invoke the following:

database.resetCompiledSqlCache();

The above will allow you to query the adjusted table and reflect the correct number of columns.

muthus55 commented 1 year ago

Hi @developernotes I created an issue in Google issue tracker for this. Meanwhile I checked your suggestion on using the database.resetCompiledSqlCache(); . I find some issues in this. Using this function will create a memory leak in the Sqlite C++ layer.

Explanation: When a String SQL is compiled to Prepared Statement in C++ code using sqlite3_prepare16_v2 in net_sqlcipher_database_SQLiteCompiledSql.cpp the pointer to the compiled statement is stored in SQLiteCompiledSql.nStatement variable.

This compiled SQL statement should be deallocated after the cache is cleared in C++ layer level. This deallocation is done properly in SqliteDatabase.close(). This method calls SqliteDatabase.closeClosable() where deallocCachedSqlStatements(); is called which iterated over the cached Prepared Statements and takes the C++ pointer stored in SqliteCompiledSql.nStatement and releases the Prepared Statement by using the native method SqliteCompiledSql.native_finalize().

But in the case of database.resetCompiledSqlCache(); this is not done. This would case memory leak issues of the stored prepared SQL statements.

Please clarify this case. Thanks in Advance.

developernotes commented 1 year ago

Hi @muthus55,

Thank you for pointing out the potential to leak memory from the statement pointers. We are just finalizing a new public release of SQLCipher and due to timing are unable to include a fix for this in our impending release. We will correct this behavior following this release.

muthus55 commented 1 year ago

Hi @developernotes, I would suggest that Sqlcipher library itself can handle this cache reset when performing SQLiteDatabase.execSQL. When you are fixing this memory leak issue please consider adding the reset logic in SQLiteDatabase.execSQL method. We can't enforce every developer to reset the cache when an execSQL is performed on each code base.

Also if it is possible to contribute I can fix this. Let me know.

muthus55 commented 1 year ago

Hi @developernotes,

Any update on this ?

developernotes commented 1 year ago

Hello @muthus55,

We plan to investigate this situation further prior to our next public release of SQLCipher. We do not have an update available at this time.

developernotes commented 1 year ago

Hi @muthus55,

We have just pushed up a fix to finalize the cached sqlite3_stmt and remove the stored reference in the call to resetCompiledSqlCache(). This will be included in the next public release of SQLCipher. Thank you for the report!

muthus55 commented 1 year ago

Hi @developernotes, What about clearing the sqlcache automatically when using "SQLiteDatabase.execSQL" ?

developernotes commented 1 year ago

Hi @muthus55,

We would prefer not to deviate from the upstream Android SQLite implementation behavior.