-
Notifications
You must be signed in to change notification settings - Fork 261
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
Limbo does not enforce primary key uniqueness on PKs that are not rowid aliases #472
Labels
bug
Something isn't working
Comments
This was referenced Dec 21, 2024
penberg
added a commit
that referenced
this issue
Dec 27, 2024
… Jussi Saurio Closes #436 This PR fixes four issues: 1. Not respecting user-provided column names (e.g. `INSERT INTO foo (b,c) values (1,2);` would just insert into the first two columns regardless of what index `b` and `c` have) 2. Limbo would get in an infinite loop when inserting too many values (too many i.e. more columns than the table has) 3. False positive unique constraint error on non-primary key columns when inserting multiple values, e.g. ``` limbo> create table t1(v1 int); limbo> insert into t1 values (1),(2); Runtime error: UNIQUE constraint failed: t1.v1 (19) ``` as seen [here](#490 (comment) ent-2545545562) 4. Limbo no longer uses a coroutine for INSERT when only inserting one row. See [this comment](#43 6#issuecomment-2533937845). For the equivalent query, Limbo now generates: ``` limbo> EXPLAIN INSERT INTO users (name, email) VALUES ('John Doe', '[email protected]'); addr opcode p1 p2 p3 p4 p5 comment ---- ----------------- ---- ---- ---- ------------- -- ------- 0 Init 0 10 0 0 Start at 10 1 OpenWriteAsync 0 2 0 0 2 OpenWriteAwait 0 0 0 0 3 String8 0 3 0 John Doe 0 r[3]='John Doe' 4 String8 0 4 0 [email protected] 0 r[4]='[email protected]' 5 NewRowId 0 1 0 0 6 MakeRecord 2 3 5 0 r[5]=mkrec(r[2..4]) 7 InsertAsync 0 5 1 0 8 InsertAwait 0 0 0 0 9 Halt 0 0 0 0 10 Transaction 0 1 0 0 11 Null 0 2 0 0 r[2]=NULL 12 Goto 0 1 0 0 ``` --- Note that this PR doesn't fix e.g. #472 which requires creating an index on the non-rowid primary key column(s), nor does it implement rollback (e.g. inserting two rows where one fails to unique constraint still inserts the other row) --- **EXAMPLES OF ERRONEOUS BEHAVIOR -- current head of main:** wrong column inserted ``` limbo> create table rowidalias_b (a, b INTEGER PRIMARY KEY, c, d); limbo> insert into rowidalias_b (d) values ('d only'); limbo> select * from rowidalias_b; d only|1|| <-- gets inserted into column a ``` wrong column inserted ``` limbo> create table textpk (a, b text primary key, c); limbo> insert into textpk (a,b,c) values ('a','b','c'); limbo> select * from textpk; a|b|c limbo> insert into textpk (b,c) values ('b','c'); limbo> select * from textpk; a|b|c b|c| <--- b gets inserted into column a ``` false positive from integer check due to attempting to insert wrong column ``` limbo> create table rowidalias_b (a, b INTEGER PRIMARY KEY, c, d); limbo> insert into rowidalias_b (a,c) values ('lol', 'bal'); Parse error: MustBeInt: the value in the register is not an integer <-- tries to insert c into b column ``` false positive from integer check due to attempting to insert wrong column ``` limbo> CREATE TABLE users ( id INTEGER PRIMARY KEY, name TEXT, email TEXT ); limbo> INSERT INTO users (name, email) VALUES ('John Doe', '[email protected]'); Parse error: MustBeInt: the value in the register is not an integer. <-- tries to insert name into id column ``` allows write of nonexistent column ``` limbo> create table a(b); limbo> insert into a (nonexistent_col) values (1); limbo> select * from a; 1 ``` hangs forever when inserting too many values ``` limbo> create table a (b integer primary key); limbo> insert into a values (1,2); <-- spinloops forever at 100% cpu ``` unique constraint error on non-unique column ``` limbo> create table t1(v1 int); limbo> insert into t1 values (1),(2); Runtime error: UNIQUE constraint failed: t1.v1 (19) ``` **EXAMPLES OF CORRECT BEHAVIOR -- this branch:** correct column inserted ``` limbo> create table rowidalias_b (a, b INTEGER PRIMARY KEY, c, d); limbo> insert into rowidalias_b (d) values ('d only'); limbo> select * from rowidalias_b; |1||d only ``` correct column inserted ``` limbo> create table textpk (a, b text primary key, c); limbo> insert into textpk (a,b,c) values ('a','b','c'); limbo> select * from textpk; a|b|c limbo> insert into textpk (b,c) values ('b','c'); limbo> select * from textpk; a|b|c |b|c ``` correct columns inserted, PK autoincremented ``` limbo> create table rowidalias_b (a, b INTEGER PRIMARY KEY, c, d); limbo> insert into rowidalias_b (a,c) values ('lol', 'bal'); limbo> select * from rowidalias_b; lol|1|bal| ``` correct column inserted, PK autoincremented ``` limbo> CREATE TABLE users ( id INTEGER PRIMARY KEY, name TEXT, email TEXT ); limbo> INSERT INTO users (name, email) VALUES ('John Doe', '[email protected]'); limbo> select * from users; 1|John Doe|[email protected] ``` reports parse error correctly about wrong number of values ``` limbo> create table a (b integer primary key); limbo> insert into a values (1,2); Parse error: table a has 1 columns but 2 values were supplied ``` reports parse error correctly about nonexistent column ``` limbo> create table a(b); limbo> insert into a (nonexistent_col) values (1); Parse error: table a has no column named nonexistent_col ``` no unique constraint error on non-unique column ``` limbo> create table t1(v1 int); limbo> insert into t1 values (1),(2); limbo> select * from t1; 1 2 ``` **Also, added multi-row inserts to simulator and ran into at least this:** ``` Seed: 9444323279823516485 path to db '"/var/folders/qj/r6wpj6657x9cj_1jx_62cpgr0000gn/T/.tmpcYczRv/simulator.db"' Initial opts SimulatorOpts { ticks: 3474, max_connections: 1, max_tables: 79, read_percent: 61, write_percent: 12, delete_percent: 27, max_interactions: 2940, page_size: 4096 } thread 'main' panicked at core/storage/sqlite3_ondisk.rs:332:36: called `Result::unwrap()` on an `Err` value: Corrupt("Invalid page type: 83") ``` Closes #533
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The text was updated successfully, but these errors were encountered: