Was running the sim with I/O faults enabled and fixed some nasty bugs.
Now, there are some more nasty bugs to fix as well. This is the command
that I use to run the simulator `cargo run -p limbo_sim -- --minimum-
tests 10 --maximum-tests 1000`
This PR mainly fixes the following bugs:
- Not decrementing in flight write counter when `pwrite` fails
- not rolling back the transaction on `step` error
- not rolling back the transaction on `run_once` error
- some functions were just being unwrapped when they could suffer io
errors
- Only change max_frame after wal sync's
Reviewed-by: Pere Diaz Bou <pere-altea@homail.com>
Reviewed-by: Pere Diaz Bou <pere-altea@homail.com>
Closes#1946
When an `UPDATE` statement modifies a table's `INTEGER PRIMARY KEY` (which acts as a `rowid` alias) alongside other indexed columns, the index entries were incorrectly retaining the old `rowid`. This led to stale index references, causing subsequent queries to return incorrect results.
This change ensures that when the `rowid` alias is part of the `SET` clause in an `UPDATE` statement, the new `rowid` value is used for generating and updating index records. This guarantees that all index entries correctly point to the updated row, resolving the data inconsistency.
Previously we implemented update as a simple `Delete` + `Insert`
procedure which seemed okay for the moment but it wasn't. `Delete` can
trigger balance and a post balance `seek` which will leave cursor
pointing to an invalid page which `Insert` will try to insert to.
We solve this by removing `Delete` from the execution plan and rely on
`Insert` to properly overwrite the cell where the rowid is the same as
the one we are inserting.
Fixes DELETE not emitting conditional jumps at all if the associated
WhereTerm is a constant, e.g.
```sql
limbo> create table t(x);
limbo> explain DELETE FROM t WHERE 5-5;
addr opcode p1 p2 p3 p4 p5 comment
---- ----------------- ---- ---- ---- ------------- -- -------
0 Init 0 7 0 0 Start at 7
1 OpenWrite 0 2 0 0 root=2; t
2 Rewind 0 6 0 0 Rewind table t
3 RowId 0 1 0 0 r[1]=t.rowid
4 Delete 0 0 0 0
5 Next 0 3 0 0
6 Halt 0 0 0 0
7 Transaction 0 1 0 0 write=true
8 Goto 0 1 0 0
```
I was adding more stuff to the simulator in a Branch of mine, and I
caught this error with delete. Upstreaming the fix here. As we do with
Update, I added the translation step for the `WhereTerms` of the query.
Edit: Closes#1732. Closes#1733. Closes#1734. Closes#1735. Closes
#1736. Closes#1738. Closes#1739. Closes#1740.
Edit: Also pushes constant where term translation to `init_loop` for
Update and Select as well.
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#1746
Previously, queries like:
```
SELECT
CASE WHEN c0 != 'x' THEN group_concat(c1, ',') ELSE 'x' END
FROM t0
GROUP BY c0;
```
would return incorrect results because c0 was not copied during the
aggregation loop into a register accessible to the logic processing the
grouped results (e.g., the CASE WHEN expression in this example).
The same issue applied to expressions in the HAVING and ORDER BY clauses.
Previously, the logic for collecting non-aggregate columns was duplicated
across multiple locations and implemented inconsistently. This caused a
bug that was revealed by the refactoring in this commit (see the added
test).
- `Update` query doesn't update `n_changes`. Let's make it work
- Add `InsertFlags` to add meta information related to insert operations
- For update query, add `UPDATE` flag
- Currently, the update query executes `Insn::Delete` and `Insn::Insert`
internally, it increases `n_change` by 2. So, for the update query,
let's skip increasing `n_change` for the `Insn::Insert`
https://github.com/tursodatabase/limbo/issues/1681
Reviewed-by: Pere Diaz Bou <pere-altea@homail.com>
Closes#1683
Again found when fuzzing nested where clause subqueries:
Aggregate registers need to be NULLed at the start because the same
registers might be reused on another invocation of a subquery, and if
they are not NULLed, the 2nd invocation of the same subquery will have
values left over from the first invocation.
Reviewed-by: Preston Thorpe (@PThorpe92)
Closes#1614
Currently we have this:
program.alloc_cursor_id(Option<String>, CursorType)`
where the String is the table's name or alias ('users' or 'u' in
the query).
This is problematic because this can happen:
`SELECT * FROM t WHERE EXISTS (SELECT * FROM t)`
There are two cursors, both with identifier 't'. This causes a bug
where the program will use the same cursor for both the main query
and the subquery, since they are keyed by 't'.
Instead introduce `CursorKey`, which is a combination of:
1. `TableInternalId`, and
2. index name (Option<String> -- in case of index cursors.
This should provide key uniqueness for cursors:
`SELECT * FROM t WHERE EXISTS (SELECT * FROM t)`
here the first 't' will have a different `TableInternalId` than the
second `t`, so there is no clash.
Currently we have some usages of LIMIT where the actual limit counter
is initialized next to the DecrJumpZero instruction, and then
`program.mark_last_insn_constant()` is used to hoist the counter
initialization to the beginning of the program.
This is very fragile, and already FROM clause subquery handling works
around this with a hack (removed in this PR), and (upcoming) WHERE clause
subqueries would also run into problems because of this, because the LIMIT
might need to be initialized once for every iteration of the subquery.
This PR removes those usages for LIMIT, and LIMIT processing is now more
intuitive:
- limit counter is now initialized at the start of the query processing
- a function init_limit() is extracted to do this for select/update/delete
