Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix: core/translate/insert: fix four issues with inserts #533

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

Conversation

jussisaurio
Copy link
Collaborator

@jussisaurio jussisaurio commented Dec 21, 2024

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

  1. Limbo no longer uses a coroutine for INSERT when only inserting one row. See this comment. For the equivalent query, Limbo now generates:
limbo> EXPLAIN INSERT INTO users (name, email) VALUES ('John Doe', 'john@example.com');
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     john@example.com  0   r[4]='john@example.com'
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', 'john@example.com');
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', 'john@example.com');
limbo> select * from users;
1|John Doe|john@example.com

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")

@jussisaurio jussisaurio changed the title Fix some issues with inserts Fix: core/translate/insert: fix three issues with inserts Dec 21, 2024
@jussisaurio jussisaurio changed the title Fix: core/translate/insert: fix three issues with inserts Fix: core/translate/insert: fix four issues with inserts Dec 21, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic primary key
1 participant