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

Duplicate seq for the same db_version #380

Open
jeromegn opened this issue Oct 5, 2023 · 1 comment
Open

Duplicate seq for the same db_version #380

jeromegn opened this issue Oct 5, 2023 · 1 comment

Comments

@jeromegn
Copy link
Contributor

jeromegn commented Oct 5, 2023

There's at least one scenario where duplicate seq values may exist for the same db_version.

Reproduction:

sqlite> create table foo (a INTEGER PRIMARY KEY NOT NULL, b INT);
sqlite> select crsql_as_crr('foo');
sqlite> .mode column
sqlite> insert into foo values (1, 1);
sqlite> insert into foo values (2, 2);
sqlite> insert into foo values (3, 3);
sqlite> select crsql_db_version();
crsql_db_version()
------------------
3
sqlite> begin;
sqlite> insert into crsql_changes values ("foo", X'010904', "b", 4, 1, 1, X'9B1D5BD20C7A47C28FC482B33DCFC412', 1, 0);
sqlite> insert into crsql_changes values ("foo", X'010905', "b", 5, 1, 2, X'9B1D5BD20C7A47C28FC482B33DCFC412', 1, 0);
sqlite> select crsql_next_db_version();
crsql_next_db_version()
-----------------------
4
sqlite> commit;
sqlite> select "table", hex(pk), cid, val, col_version, db_version, hex(coalesce(site_id, crsql_site_id())), cl, seq from crsql_changes;
table  hex(pk)  cid  val  col_version  db_version  hex(coalesce(site_id, crsql_site_id()))  cl  seq
-----  -------  ---  ---  -----------  ----------  ---------------------------------------  --  ---
foo    010901   b    1    1            1           1864106555524AA0B5FA926536A7C047         1   0
foo    010902   b    2    1            2           1864106555524AA0B5FA926536A7C047         1   0
foo    010903   b    3    1            3           1864106555524AA0B5FA926536A7C047         1   0
foo    010904   b    4    1            4           9B1D5BD20C7A47C28FC482B33DCFC412         1   0
foo    010905   b    5    1            4           9B1D5BD20C7A47C28FC482B33DCFC412         1   0
@tantaman
Copy link
Collaborator

tantaman commented Oct 6, 2023

We decided that the appropriate fix here is to increment db_version each time we see a new db_version in the current transaction rather than only incrementing if the seen version is greater than the current version.

Todo:

  • Extend ext_data to keep track of db_versions seen in the current commit to crsql_changes
  • Update crsql_next_db_version(x) to bump the version if the provided version x was not yet seen

We currently only bump versions iff:

  • it is the first call to crsql_next_db_version
  • the provided value x is greater than the currently assigned next_db_version

Separately, we should be assigning to x + 1 rather than just x if x is greater than the currently assigned next_db_vesrion

tantaman added a commit that referenced this issue Oct 12, 2023
tantaman added a commit that referenced this issue Oct 30, 2023
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

No branches or pull requests

2 participants