Skip to content

Added refreshSchema() #57

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

Merged
merged 9 commits into from
Aug 20, 2024
Merged

Added refreshSchema() #57

merged 9 commits into from
Aug 20, 2024

Conversation

Chriztiaan
Copy link
Contributor

@Chriztiaan Chriztiaan commented Aug 13, 2024

Updating the schema can causing queries and watch calls to become stale as they would use outdated source tables from a previous version of the schema. This PR adds the ability to explicitly update the version of the schema used for existing queries and watch calls.

Copy link
Contributor

@rkistner rkistner left a comment

Choose a reason for hiding this comment

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

The build checks are failing due to not supporting the lastest drift version. It's safe to increase the pana score threshold for now, until we properly fix the compatibility version later.

@Chriztiaan Chriztiaan changed the title Added refreshSchema() to SqliteConnection interface. Added refreshSchema() and exclusiveLock() to SqliteDatabaseMixin Aug 19, 2024
@Chriztiaan Chriztiaan marked this pull request as ready for review August 19, 2024 09:25
@Chriztiaan Chriztiaan changed the title Added refreshSchema() and exclusiveLock() to SqliteDatabaseMixin Added refreshSchema() Aug 19, 2024
@Chriztiaan
Copy link
Contributor Author

Chriztiaan commented Aug 19, 2024

@rkistner @stevensJourney I have removed the exclusiveLock implementation for the moment. If there are concerns in the powersync package, we can instead depend on a lock there.

This version of the PR looks much cleaner than the first iteration so I am happy about it.
Just not 100% sure about putting the default refreshSchema implementation in the SqliteQueries mixin.

@Chriztiaan
Copy link
Contributor Author

@rkistner @stevensJourney I have removed the exclusiveLock implementation for the moment. If there are concerns in the powersync package, we can instead depend on a lock there.

This version of the PR looks much cleaner than the first iteration so I am happy about it. Just not 100% sure about putting the default refreshSchema implementation in the SqliteQueries mixin.

I realised with this implementation, I don't have access to refreshSchema in transactions - which would be nice as it would allow us to pair an updateSchema and refreshSchema inside a lock/transaction in the PowerSync package.

@rkistner
Copy link
Contributor

I realised with this implementation, I don't have access to refreshSchema in transactions - which would be nice as it would allow us to pair an updateSchema and refreshSchema inside a lock/transaction in the PowerSync package.

I think that's expected. Transactions can't span multiple connections, so there is no way to refresh the schema across connections in one transaction. And if you specifically want that command within a transaction, you can just call the PRAGMA statement directly.

rkistner
rkistner previously approved these changes Aug 19, 2024
@Chriztiaan Chriztiaan force-pushed the feature/refresh-schema branch from 92fdbb4 to 31c6989 Compare August 20, 2024 08:23
@Chriztiaan Chriztiaan merged commit 0623505 into main Aug 20, 2024
6 checks passed
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.

2 participants