pushtorefresh / storio

Reactive API for SQLiteDatabase and ContentResolver.
Apache License 2.0
2.55k stars 182 forks source link

Wrong get result #731

Open aspoliakov opened 7 years ago

aspoliakov commented 7 years ago

such a case:

  1. Get something from the database (the table 'Accounts' for example)
  2. Alert table (add new column) using the following:

.executeSQL() .withQuery( RawQuery.builder() .query("ALTER TABLE Accounts ADD COLUMN Account5 INTEGER DEFAULT 0") .build()) .prepare();

  1. The next get query anywhere in the application returns wrong result (returns data without new column). But the following query in the database gives the correct result.

Manipulations with observesTables()/affectsTables() did not yield results. However, when I open the database using SQLiteBrowser after "ALTER TABLE..." I see that the column really added. A feeling that StorIO did't know about the changes in the table and gives first result incorrect.

nikitin-da commented 7 years ago

Hi, @S1owaway ! There is the same behavior with the "raw" sqlOpenHelper:

try(Cursor cursor = dbProvider.sqLiteOpenHelper.getReadableDatabase().query("accounts", null, null, null, null, null, null, null)) {
    cursor.moveToFirst();
    assertThat(cursor.getColumnIndex("account5")).isEqualTo(-1);
}

dbProvider.sqLiteOpenHelper.getWritableDatabase().execSQL("ALTER TABLE accounts ADD COLUMN account5 INTEGER DEFAULT 1");

try(Cursor cursor = dbProvider.sqLiteOpenHelper.getReadableDatabase().query("accounts", null, null, null, null, null, null, null)) {
    cursor.moveToFirst();
    assertThat(cursor.getColumnIndex("account5")).isEqualTo(-1);
}

try(Cursor cursor = dbProvider.sqLiteOpenHelper.getReadableDatabase().query("accounts", null, null, null, null, null, null, null)) {
    cursor.moveToFirst();
    assertThat(cursor.getInt(cursor.getColumnIndexOrThrow("account5"))).isEqualTo(1);
}

On the sqlfiddle SQLite (SQL.js) all works fine http://sqlfiddle.com/#!5/725e6/8

aspoliakov commented 7 years ago

Спасибо за ответ. Поведение в представленном вами примере понятно и логично. А вот поведение StorIO непонятно. Либо это бага, либо фича такая интересная. Обидно, так как библиотека уже вплотную используется в немаленьком проекте. Неужели придется переезжать на что нибудь другое. Опишу еще раз проблему, возможно меня не так поняли. Есть таблица, например с 4мя столбцами, первые 2 статические, остальные динамические, и пока добавлено 2 динамических - account1 и account2. Я делаю get запрос и получаю данные по account1 и account2. Потом я на другом экране добавляю account3 (посредством ALERT TABLE add column...) и делаю еще раз get запрос. Вижу выгруженные данные по account1 и account2, хотя ожидаю увидеть теперь и account3. Затем опять делаю get запрос следом за предыдущим, и вот со второго раза получаю уже полную картину с account1 account2 account3. Такое поведение крайне неудобно, потому что запрос данных сразу после ALERT TABLE завязан на бизнес логику. Пока наделал костылей, чтобы после ALERT TABLE фейково вызывался get запрос (andThen(getData().asSingle().toCompletable())) и никуда ничего не возвращал, а следующий get запрос, который вызывает бизнес-логика - давал уже верный результат. Но обнаружил другую проблему. После ALERT TABLE мне не всегда необходимо сделать именно тот get запрос который я делал до ALERT TABLE. Допустим, сначала я вызвал getData(), потом в каком нибудь другом месте приложения я вызвал getDataOrderedByDate(), потом добавил колонку (ALERT TABLE), а после добавления колонки опять пытаюсь взять данные, но оба метода дадут неправильный результат при первом запросе, и только при втором дадут правильный. Выливается это в то, что я при реализации логики ALERT TABLE должен в andThen фейково вызывать все виды getData в DAO для данной таблицы, чтобы следующий запрос который реально понадобится был верный. Описал очень подробно, надеюсь в этом можно будет разобраться. Спасибо

nikitin-da commented 7 years ago

Can we please continue in english to keep issue available to all interested?

First of all StrIO uses sqlOpenHelper under the hood. As I said before in the example I met the same behavior in sqlOpenHelper. I also read the value twice. And first time it was not updated.

try(Cursor cursor = dbProvider.sqLiteOpenHelper.getReadableDatabase().query("accounts", null, null, null, null, null, null, null)) {
    cursor.moveToFirst();
    assertThat(cursor.getColumnIndex("account5")).isEqualTo(-1);
}

dbProvider.sqLiteOpenHelper.getWritableDatabase().execSQL("ALTER TABLE accounts ADD COLUMN account5 INTEGER DEFAULT 1");

try(Cursor cursor = dbProvider.sqLiteOpenHelper.getReadableDatabase().query("accounts", null, null, null, null, null, null, null)) {
    cursor.moveToFirst();
    assertThat(cursor.getColumnIndex("account5")).isEqualTo(-1);   // value was not updated
}

try(Cursor cursor = dbProvider.sqLiteOpenHelper.getReadableDatabase().query("accounts", null, null, null, null, null, null, null)) {
    cursor.moveToFirst();
    assertThat(cursor.getInt(cursor.getColumnIndexOrThrow("account5"))).isEqualTo(1);
}

I havn't investigate the cause of this problem yet, but can't you instead of adding columns for every account decompose this table and add rows instead. Otherwise you can can met performance problem too.

aspoliakov commented 7 years ago

Thank you for your help. It seems the problem is really on the SQLite level. In my case, add a column for the table is the only way. Fortunatly it is the very rare operation. Thank you for StorIO.

P.S.: by the way, using the library I faced with some discomfort. When I update some field in the table, I want to know where exactly this update happened (particular id). The current table observers are not emit data (it works only on adding and removing fields). To solve this problem i use the EventBus in bunch with observables in the DAO class. This is not very convenient.

artem-zinnatullin commented 7 years ago

We can attach Query that caused Change, but it might not be helful in cases like DELETE * WHERE id LIKE %s% FROM users since we don't know which rows will be affected by such queries so we decided to not include such feature into initial design.