op-sqlite icon indicating copy to clipboard operation
op-sqlite copied to clipboard

Reactive queries sometimes return a corrupt DB error

Open ospfranco opened this issue 1 year ago • 6 comments

Describe the bug When you have a reactive query and try to execute a transaction that deletes data, the reactive query kaputs itself (and maybe the transaction?)

Reproducible example.

It seems the problem is executing the query again in the middle of the steps in the transaction (maybe only deletes or indexes or foreign keys? because otherwise it seems to work quite well.).

One possible cause:

The update hook implementation must not do anything that will modify the database connection that invoked the update hook. Any actions to modify the database connection must be deferred until after the completion of the sqlite3_step() call that triggered the update hook. Note that sqlite3_prepare_v2() and sqlite3_step() both modify their database connections for the meaning of "modify" in this paragraph.

I've tried adding a mutex but to no avail since everything runs on the same thread and just skipping until the batch operation has finished is also no good since only updates trigger the reactive query. This means a different implementation of the reactive queries is necessary.

At the moment, the update hook triggers the re-execution of the prepared query the moment it is called, which will break the rule above. We need to queue the statements and execute them until it is safe to do so, so a major re-engineering effort is necessary.

ospfranco avatar Jul 30 '24 01:07 ospfranco

Hi @ospfranco is there any workaround for this? Also, I am facing this issue is without transaction as well while using normal db.execute, so I think it's not specific to transactions only.

rvibit avatar Aug 28 '24 18:08 rvibit

no, no workaround at the moment. I need to re-write how the callbacks are executed, I just don't have the time for it.

ospfranco avatar Aug 28 '24 18:08 ospfranco

no, no workaround at the moment. I need to re-write how the callbacks are executed, I just don't have the time for it.

I was just trying different ways and added this hacky code for now and working fine for my use case.

db.reactiveExecute({
  query: query, //a query with left join
  arguments: [],
  fireOn: [
    {
      table: TableName.ACCOUNTS,
    },
    {
      table: TableName.TRANSACTIONS,
    },
  ],
  callback: (res) => {
    try {
//res is currpt DB here in delete case so added a condition to execute the query manually in else block
      const accounts = res?.rows?._array;
      if (accounts) {
        defaultStore.set(accountsAtom, accounts);
      } else {
        const data = db.execute(query).rows?._array;
        defaultStore.set(accountsAtom, data);
      }
    } catch (error) {
      console.log(error);
    }
  },
});

rvibit avatar Aug 28 '24 18:08 rvibit

I am also experiencing crashes, but the try catch did not fix my problem.

devYonz avatar Sep 08 '24 21:09 devYonz

I am also experiencing crashes, but the try catch did not fix my problem.

Try catch is not solving the problem , the if else condition is the key, when the crash happenes accounts is undefined so wrapping it in if worked for me

rvibit avatar Sep 09 '24 03:09 rvibit

My situation is that the whole app crashes, next I kept needing to have executed an regular query before being able to use reactive query. We finally have a stable reactive query after we turned off the thread unsafe perf mode and started using the cr-sqlite binary. In other wods, set your package.json to:

  "op-sqlite": {
    "sqlcipher": false,
    "crsqlite": true,  <-------------   Important
    "performanceMode": "2", <---- Important
    "iosSqlite": false,
    "sqliteFlags": "-DSQLITE_DQS=0",
    "fts5": false,
    "libsql": false,
    "sqliteVec": true
  }
}

To be safe, nuke ad re-create your ios & android folders when you switch from regular sqlite to cr-sqlite

devYonz avatar Sep 12 '24 23:09 devYonz