very small fix when i was reading the codebase with rust-analyser while
trying to find a bug for simulator.
original error:
`non-primitive cast: <Range<i32> as Iterator>::Item as i32 rust-analyzer
E0605`
Reviewed-by: Preston Thorpe <preston@turso.tech>
Closes#3043
Closes#2993
## Background
When a `CREATE TABLE` statement specifies constraints like `col UNIQUE`,
`col PRIMARY KEY`, `UNIQUE (col1, col2)`, `PRIMARY KEY(col3, col4)`,
SQLite creates indexes for these constraints automatically with the
naming scheme `sqlite_autoindex_<table_name>_<increasing_number>`.
## Problem
SQLite expects these indexes to be created in table definition order.
For example:
```sql
CREATE TABLE t(x UNIQUE, y PRIMARY KEY, c, d, UNIQUE(c,d));
```
Should result in:
```sql
sqlite_autoindex_t_1 -- x UNIQUE
sqlite_autoindex_t_2 -- y PRIMARY KEY
sqlite_autoindex_t_3-- UNIQUE(c,d)
```
However, `tursodb` currently doesn't uphold this invariant -- for
example: the PRIMARY KEY index is always constructed first. SQLite flags
this as a corruption error (see #2993).
## Solution
- Process "unique sets" in table definition order. "Unique sets" are
groups of 1-n columns that are part of either a UNIQUE or a PRIMARY KEY
constraint.
- Deduplicate unique sets properly: a PRIMARY KEY of a rowid alias
(INTEGER PRIMARY KEY) is not a unique set. `UNIQUE (a desc, b)` and
`PRIMARY KEY(a, b)` are a single unique set, not two.
- Unify logic for creating automatic indexes and parsing them - remove
separate logic in `check_automatic_pk_index_required()` and use the
existing `create_table()` utility in both index creation and
deserialization.
- Deserialize a single automatic index per unique set, and assert that
`unique_sets.len() == autoindexes.len()`.
- Verify consistent behavior by adding a fuzz tests that creates 1000
databases with 1 table each and runs `PRAGMA integrity_check` on all of
them with SQLite.
## Trivia
Apart from fixing the exact issue #2993, this PR also fixes other bugs
related to autoindex construction - namely cases where too many indexes
were created due to improper deduplication of unique sets.
Reviewed-by: Preston Thorpe <preston@turso.tech>
Closes#3018
Our simulator is currently limited to concurrency of one. This
introduces a much less sophisticated DST with focus on finding
concurrency bugs.
Closes#2985
If both of the following are true:
1. All read locks are already held
2. The highest readmark of any read lock is less than the committed max frame
Then we must return Busy to the reader, because otherwise they would begin a
transaction with a stale local max frame, and thus not see some committed
changes.
indexes with the naming scheme "sqlite_autoindex_<tblname>_<number>"
are automatically created when a table is created with UNIQUE or
PRIMARY KEY definitions.
these indexes must map to the table definition SQL in definition order,
i.e. sqlite_autoindex_foo_1 must be the first instance of UNIQUE or
PRIMARY KEY and so on.
this commit fixes our autoindex creation / parsing so that this invariant
is upheld.
Value conversion to float for math functions work in a more strict way
than general numeric conversion. For example, valid prefixes that can be
converted to a integer, like `"44s"` will be converted to `Value::Null`
instead of trying to recover like the math operators.
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#3012
Particularly we were tracing `ImmutableRecord` / `BTreeKey` which would
then trace the bytes of records. These are super super hot paths and I
think we can probably remove even more to under debug assertions so we
dont eat those atomics/branches all the time.
This PR also introduces the `tracing_release` feature, which turns all
`trace!` and `debug!` macro invocations to noops at compile time, and
makes that feature available for all bindings.
it also removes the unused `lru` dependency, and cleans up the makefile
a bit
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#2995
Closes#2997
Fixes issue #2997 where connection 2 cannot see tables created by
another connection 1, because `Connection::query()` was not checking
whether its copy of the schema was stale.
Reviewed-by: Preston Thorpe <preston@turso.tech>
Closes#3008
Closes#1714
This PR enables the use of an index as the iteration cursor for a point
or range deletion operation. Main changes:
- Use `Delete` opcode for the index that is iterating the rows - avoids
unnecessary seeking on that index, since it's already positioned
correctly
- Fix delete balancing; details below:
### current state
- a deletion may cause a btree rebalancing operation
- to get the cursor back to the right place after a rebalancing, we must
remember what the deleted key was and seek to it
- right now we are using `SeekOp::LT` to move to one slot BEFORE the
deleted key, so that if we delete rows in a loop, the following `Next()`
call will put us back into the right place
### problem
- When we delete multiple rows, we always iterate forwards. Using
`SeekOp::LT` implies backwards iteration, but it works OK for table
btrees since the cursor never remains on an internal node, because table
internal cells do not have payloads. However: this behavior is
problematic for indexes because we can effectively end up skipping
visiting a page entirely. Honestly: despite spending some debugging the
_old_ code, I still don't remember what exactly causes this to happen.
:) It's one of the `iter_dir` specific behaviors in `indexbtree_move_to`
or `get_prev_record()`, but I'm too tired to spend more time figuring it
out. I had the reason in my head before going on vacation, but it was
evicted from the cache it seems...
### solution
use `SeekOp::GE { eq_only: true }` instead and make the next call to
`Next()` a no-op instead. This has the same effect as SeekOp::LT +
next(), but without introducing bugs due to `LT` being implied backwards
iteration.
Reviewed-by: Nikita Sivukhin (@sivukhin)
Closes#2981
The `run_once()` name is just a historical accident. Furthermore, it now
started to appear elsewhere as well, so let's just call it IO::step() as
we should have from the beginning.
Closes#3001
Resolves#2677
- Implements the modifier for Floor on the datetime object.
- Implements the modifier for Ceiling on the datetime object.
- Includes additional testing changes in testing/scalar-functions-
datetime.test to test the sql functionality.
Consolidation PR of #2678 and #2679 since the functions ended up being
much more complicated than I initially thought and had to be done in a
single PR.
Closes#2757
The `run_once()` name is just a historical accident. Furthermore, it now
started to appear elsewhere as well, so let's just call it IO::step() as we
should have from the beginning.
Because `io_uring` may have many other I/O submission events queued
(that are relevant to the operation) when we experience an error,
marking our `Completion` objects as aborted is not sufficient, the
kernel will still execute queued I/O, which can mutate WAL or DB state
after we’ve declared failure and keep references (iovec arrays, buffers)
alive and stall reuse. We need to stop those in-flight SQEs at the
kernel and then drain the ring to a known-empty state before reusing any
resources.
The following methods were added to the `IO` trait:
`cancel`: which takes a slice of `Completion` objects and has a default
implementation that simply marks them as `aborted`.
`drain`: which has a default noop implementation, but the `io_uring`
backend implements this method to drain the ring.
CC @sivukhin
Reviewed-by: Pere Diaz Bou <pere-altea@homail.com>
Closes#2787
