-
Notifications
You must be signed in to change notification settings - Fork 12
Allow transforming table updates from sqlite_async. #102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
We can add tests if you guys are ok with this API |
Other 2 alternatives could be: /// Let the user transform the Stream however they wish
Stream<UpdateNotification> Function(Stream<UpdateNotification>)? transformTableUpdatesStream
/// Assume this is the only use case, and only allow changing the table name
String Function(String)? mapTableUpdateName |
I think it would be nice if this could be handled automatically, and I think we're trying to do that already so this might just be an implementation detail that needs to be fixed instead of a new API. We pipe the |
@simolus3 That would be ideal. Worth noting that the example doesn't suffer from this issue because the following function(https://github.com/powersync-ja/powersync.dart/blob/12488fbd6f4baeb72df3a520c471d1505cf2fb1a/packages/powersync_core/lib/src/database/powersync_db_mixin.dart#L596) internally gets the affected tables by doing a EXPLAIN query. It's only a problem when integrating with drift. In that case it would have to be an option when creating the PowerSync database? The prefix is a different one from ps_local__ despite its similarities. Only the end user knows how the local table was named. |
Also, related to my previous comment. If that function receives a non empty set of table names, even the demo would fail. The end user would have to write all watch queries that pass the |
// This is useful to map local table names from PowerSync that are backed by a view name | ||
// which is the entity that the user interacts with. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make this a dartdoc comment on the constructor itself?
It would also be good to have a small unit test for this scenario - I'd be happy to help with that too.
We found that when using optional sync feature (like in this example https://github.com/powersync-ja/powersync.dart/tree/main/demos/supabase-todolist-optional-sync) when the user is in local mode table updates coming from the stream will have the local_ prefix.
We need to have a way to convert them to the view name so that stream queries in drift work ok.
Not sure if there would be a better way to support this. Maybe in PowerSync core itself.