Fixed
Status Update
Comments
yb...@google.com <yb...@google.com>
ya...@google.com <ya...@google.com>
ya...@google.com <ya...@google.com> #2
I'm still working on a fix, but here's the current state of OnConflictStrategy.
# REPLACE
Works as expected. Old rows are replaced.
# IGNORE
Works as expected. New rows are ignored.
# ROLLBACK
This ends the transaction on conflict. Thus, the following call of endTransaction() always fails. This is basically unusable.
# ABORT (default)
As the SQLite doc says, all the changes prior to this statement are preserved, but they are rolled back by the surrounding transaction anyway.
# FAIL
This works pretty much the same as ABORT, except that it does not roll back the current statement at first. It is, however, rolled back by the surrounding transaction along with the preceding changes.
In summary, ABORT and FAIL produce the same results, and unlike the SQLite doc says, they both roll back on conflict (because of the surrounding transaction). ROLLBACK always fails.
# REPLACE
Works as expected. Old rows are replaced.
# IGNORE
Works as expected. New rows are ignored.
# ROLLBACK
This ends the transaction on conflict. Thus, the following call of endTransaction() always fails. This is basically unusable.
# ABORT (default)
As the SQLite doc says, all the changes prior to this statement are preserved, but they are rolled back by the surrounding transaction anyway.
# FAIL
This works pretty much the same as ABORT, except that it does not roll back the current statement at first. It is, however, rolled back by the surrounding transaction along with the preceding changes.
In summary, ABORT and FAIL produce the same results, and unlike the SQLite doc says, they both roll back on conflict (because of the surrounding transaction). ROLLBACK always fails.
gm...@gmail.com <gm...@gmail.com> #3
Yes, this is exactly what I observed. It seems that FAIL and ROLLBACK strategies don't make much sense for Room. If this is correct, maybe you should deprecate and later remove them to avoid confusion?
gm...@gmail.com <gm...@gmail.com> #5
Fine!
Maybe it would also be useful to mention in the docs which strategies can throw an SQLiteConstraintException (btw, is it the only possible type of exception?) and which cannot.
Maybe it would also be useful to mention in the docs which strategies can throw an SQLiteConstraintException (btw, is it the only possible type of exception?) and which cannot.
ya...@google.com <ya...@google.com> #6
SQLiteConstraintException is the only type thrown by the platform SQLite, but if you use custom implementation [1], it might throw something else.
[1]:https://developer.android.com/reference/android/arch/persistence/room/RoomDatabase.Builder.html#openHelperFactory(android.arch.persistence.db.SupportSQLiteOpenHelper.Factory)
[1]:
ap...@google.com <ap...@google.com> #7
Project: platform/frameworks/support
Branch: androidx-master-dev
commit b8b669fd8cc863ad19a5e6c912b163aab107a86f
Author: Yuichi Araki <yaraki@google.com>
Date: Mon Dec 03 11:40:46 2018
Deprecate OnConflictStrategy.ROLLBACK and FAIL
OnConflictStrategy.ROLLBACK was never working properly. On conflict, it
always fails with SQLiteException since Room tries to end the
transaction which has already been rolled back by the conflict. In
addition, framework has a bug in handling of transaction around ROLLBACK
( b/120397728 ), so it is impossible to make this work as expected.
Instead, ABORT has always worked like ROLLBACK. This is because the
INSERT throws SQLiteConstraintException on conflict, so the wrapping
transaction is ended without being marked as successful and thus rolled
back. We could change this to work more like SQLite's original ABORT
(keep preceding changes in the transaction), but it is too risky to
change the behavior of ABORT since it is the default option of @Insert
and @Update. This CL just clarifies the behavior in Javadoc.
OnConflictStrategy.FAIL is no different from ABORT because of the
wrapping transaction. We should just remove it for clarity.
REPLACE and IGNORE are working fine.
Bug: 117266738
Test: OnConflictStrategyTest
Change-Id: Id62920d531afe4644d9d37ffc823a65132b54b4f
M room/common/api/2.1.0-alpha03.txt
M room/common/api/current.txt
M room/common/src/main/java/androidx/room/Insert.java
M room/common/src/main/java/androidx/room/OnConflictStrategy.java
M room/common/src/main/java/androidx/room/Update.java
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/OnConflictStrategyTest.java
https://android-review.googlesource.com/839274
https://goto.google.com/android-sha1/b8b669fd8cc863ad19a5e6c912b163aab107a86f
Branch: androidx-master-dev
commit b8b669fd8cc863ad19a5e6c912b163aab107a86f
Author: Yuichi Araki <yaraki@google.com>
Date: Mon Dec 03 11:40:46 2018
Deprecate OnConflictStrategy.ROLLBACK and FAIL
OnConflictStrategy.ROLLBACK was never working properly. On conflict, it
always fails with SQLiteException since Room tries to end the
transaction which has already been rolled back by the conflict. In
addition, framework has a bug in handling of transaction around ROLLBACK
(
Instead, ABORT has always worked like ROLLBACK. This is because the
INSERT throws SQLiteConstraintException on conflict, so the wrapping
transaction is ended without being marked as successful and thus rolled
back. We could change this to work more like SQLite's original ABORT
(keep preceding changes in the transaction), but it is too risky to
change the behavior of ABORT since it is the default option of @Insert
and @Update. This CL just clarifies the behavior in Javadoc.
OnConflictStrategy.FAIL is no different from ABORT because of the
wrapping transaction. We should just remove it for clarity.
REPLACE and IGNORE are working fine.
Bug: 117266738
Test: OnConflictStrategyTest
Change-Id: Id62920d531afe4644d9d37ffc823a65132b54b4f
M room/common/api/2.1.0-alpha03.txt
M room/common/api/current.txt
M room/common/src/main/java/androidx/room/Insert.java
M room/common/src/main/java/androidx/room/OnConflictStrategy.java
M room/common/src/main/java/androidx/room/Update.java
A room/integration-tests/testapp/src/androidTest/java/androidx/room/integration/testapp/test/OnConflictStrategyTest.java
Description
Version used: 2.0.0
Devices/Android versions reproduced on: Wileyfox Swift (API 25)
Not reproduced on Android Emulator API 15 (probably due to different SQLite version).
SQLite constraint violation on method annotated with @Insert(onConflict = OnConflictStrategy.ROLLBACK) causes SQLiteException instead of SQLiteConstraintException.
Stack trace example:
E/AndroidRuntime: FATAL EXCEPTION: Thread-8
Process: gmk57.roomtest, PID: 2152
android.database.sqlite.SQLiteException: cannot rollback - no transaction is active (code 1)
at android.database.sqlite.SQLiteConnection.nativeExecute(Native Method)
at android.database.sqlite.SQLiteConnection.execute(SQLiteConnection.java:555)
at android.database.sqlite.SQLiteSession.endTransactionUnchecked(SQLiteSession.java:439)
at android.database.sqlite.SQLiteSession.endTransaction(SQLiteSession.java:401)
at android.database.sqlite.SQLiteDatabase.endTransaction(SQLiteDatabase.java:524)
at androidx.sqlite.db.framework.FrameworkSQLiteDatabase.endTransaction(FrameworkSQLiteDatabase.java:90)
at androidx.room.RoomDatabase.endTransaction(RoomDatabase.java:266)
at gmk57.roomtest.DataDao_Impl.insertRollback(DataDao_Impl.java:99)
at gmk57.roomtest.MainActivity.violateRollback(MainActivity.java:67)
Taking into account that
1) Room wraps methods annotated with @Insert, @Update and @Transaction in explicit transaction, and
2) ABORT, FAIL and ROLLBACK all throw SQLiteConstraintException, effectively rolling back this transaction
- the broader question is: is there any reason to provide FAIL and ROLLBACK options at all?
I also think the documentation should clearly describe the behavior of each strategy in specific case of Android + Room (e.g. which strategies throw an exception and which do not), instead of just linking to partly irrelevant SQLite documentation.
Sample project attached.