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

Use BigInt for OpId internally #209

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Use BigInt for OpId internally #209

wants to merge 9 commits into from

Conversation

rkistner
Copy link
Contributor

@rkistner rkistner commented Feb 19, 2025

For our JSON protocol, we use strings to represent OpIds, since JS cannot reliably parse large numbers in JSON.

However, for internal usage, there was a mixture of string and bigint for that, leading in conversions between the two everywhere. This switches over to bigint in most places, only converting to return a response to the client. This results in less conversion between the two.

There are a small number of places where we use the large protocol types internally, and these do still use the string version.

This also fixes a couple of places where not-null or casts were incorrectly used, resulting in unexpected type mismatches at runtime. This specifically fixes an issue of MySQL BigLogStream tests not being setup correctly, as well as a race condition when stopping MySQL replication immediately after starting it.

Copy link

changeset-bot bot commented Feb 19, 2025

🦋 Changeset detected

Latest commit: 27b5266

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@powersync/service-module-postgres-storage Minor
@powersync/service-module-mongodb-storage Minor
@powersync/service-core-tests Minor
@powersync/service-module-postgres Minor
@powersync/service-module-mongodb Minor
@powersync/service-core Minor
@powersync/service-module-mysql Minor
@powersync/lib-service-postgres Minor
@powersync/service-image Minor
test-client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@rkistner rkistner changed the title [WIP] Use BigInt for OpId internally Use BigInt for OpId internally Feb 20, 2025
@rkistner rkistner marked this pull request as ready for review February 20, 2025 10: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.

1 participant