This PR makes Completion to be `Send` and also force internal callbacks
to be `Send`.
The reasons for that is following:
1. `io_uring` right now can execute completion at any moment potentially
on arbitrary thread, so we already implicitly rely on that property of
`Completion` and its callbacks
2. In case of partial sync
(https://github.com/tursodatabase/turso/pull/3931), there will be an
additional requirement for Completion to be Send as it will be put in
the separate queue associated with `DatabaseStorage` (which is Send +
Sync) processed in parallel with main IO
3. Generally, it sounds pretty natural in the context of async io to
have `Send` Completion so it can be safely transferred between threads
The approach in the PR is hacky as `Completion` made `Send` in a pretty
unsafe way. The main reason why Rust can't derive `Send` automatically
is following:
1. Many completions holds `Arc<Buffer>` internally which needs to be
marked with unsafe traits explicitly as it holds `ptr: NonNull<u8>`
2. `Completion` holds `CompletionInner` as `Arc` which internally holds
completion callback as `Box<XXXComplete>`, but because it's guarded by
`Arc` - Rust forces completion callback to also be Sync (not only Send)
and as we usually move Completion in the callback - we get a cycle here
and with current code Send for Completion implies Sync for Completion.
So, in order to fix this, PR marks `ArenaBuffer` as Send + Sync and
forces completion callbacks to be Send + Sync too. It's seems like
`Sync` requirement is theoretically unnecessary and `Send` should be
enough - but with current code organization Send + Sync looks like the
simplest approach.
Making `ArenaBuffer` Sync sounds almost correct, although I am worried
about read/write access to it as internally `ArenaBuffer` do not
introduce any synchronization of its reads/writes - so potentially we
already can hit some multi-threading bugs with io_uring do to
`ArenaBuffer` used from different threads (or maybe there are some
implicit memory barriers in another parts of the code which can
guarantee us that we will properly use `ArenaBuffer` - but this sounds
like a pure luck)
Reviewed-by: Preston Thorpe <preston@turso.tech>
Closes#3935
The current implementation is simple, we have a pointer called
`CursorPosition::Loaded` that points to a rowid and if it's poiting to
either btree or mvcc.
Moving with `next` will `peek` both btree and mvcc to ensure we load the
correct next value. This draws some inefficiencies for now as we could
simply skip one or other in different cases.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> Combine MVCC index with a BTree-backed lazy cursor (including rootpage
mapping) and add row-version state checks, updating VDBE open paths and
tests.
>
> - **MVCC Cursor (`core/mvcc/cursor.rs`)**:
> - Introduce hybrid cursor that merges MVCC index with `BTreeCursor`;
enhanced `CursorPosition` (tracks `in_btree`/`btree_consumed`).
> - Implement state machine for `next`, coordinating MVCC/BTree
iteration and filtering via `RowVersionState`.
> - `current_row()` now yields immutable records from BTree or MVCC;
add `read_mvcc_current_row`.
> - Update `rowid`, `seek`, `rewind`, `last`, `seek_to_last`,
`exists`, `insert` to honor hybrid positioning.
> - **MVCC Store (`core/mvcc/database/mod.rs`)**:
> - Add `RowVersionState` and `find_row_last_version_state`.
> - Remove eager table initialization/scan helpers and `loaded_tables`
tracking.
> - Add `get_real_table_id` for mapping negative IDs to physical root
pages.
> - **VDBE (`core/vdbe/execute.rs`)**:
> - Route BTree cursor creation through
`maybe_transform_root_page_to_positive` and promote to `MvCursor`
without pager arg.
> - Apply mapping in `OpenRead`, `OpenWrite`, `OpenDup`, and index
open paths.
> - **Tests (`core/mvcc/database/tests.rs`)**:
> - Adjust to new cursor API; add coverage for BTree+MVCC iteration
and gaps after checkpoint/restart.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
b581519be4. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Closes#3829
Depends on #3923 .
To have similar semantics to how `op_compare` works, we need to apply an
affinity to the values referenced in the `SeekKey` that is used for
seeking. This means keeping some affinity metadata for the `WhereTerms`
in the optimization phase, then before seeking, we emit an affinity
conversion. Had to dig deep in the sqlite code to understand this
better.
Unfortunately, we cannot have just one compare function to rule them all
here, as we have a specialized/optimized compare code to handle records
that have not yet been deserialized.
Closes#3707
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#3925
It seems that the build on macos arm is failing with `aegis` v0.9.0.
So, here I update `aegis`.
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#3561
## Purpose
* Implement `setAsciiStream(int, InputStream, int)`,
`setUnicodeStream(int, InputStream, int)`, and `setBinaryStream(int,
InputStream, int)` methods in JDBC4PreparedStatemen
## Changes
* `setAsciiStream(int, InputStream, int)`: Reads ASCII bytes, converts
to `String` using `US_ASCII` and binds with `bindText()`.
* `setUnicodeStream(int, InputStream, int)`: Reads bytes as `UTF-8`
encoded text and binds with `bindText()`.
* `setBinaryStream(int, InputStream, int)`: Reads raw bytes and binds
with `bindBlob()`.
* Added consistent error handling and validation
* null stream - `bindNull()`
* Negative length - throws `SQLException`
* Empty stream - Empty String or Empty Array
* I/O errors - throw `SQLException`
* Ensures consistency between `setXxxStream` and `getXxxStream` methods,
so data written and read use the same encoding.
## Related Issue
* #615
Reviewed-by: Kim Seon Woo (@seonWKim)
Closes#3917
Previously we didn't use GITHUB_TOKEN for anything. But now that PR
meta-data must be fetched with a extra GitHub API call, then PRs at
least will always nedd GITHUB_TOKEN.
Closes#3918
Depends on #3920
Moves some code around so it is easier to reuse and less cluttered in
`execute.rs`, and changes how `compare` works. Instead of mutating some
register, we now just return the possible `ValueRef` representation of
that affinity. This allows other parts of the codebase to reuse this
logic without needing to have an owned `Value` or a `&mut Register`
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#3923
Depends on #3919
Also change `op_compare` to reuse the same compare_immutable logic
First step to finish #2304
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#3920
Makes it easier to visualize what is related to Value and what is
related to opcodes. This will also facilitate in my next PR to
generalize certain function over `Value` and `ValueRef` as listed in
#2304Closes#3919
