`commit_txn` in MVCC was hacking its way through I/O until now. After
adding this and the test for concurrent writers we now see `busy` errors
returning as expected because there is no `commit` queueing happening
yet until next PR I open.
Closes#2895
@penberg this PR try to clean up `turso_parser`'s`fmt` code.
- `get_table_name` and `get_column_name` should return None when
table/column does not exist.
```rust
/// Context to be used in ToSqlString
pub trait ToSqlContext {
/// Given an id, get the table name
/// First Option indicates whether the table exists
///
/// Currently not considering aliases
fn get_table_name(&self, _id: TableInternalId) -> Option<&str> {
None
}
/// Given a table id and a column index, get the column name
/// First Option indicates whether the column exists
/// Second Option indicates whether the column has a name
fn get_column_name(&self, _table_id: TableInternalId, _col_idx: usize) -> Option<Option<&str>> {
None
}
// help function to handle missing table/column names
fn get_table_and_column_names(
&self,
table_id: TableInternalId,
col_idx: usize,
) -> (String, String) {
let table_name = self
.get_table_name(table_id)
.map(|s| s.to_owned())
.unwrap_or_else(|| format!("t{}", table_id.0));
let column_name = self
.get_column_name(table_id, col_idx)
.map(|opt| {
opt.map(|s| s.to_owned())
.unwrap_or_else(|| format!("c{col_idx}"))
})
.unwrap_or_else(|| format!("c{col_idx}"));
(table_name, column_name)
}
}
```
- remove `FmtTokenStream` because it is same as `WriteTokenStream `
- remove useless functions and simplify `ToTokens`
```rust
/// Generate token(s) from AST node
/// Also implements Display to make sure devs won't forget Display
pub trait ToTokens: Display {
/// Send token(s) to the specified stream with context
fn to_tokens<S: TokenStream + ?Sized, C: ToSqlContext>(
&self,
s: &mut S,
context: &C,
) -> Result<(), S::Error>;
// Return displayer representation with context
fn displayer<'a, 'b, C: ToSqlContext>(&'b self, ctx: &'a C) -> SqlDisplayer<'a, 'b, C, Self>
where
Self: Sized,
{
SqlDisplayer::new(ctx, self)
}
}
```
Closes#2748
Because we can abort a read_page completion, this means a page can be in
the cache but be unloaded and unlocked. However, if we do not evict that
page from the page cache, we will return an unloaded page later which
will trigger assertions later on. This is worsened by the fact that page
cache is not per `Statement`, so you can abort a completion in one
Statement, and trigger some error in the next one if we don't evict the
page in these circumstances.
Also, to propagate IO errors we need to return the Error from
IOCompletions on step.
Closes#2785
Using `usize` to compute file offsets caps us at ~16GB on 32-bit
systems. For example, with 4 KiB pages we can only address up to 1048576
pages; attempting the next page overflows a 32-bit usize and can wrap
the write offset, corrupting data. Switching our I/O APIs and offset
math to u64 avoids this overflow on 32-bit targets
Closes#2791
- check free list trunk and pages
- use shared hash map to check for duplicate references for pages
- properly check overflow pages
Reviewed-by: Pere Diaz Bou <pere-altea@homail.com>
Closes#2816
My goal with this patch is to be able to implement the ProjectOperator
for DBSP circuits using VDBE for expression evaluation.
*not* doing so is dangerous for the following reason: we will end up
with different, subtle, and incompatible behavior between SQLite
expressions if they are used in views versus outside of views.
In fact, even in our prototype had them: our projection tests, which
used to pass, were actually wrong =) (sqlite would return something
different if those functions were executed outside the view context)
For optimization reasons, we single out trivial expressions: they don't
have go through VDBE. Trivial expressions are expressions that only
involve Columns, Literals, and simple operators on elements of the same
type. Even type coercion takes this out of the realm of trivial.
Everything that is not trivial, is then translated with translate_expr -
in the same way SQLite will, and then compiled with VDBE.
We can, over time, make this process much better. There are essentially
infinite opportunities for optimization here. But for now, the main
warts are:
* VDBE execution needs a connection
* There is no good way in VDBE to pass parameters to a program.
* It is almost trivial to pollute the original connection. For example,
we need to issue HALT for the program to stop, but seeing that halt
will usually cause the program to try and halt the original program.
Subprograms, like the ones we use in triggers are a possible solution,
but they are much more expensive to execute, especially given that our
execution would essentially have to have a program with no other role
than to wrap the subprogram.
Therefore, what I am doing is:
* There is an in-memory database inside the projection operator (an
obvious optimization is to share it with *all* projection operators).
* We obtain a connection to that database when the operator is created
* We use that connection to execute our VDBE, which offers a clean, safe
and isolated way to execute the expression.
* We feed the values to the program manually by editing the registers
directly.
If a connection does e.g. CREATE TABLE, it will start a "child statement"
to reparse the schema. That statement does not start its own transaction,
and so should not try to end the existing one either.
We had a logic bug where these steps would happen:
- `CREATE TABLE` executed successfully
- pread fault happens inside `ParseSchema` child stmt
- `handle_program_error()` is called
- `pager.end_tx()` returns immediately because `is_nested_stmt` is true
and we correctly no-op it.
- however, crucially: `handle_program_error()` then sets tx state to None
- parent statement now catches error from nested stmt and calls
`handle_program_error()`, which calls `pager.end_tx()` again, and since
txn state is None, when it calls `rollback()` we panic on the assertion
`"dirty pages should be empty for read txn"`
Solution:
Do not do _any_ error processing in `handle_program_error()` inside a nested
stmt. This means that the parent write txn is still active when it processes
the error from the child and we avoid this panic.
SQLite does not store the rowid alias column in the record at all
when it is a rowid alias, because the rowid is always stored anyway
in the record header.
We were storing `txid` in `ProgramState`, this meant it was impossible
to track interactive transactions. This was extracted to `Connection`
instead.
Moreover, transaction state for mvcc now is reset on commit.
Closes#2689
This gets rid of `InsertState` in `BTreeCursor` plus the `moved_before`
parameter to `BTreeCursor::insert` -- instead, seek logic is now in the
existing state machines for `op_insert` and `op_idx_insert`
Reviewed-by: Preston Thorpe <preston@turso.tech>
Closes#2639
This not only changes schema_did_change on commit_txn for mvcc, but also
extracts the connection transaction state from non mvcc transactions to
mvcc too.
We have halt and op_halt, doing essentially the same thing.
This PR unifies them. There is a minor difference between them now in
the way halt() handles auto-commit. My current understanding of the code
is that what we have in halt *is a bug*, which is already one bad
consequence of the duplication.
Closes#2631
We have halt and op_halt, doing essentially the same thing.
This PR unifies them. There is a minor difference between them now in
the way halt() handles auto-commit. My current understanding of the code
is that what we have in halt *is a bug*, which is already one bad
consequence of the duplication.
