powersync-ja/powersync.dart

`watch` not working properly

JCKodel opened this issue · 3 comments

I have this code:

Future<Stream<ResultSet>> watch(
  String query,
  Map<String, Object?> parameters, [
  List<String>? triggerOnTables,
]) async {
  final db = await _getDb();
  final (q, p) = _convertQuery(query, parameters);

  return db.database.watch(
    q,
    parameters: p,
    triggerOnTables: triggerOnTables,
  );
}

I'm calling it with:

watch(
  "SELECT * FROM users WHERE id = @id LIMIT 1;",
  {"id": 1},
  ["users", "user_settings"],
);

(don't worry about the @id, consider it is a valid SQLite query)

The watch is made by sqlite_async, on this line: https://github.com/powersync-ja/sqlite_async.dart/blob/26315e1e5c8e9b7ea25c70db93e0cfe5604bf888/lib/src/update_notification.dart#L62

Problem is: power sync changes the names of the tables and give us views emulating our schema, so this watch fails, because user and user_settings should be ps_data__user and ps_data__user_settings.

I know it is not really ps fault, but this should be handled somehow... maybe encapsulating sqlite_async and providing the same interface, with some safeguards (such as correcting those table names)?

Thanks for the report, we'll look into ways we can handle this better.

Is there a reason you need to pass in the table names explicitly, instead of relying on the built-in query parsing to get the tables from the query?

If you do need to pass it in explicitly, is it an option to use the full table names for now, e.g. ps_data__user? One workaround would be to do this in your watch function - if you watch on ["users", "user_settings", "ps_data__user", "ps_data__user_settings", "ps_data_local__user", "ps_data_local__user_settings"], it will cover all the permutations of standard tables and PowerSync views, and the performance impact should be minimal. This may be the workaround we end up implementing in the library itself.

Is there a reason you need to pass in the table names explicitly

Yes. My professional speciality: smelly code 🤫 (I fixed now, but that's what made me realize the problem).

I had a watch on my authentication service, that is also responsible for loading user settings. So I was using that watch to trigger the whole authentication user update mechanism that would also load the user settings (so, basically, when user_settings changed, the watch on SELECT * FROM users WHERE id = ?1 had to be triggered).

Now I'm doing proper separation (an entire separated theme service with its own watch on user_settings).

TL;DR: while tinkering some solution, I came across that issue (and that's precisely why that argument exists: to hint tables that are not available in our query (after all, there are triggers that can make a query change another table)).