General development suggestions

[vhakulinen]

Responding to https://github.com/margelo/react-native-quick-sqlite/pull/55#issuecomment-2391285662, since bloating that PR with extra hopes and dreams wouldn't be productive.

• [ ] Concurrency. The sqlite connection is currently opened in serialized mode, which means (afaik) that threadpool created for each connection is basically useless. In serialized mode only one operation can be executed at any given time, which means that only one thread in the thread pool is able to do any sql work. Lifting the serialized mode with the current design won't do any good, because you would need to duplicate all the locking work that sqlite is currently doing (so you gain nothing). Best to just drop the thread pool and use one background thread per connection.
• [ ] Only first statement is executed, rest is (silently) ignored. Excluding the silently ignoring stuff this is generally fine, but it would be really useful to have API for executing all the sql statements in a string (think of large schema migration files).
• [ ] Extra copying, part of which is mentioned in #30
• [ ] Disjoint between cpp <-> js w.r.t. the implementation. There is extra book keeping of open connections, "transactions" are implemented in js side which sounds like a leaky abstraction. Probably other things that I don't remember at the moment.
• [ ] No way to hold references to prepared statements. Not the most important thing, but has performance implications.
• [ ] General API improvements (named parameters, possibility to get returned rows as objects along with arrays, etc..?)

W.r.t. Concurrency. It would be nice to be able to have a read only connection pool and the one connection (or another connection pool) for writing. This could be a huge performance win for more complex application that has heavy database usage.

Having studied this code base (and having had my own, long abandoned venture to alternative, sqlite-jsi), I have some idea of the effort required to implement these things. While I dont have the time / resources to work on this myself (nor am I expecting anyone to do the work either!), if someone is up for the task I am more than happy to chat about the ideas / design / implementation details.

As the sqlite-jsi repo might suggest, I have interest in having a solid sqlite library for react-native - I just lack the time to work on it.

[chrispader]

Thanks for all the input! I'm going to try improve the codebase and implement most of this in the next weeks! 🚀

We might plan on doing a greater API redesign anyway, so some of these points might already be covered by other changes

[mrousavy]

💯

[vhakulinen]

Oh, another quite thing: testing support!

Running tests on another runtime (i.e. not hermes) is PITA, mainly because the current API diverges from the "normal" so much. With better API testing would be easier (because you can implement better mocks / adapters for other runtimes).

At the moment I'm using the following mock, but only does the bare minimum. This mock uses better-sqlite3 node package, but I suppose the same could be adapted to other runtimes too. Because quick-sqlite doesn't support named parameters, and better-sqlite3 doesnt support the ?NNNN syntax, I can't use all our queries in our tests at the moment.

import BetterDB from 'better-sqlite3';
import { QuickSQLiteConnection } from 'react-native-quick-sqlite';

/**
 * Partially adapts better-sqlite3 to the quick-sqlite interface.
 */
export const adaptBetterSqlite = (conn: BetterDB.Database): QuickSQLiteConnection => {

  const execute: QuickSQLiteConnection['execute'] = (query, params) => {
    const stmt = conn.prepare(query);

    if (stmt.reader) {
      const rows = stmt.all(params || []);

      return {
        rowsAffected: rows.length,
        rows: {
          length: rows.length,
          _array: rows,
          item: (i: number) => rows[i],
        }
      };
    } else {
      const rows = stmt.run(params || []);

      return {
        rowsAffected: rows.changes,
        rows: {
          length: 0,
          _array: [],
          item: (i: number) => [][i],
        }
      };
    }
  };

  const executeAsync: QuickSQLiteConnection['executeAsync'] = (query: string, params?: unknown[]) => {
    return Promise.resolve(execute(query, params));
  };

  const executeBatch: QuickSQLiteConnection['executeBatch'] = tuples => {
    let n = 0;

    for (const tuple of tuples) {
      const stmt = conn.prepare(tuple[0]);

      const res = stmt.run(tuple[1] || []);

      n += res.changes;
    }

    return { rowsAffected: n };
  };

  const executeBatchAsync: QuickSQLiteConnection['executeBatchAsync'] = tuples => {
    return Promise.resolve(executeBatch(tuples));
  };

  const transaction: QuickSQLiteConnection['transaction'] = async fn => {
    const commit = () => {
      return execute('COMMIT');
    };

    const rollback = () => {
      return execute('ROLLBACK');
    };

    try {
      execute('BEGIN TRANSACTION');

      await fn({
        commit,
        execute,
        executeAsync,
        rollback,
      });

      commit();
    } catch (err) {
      rollback();
      throw err;
    }
  };

  const notImplemented = () => {
    throw new Error('not implemented');
  };

  return {
    execute,
    executeAsync,
    executeBatch,
    executeBatchAsync,
    transaction,

    close: notImplemented,
    delete: notImplemented,
    detach: notImplemented,
    attach: notImplemented,
    loadFile: notImplemented,
    loadFileAsync: notImplemented,
  };
};

[vhakulinen]

Another performance related issue: cancellation.

Say you have a view that loads data for ~0.5s from the database, but the user can press a button to change the query parameters (for example, time frame) faster that the query executes. This will currently cause the queries to queue up and delay the UI quite a bit.