Skip to content

Commit

Permalink
Merge 'Fix: core/translate/insert: fix four issues with inserts' from…
Browse files Browse the repository at this point in the history
… 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
  • Loading branch information
penberg committed Dec 27, 2024
2 parents 548f66e + c4e2a34 commit cf1a3fb
Show file tree
Hide file tree
Showing 4 changed files with 343 additions and 121 deletions.
Loading

0 comments on commit cf1a3fb

Please sign in to comment.