Skip to content
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

[Fix] Table change updates in write transactions #13

Merged
merged 10 commits into from
Jan 24, 2024

Conversation

stevensJourney
Copy link
Collaborator

@stevensJourney stevensJourney commented Jan 22, 2024

This PR fixes an issue where table change updates registered via registerUpdateHook would trigger from writeTransactions before the changes have been committed or rolled back.

If a client used the update hook to trigger a read from a read connection the updated data would not yet be visible. This causes inconsistent results when implementing watched queries.

This PR monitors the state of write transactions in order to emit update events only once the changes have been committed. This work is looseley based of the implementation here https://github.com/powersync-ja/sqlite_async.dart/blob/f994e165d5410c9ccca9a187de9c51fcedfd1182/lib/src/sqlite_connection_impl.dart#L310.

This PR also adds extra listeners which emit events for all table changes and write transaction events. This gives users access to table changes during transactions and allows for custom logic for deferring table change events during transactions.

@stevensJourney stevensJourney marked this pull request as ready for review January 22, 2024 13:50
@stevensJourney stevensJourney changed the title [Fix] Table change updates in write locks [Fix] Table change updates in write transactions Jan 22, 2024
Comment on lines 48 to 49
private _writeTransactionActive: boolean;
private updateBuffer: UpdateNotification[];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpic: Use a consistent style for private members. (Personally using a _ prefix feels unnecessary, because we are accessing it with this.)

rkistner
rkistner previously approved these changes Jan 23, 2024
@stevensJourney
Copy link
Collaborator Author

Automated tests are currently a bit broken. The Android emulator seems to be freezing at random intervals. Sometimes before tests begin and sometimes during tests.
Tests have been verified locally on Android and iOS
image
image
Watched queries have also been verified in here powersync-ja/powersync-js#43

@stevensJourney stevensJourney merged commit 3bb0212 into main Jan 24, 2024
1 of 2 checks passed
@stevensJourney stevensJourney deleted the fix/write-transaction-table-updates branch January 24, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants