vlcn-io / cr-sqlite

Convergent, Replicated SQLite. Multi-writer and CRDT support for SQLite
https://vlcn.io
MIT License
2.77k stars 76 forks source link

bug report | null primary keys fail to sync #354

Closed tantaman closed 1 year ago

tantaman commented 1 year ago

example of the error: https://discord.com/channels/989870439897653248/1022913265946349690/1152224235461287977

reported by @jmatsushita

tantaman commented 1 year ago

Null primary keys violate unique constraints --

image

We can't support a null primary key and no DB actually does besides SQLite. To add validation on crr creation to deny making crrs that have null primary keys.

Azarattum commented 1 year ago

So, crsqlite now has to require NOT NULL on primary key definition, right? What about tables with multiple primary keys? Is it ok if only one of pk columns is nullable?

Azarattum commented 1 year ago

Oh. The original report was because of the composite keys... All the pk columns should be NOT NULL then.

tantaman commented 1 year ago

Yeah, I think all columns need to be not null.

Even allowing a single primary key column to be null gives bogus behavior:

sqlite> create table baz (a integer not null, b integer, primary key(a, b));
sqlite> insert into baz values (1, null);
sqlite> insert into baz values (1, null);
sqlite> insert into baz values (1, null);
sqlite> insert into baz values (1, 1);
sqlite> insert into baz values (1, 1);
Runtime error: UNIQUE constraint failed: baz.a, baz.b (19)
sqlite> select * from baz;
┌───┬──────┐
│ a │  b   │
├───┼──────┤
│ 1 │ NULL │
│ 1 │ NULL │
│ 1 │ NULL │
│ 1 │ 1    │
└───┴──────┘
sqlite>