It is insane that SQLite even allows this.
They actually don't if "defensive mode" is enabled:
"It is always safe to read the schema_version, but changing the
schema_version can cause problems. For this reason, attempts to change
the value of schema_version are a silent no-op when defensive mode is
enabled for a database connection.
Warning: Misuse of this pragma can result in database corruption."
We also update the compat table, which was not updated to reflect
the read version of this pragma being implemented.
Closes#2165
The following sequence of events is possible:
- init_chunk_heap() called
- flush() gets called, and all chunks start writing to disk
- chunk A status is WriteComplete, so chunk.read() gets called on chunk
A
- chunk A sets its status to WaitingForRead
- some other chunk B is still in WaitingForWrite status after flush()
- for this reason, init_chunk_heap() returns IOResult::IO
- init_chunk_heap() is called again
- we panic because chunk A is in WaitingForRead status
So - we just allow WaitingForRead status in init_chunk_heap() instead.
This panic was caught thanks to Pedro's IO latency enhancement to the
sim!
Reviewed-by: Iaroslav Zeigerman (@izeigerman)
Closes#2166
I ended up hitting #1974 today and wanted to fix it. I worked with
Claude to generate a more comprehensive set of queries that could fail
aside from just the insert query described in the issue. He got most of
them right - lots of cases were indeed failing. The ones that were
gibberish, he told me I was absolutely right for pointing out they were
bad.
But alas. With the test cases generated, we can work on fixing it. The
place where the assertion was hit, all we need to do there is return
true (but we assert that this is indeed a string literal, it shouldn't
be anything else at this point).
There are then just a couple of places where we need to make sure we
handle double quotes correctly. We already tested for single quotes in a
couple of places, but never for double quotes.
There is one funny corner case where you can just select "col" from tbl,
and if there is no column "col" on the table, that is treated as a
string literal. We handle that too.
Fixes#1974Closes#2152
I ended up hitting #1974 today and wanted to fix it. I worked with
Claude to generate a more comprehensive set of queries that could fail
aside from just the insert query described in the issue. He got most of
them right - lots of cases were indeed failing. The ones that were
gibberish, he told me I was absolutely right for pointing out they were
bad.
But alas. With the test cases generated, we can work on fixing it. The
place where the assertion was hit, all we need to do there is return
true (but we assert that this is indeed a string literal, it shouldn't
be anything else at this point).
There are then just a couple of places where we need to make sure we
handle double quotes correctly. We already tested for single quotes in a
couple of places, but never for double quotes.
There is one funny corner case where you can just select "col" from tbl,
and if there is no column "col" on the table, that is treated as a
string literal. We handle that too.
Fixes#1974
The following sequence of events is possible:
- init_chunk_heap() called
- chunk A status is WriteComplete, so chunk.read() gets called on chunk A
- some other chunk B is in WaitingForWrite status after flush()
- init_chunk_heap() returns IOResult::IO
- init_chunk_heap() is called again
- we panic because chunk A is in WaitingForRead status
So - we just allow WaitingForRead status in init_chunk_heap() instead.
This panic was caught thanks to Pedro's IO latency enhancement to the sim!
Another fix extracted from running simulations on the #1988 branch.
When interior cell replacement happens as described in #2108,
we use the `cursor.prev()` method to locate the largest key in the
left subtree.
There was an error during backwards traversal in the `get_prev_record()`
method where the parent's cell index was set as `i32::MAX` but not properly
set to `cell_count + 1` (indicating that rightmost pointer has been visited).
The reason `i32::MAX` is used is that the cell count of the page is not
necessarily known at the time it is pushed to the stack.
This PR fixes the issue by setting the cell index of the parent properly
when visiting the rightmost child.
Fixes#2153.
Not so sure if SQLite doesn't rollback in more cases, we should
definitively check this out.
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#2154
pread and pwrite is usually less instructions then seek and read. Also
added possibility for io to retry if AGAIN error happens. And made write
to wait for Event::writable
Reviewed-by: Preston Thorpe (@PThorpe92)
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#2010
Span creation in debug mode is very slow and impacts our ability to run
the Simulator faster.
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#2146
This PR updates to version Rust 1.88.0 ([Release
notes](https://releases.rs/docs/1.88.0/)) and fixes all the clippy
errors that come with the new Rust version.
This is possible in the latest Rust version:
```rust
if let Some(foo) = bar && foo.is_cool() {
...
}
```
There are three complications in the migration (so far):
- A BUNCH of Clippy warnings (mostly fixed in
https://github.com/tursodatabase/limbo/pull/1827)
- Windows cross compilation failed; linking `advapi32` on windows fixes
it
- Since Rust 1.87.0, advapi32 is not linked by default anymore
([Release notes](https://github.com/rust-
lang/rust/blob/master/RELEASES.md#compatibility-notes-1),
[PR](https://github.com/rust-lang/rust/pull/138233))
- Rust is more strict with FFIs and aligning pointers now. CI checks
failed with error below
- Fixed in https://github.com/tursodatabase/turso/pull/2064
```
thread 'main' panicked at
core/ext/vtab_xconnect.rs:64:25:
misaligned pointer dereference: address must be
a multiple of 0x8 but is 0x7ffd9d901554
```
Closes#1807
We were passing the table columns' collations (all of them) in order,
instead of the index column collations. Two issues:
1. This is wrong
2. There's now an assertion in the Sorter that actually panics if the
length of sort order and collations is not the same
Closes#2140
Closes#1998. Now I am queuing IO to be run at some later point in time.
Also Latency for some reason is slowing the simulator a looot for some
runs.
This PR also adds a StateMachine variant in Balance as now `free_pages`
is correctly an asynchronous function. With this change, we now need a
state machine in the `Pager` so that `free_pages` can be reentrant.
Lastly, I removed a timeout in `checkpoint_shutdown` as it was
triggering constantly due to the slightly increased latency.
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#1943
During running simulations for #1988 I ran into a post-balance validation
error where the correct divider cell could not be found from the parent.
This was caused by divider cell insertion happening this way:
- First divider cell caused overflow
- Second technically had space to fit, so we didn't add it to overflow cells
I looked at SQLite source, and it seems SQLite always adds the cell to overflow
cells if there are existing overflow cells:
```c
if( pPage->nOverflow || sz+2>pPage->nFree ){
...add to overflow cells...
}
```
So, I changed our implementation to do the same, which fixed the balance validation
issue.
However, then I ran into another issue:
A cell inserted during balancing in the `edit_page()` stage was added to overflow cells,
which should not happen. The reason for this was the changed logic in `insert_into_page()`,
outlined above.
It looks like SQLite doesn't use `insert_into_cell()´ in its implementation of `page_insert_array()`
which explains this.
For simplicity, I made a second version of `insert_into_cell()` called `insert_into_cell_during_balance()`
which allows regular cell insertion despite existing overflow cells, since the existing overflow cells are
what caused the balance to happen in the first place and will be cleared as soon as `edit_page()` is done.
