Backward scan of a table wasn't implemented yet in MVCC so this achieves
that. I added simple test for mixed btree and mvcc backward scan but I
should add more intense testing for this.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Implements backward scanning and last() in MVCC lazy cursor and adds
directional rowid iteration in the MVCC store, with new tests for mixed
MVCC+B-Tree backward scans.
>
> - **MVCC Cursor (`core/mvcc/cursor.rs`)**:
> - Implement `prev()` and `last()` with mixed MVCC/B-Tree
coordination using `IterationDirection`.
> - Add `PrevState` and extend state machine to handle backward
iteration.
> - Update `get_new_position_from_mvcc_and_btree(...)` to choose
rowids based on direction.
> - Integrate B-Tree cursor calls (`last`, `prev`) and adjust
`rewind`/rowid selection; tweak next-rowid when at `End`.
> - **MVCC Store (`core/mvcc/database/mod.rs`)**:
> - Add `get_prev_row_id_for_table(...)` and generalized
`get_row_id_for_table_in_direction(...)` supporting forward/backward
scans.
> - Add tracing and minor refactors around next/prev rowid retrieval.
> - **Tests (`core/mvcc/database/tests.rs`)**:
> - Add test for backward scan combining B-Tree and MVCC and an
ignored test covering delete during backward scan.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
430bd457e6. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Closes#3980
Still in draft, because there's a ton of stupid AI slop
## Fixes
Closes#3983Closes#3984
- Disallow DROP COLUMN on columns referenced in triggers
- Propagate RENAME COLUMN to trigger SQL definitions
## Drop Column details
DROP COLUMN is not allowed when the column is mentioned in a trigger on
the table the column is dropped from, eg:
```
turso> CREATE TABLE t(x,y);
turso> CREATE TRIGGER foo BEFORE INSERT ON t BEGIN INSERT INTO t VALUES (NEW.x); END;
turso> ALTER TABLE t DROP COLUMN x;
× Parse error: cannot drop column "x": it is referenced in trigger foo
```
However, it is allowed if the trigger is on another table:
```
turso> CREATE TABLE t(x,y);
turso> CREATE TABLE u(x,y);
turso> CREATE TRIGGER bar BEFORE INSERT ON t BEGIN INSERT INTO u(y) VALUES (NEW.x); END;
turso> ALTER TABLE u DROP COLUMN y;
turso> INSERT INTO t VALUES (1,1);
× Parse error: table u has no column named y
```
## AI Disclosure
Nearly all of the code here is vibecoded. I first asked Cursor Composer
to create an initial implementation. Then, I asked it to try to discover
edge cases using the `turso` and `sqlite3` CLIs, and write tests+fixes
for the edge cases found.
The code is a bit slop and there is a LOT of it because the AST
traversal to rewrite column references is all mostly from scratch, but
this isn't a particularly performance-critical use case and it should
solve most of the issues with RENAME and DROP COLUMN.
Closes#3986
SQLite doesn't rewrite INSERT lists or WHEN clause, it instead
lets the trigger go "stale" and will cause runtime errors. This
may not be great behavior, but it's compatible...
- Disallow DROP COLUMN on columns referenced in triggers
- Propagate RENAME COLUMN to trigger SQL definitions
DROP COLUMN is not allowed when the column is mentioned in a trigger
on the table the column is dropped from, eg:
```
turso> CREATE TABLE t(x,y);
turso> CREATE TRIGGER foo BEFORE INSERT ON t BEGIN INSERT INTO t VALUES (NEW.x); END;
turso> ALTER TABLE t DROP COLUMN x;
× Parse error: cannot drop column "x": it is referenced in trigger foo
```
However, it is allowed if the trigger is on another table:
```
turso> CREATE TABLE t(x,y);
turso> CREATE TABLE u(x,y);
turso> CREATE TRIGGER bar BEFORE INSERT ON t BEGIN INSERT INTO u(y) VALUES (NEW.x); END;
turso> ALTER TABLE u DROP COLUMN y;
turso> INSERT INTO t VALUES (1,1);
× Parse error: table u has no column named y
```
Nearly all of the code here is vibecoded. I first asked Cursor Composer to create
an initial implementation. Then, I asked it to try to discover edge cases using the
`turso` and `sqlite3` CLIs, and write tests+fixes for the edge cases found.
The code is a bit slop, but this isn't a particularly performance-critical use case
and it should solve most of the issues with RENAME and DROP COLUMN.
Depends on #3775 - to remove noise from this PR.
## Motivation
In my continued efforts in making the simulator more accessible and
simpler to work with, I have over time simplified and optimized some
parts of the codebase like query generation and decision making so that
more people from the community can contribute and enhance the simulator.
This PR is one more step in that direction.
Before this PR, our `InteractionPlan` stored `Vec<Interactions>`.
`Interactions` are a higher level collection that will generate a list
of `Interaction` (yes I know the naming can be slightly confusing
sometimes. Maybe we can change it later as well. Especially because
`Interactions` are mainly just `Property`). However, this architecture
imposed a problem when MVCC enters the picture. MVCC requires us to make
sure that DDL statements are executed serially. To avoid adding even
more complexity to plan generation, I opted on previous PRs to check
before emitting an `Interaction` for execution, if the interaction is a
DDL statement, and if it is, I emit a `Commit` for each connection still
in a transaction. This worked slightly fine, but as we do not store the
actual execution of interactions in the interaction plan, only the
higher level `Interactions`, this meant that I had to do some
workarounds to modify the `Interactions` inside the plan to persist the
`Commit` I generated on demand.
## Problem
However, I was stupid and overlooked the fact that for certain
properties that allow queries to be generated in the middle (referenced
as extensional queries in the code), we cannot specify the connection
that should execute that query, meaning if a DDL statement occurred
there, the simulator could emit the query but could not save it properly
in the plan to reproduce in shrinking. So to correct and make
interaction generation/emission less brittle, I refactored the
`InteractionPlan` so that it stores `Vec<Interaction>` instead.
## Implications
- `Interaction` is not currently serializable using `Serde` due to the
fact that it stores a function in `Assertion`. This means that we cannot
serialize the plan into a `plan.json`. Which to me is honestly fine, as
the only things that used `plan.json` was `--load` and `--watch`
options. Which are options almost nobody really used.
- For load, instead of generating the whole plan it just read the plan
from disk. The workaround for that right now is just load the `cli_opts`
that were last run for that particular seed and use those exact options
to run the simulation.
- For watch, currently there is not workaround but, @alpaylan told me
has some plans to make assertions serializable by embedding a custom
language into the `plan.sql` file, meaning we will probably not need a
json file at all to store the interaction plan. And this embedded
language will make it much easier to bring back a more proper watch
mode.
- The current shrinking algorithms all have some notion of properties
and removal of properties, but `Interaction` do not have this concept.
So I added some metadata to interactions and a origin ID to each
`Interaction` so that we can search through the list of interactions
using binary search to get all of the interactions that are part of the
same `Property`. To support this, I added an `InteractionBuilder` and
some utilities to iterate and remove properties in the `InteractionPlan`
## Conclusion
Overall, this code simplifies emission of interactions and ensures the
`InteractionPlan` always stores the actual interactions that get
executed. This also decouples more query generation logic from query
emission logic.
Closes#3774
## Trigger Support
This PR adds support for triggers:
- `CREATE TRIGGER`
- `DROP TRIGGER`
Supported
- `BEFORE/AFTER INSERT`
- `BEFORE/AFTER DELETE`
- `BEFORE/AFTER UPDATE [OF <col1,col2,col3>]`
Not supported:
- `INSTEAD OF`
- `TEMPORARY`
### Implementation details
- Triggers are executed within a new `Insn::Program` instruction. The
spec of the insn differs a bit from SQlite: we store a `Statement`
inside that instruction that we can `reset()` for every invocation.
- Like Sqlite, trigger programs take `NEW` and `OLD` rows as program
parameters.
Whenever there are triggers that would fire as the result of a DML
statement:
- `DELETE` writes the rows being deleted into a `RowSet` first.
- `UPDATE` and `INSERT` write the rows being updated into an ephemeral
table first.
### Other shit
Also added `EXPLAIN` support - the bytecode plans for trigger
subprograms are appended after the main program.
### AI disclosure
Used Cursor quite a bit for generating boilerplate code for this - you
can blame all the bad code on the AI of course 🤡
### Follow-ups:
1. ALTER TABLE ops need to rewrite the sql in the CREATE TRIGGER
statement e.g. if a column is renamed. Columns cannot be dropped if
referenced in triggers.
2. Fix weird rowid -1 fallback:
https://github.com/tursodatabase/turso/pull/3979#issuecomment-3547999449Closes#3979
rowid should only try to use the current's position. So if we are not
pointing to a `Loaded` row, then it should return None
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Change `rowid()` to return `None` unless cursor is on a `Loaded` row,
removing the implicit seek from `BeforeFirst`.
>
> - **Core MVCC Cursor (`core/mvcc/cursor.rs`)**:
> - Adjust `rowid()` behavior: remove implicit first-row seek when
`BeforeFirst`; return `None` unless position is `Loaded`.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
8848775a71. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#3977
This PR implements support for `INSERT OR REPLACE INTO t`.
For `OR IGNORE`, we currently rewrite this internally to an `ON CONFLICT
DO NOTHING`, and I was hopeful we could do this with OR REPLACE, however
it seems SQLite actually deletes the row and then proceeds to insert, so
we could not simply rewrite this to an `ON CONFLICT DO UPDATE SET
col=excluded.col`, as this would result in differing rowid's when
compared to SQLite.
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#3972
