This PR adds information about checkpoint sequence number to the WAL raw
API. Will be used in the sync engine.
Depends on the #2699
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#2707
Closes#2686
**Note**: this PR also incorporates #2700, which I cannot merge
separately because it reveals the bug described in #2686, which nothing
on `main` currently detects.
_(Also as background, the way this all started was: I was trying to
enable `UNIQUE` and `PRIMARY KEY` usage in `simulator` in #2641 , but
couldn't because it would constantly fail due to #2686.)_
---
I'll admit this PR went in a different direction than I had envisioned.
I was trying to debug and minimally fix#2686, but was finding it
increasingly hard to understand the flow of `translate_insert` and fix
this specific issue without breaking something else. So, I thought it
might be benefit from restructuring.
---
## Functional changes
- Fixes#2686.
* If an index contained a `rowid` alias column, we were inserting
`NULL` into the index instead of the actual integer value.
* The root cause of this is that SQLite does insert `NULL` in place
of the rowid alias column into the table, presumably to save space.
* This is not a problem for tables as `SELECT`ing the rowid alias
column will always be mapped into `Insn::Rowid`, but it is a major
problem for indexes as index lookups will never find anything.
## Code structure changes
The responsibility of holding the information about what to insert is
now contained in these new data structures:
```rust
/// Represents how a table should be populated during an INSERT.
#[derive(Debug)]
struct Insertion<'a> {
/// The integer key ("rowid") provided to the VDBE.
key: InsertionKey<'a>,
/// The column values that will be fed to the MakeRecord instruction to insert the row.
/// If the table has a rowid alias column, it will also be included in this record,
/// but a NULL will be stored for it.
col_mappings: Vec<ColumnMapping<'a>>,
/// The register that will contain the record built using the MakeRecord instruction.
record_reg: usize,
}
#[derive(Debug)]
enum InsertionKey<'a> {
/// Rowid is not provided by user and will be autogenerated.
Autogenerated { register: usize },
/// Rowid is provided via the 'rowid' keyword.
LiteralRowid {
value_index: Option<usize>,
register: usize,
},
/// Rowid is provided via a rowid alias column.
RowidAlias(ColumnMapping<'a>),
}
/// Represents how a column in a table should be populated during an INSERT.
/// In a vector of InsertionMapping, the index of a given InsertionMapping is
/// the position of the column in the table.
#[derive(Debug)]
struct ColumnMapping<'a> {
/// Column definition
column: &'a Column,
/// Index of the value to use from a tuple in the insert statement.
/// This is needed because the values in the insert statement are not necessarily
/// in the same order as the columns in the table, nor do they necessarily contain
/// all of the columns in the table.
/// If None, a NULL will be emitted for the column, unless it has a default value.
/// A NULL rowid alias column's value will be autogenerated.
value_index: Option<usize>,
/// Register where the value will be stored for insertion into the table.
register: usize,
}
```
---
This gets rid of a few things that are a bit hard to follow in the
current implementation:
1. Needing to keep track of "the last rowid explicit value" and other
weird edge case code related to rowids
```rust
// <old code>
// In case when both rowid and rowid-alias column provided in the query - turso-db overwrite rowid with **latest** value from the list
// As we iterate by column in natural order of their definition in scheme,
// we need to track last value_index we wrote to the rowid and overwrite rowid register only if new value_index is greater
let mut last_rowid_explicit_value = None;
...
// <more old code>
// When inserting a single row, SQLite writes the value provided for the rowid alias column (INTEGER PRIMARY KEY)
// directly into the rowid register and writes a NULL into the rowid alias column.
let write_directly_to_rowid_reg = mapping.column.is_rowid_alias;
let write_reg = if write_directly_to_rowid_reg {
if last_rowid_explicit_value.is_some_and(|x| x > value_index) {
continue;
}
last_rowid_explicit_value = Some(value_index);
column_registers_start // rowid always the first register in the array for insertion record
} else {
column_register
};
```
Instead when the `Insertion` struct is constructed, it will simply
overwrite `InsertionKey` if a rowid reference is encountered multiple
times, so we naturally use the "last seen" rowid value. Moreover,
`InsertionKey` is always translated first, so we need no special logic
for it:
```rust
// <new code>
translate_key(program, insertion, &mut translate_value_fn, resolver)?;
for col in insertion.col_mappings.iter() {
translate_column(
program,
col.column,
col.register,
col.value_index,
&mut translate_value_fn,
resolver,
)?;
}
```
---
2. Needing to keep track of registers in the main execution flow:
```rust
// <old code>
// allocate a register for each column in the table. if not provided by user, they will simply be set as null.
// allocate an extra register for rowid regardless of whether user provided a rowid alias column.
let num_cols = btree_table.columns.len();
let rowid_and_columns_start_register = program.alloc_registers(num_cols + 1);
let columns_start_register = rowid_and_columns_start_register + 1;
```
Now the main execution flow just uses these methods on `Insertion`:
```rust
// Create and insert the record
program.emit_insn(Insn::MakeRecord {
start_reg: insertion.first_col_register(),
count: insertion.col_mappings.len(),
dest_reg: insertion.record_register(),
index_name: None,
});
program.emit_insn(Insn::Insert {
cursor: cursor_id,
key_reg: insertion.key_register(),
record_reg: insertion.record_register(),
flag: InsertFlags::new(),
table_name: table_name.to_string(),
});
```
---
The translation of a row now uses the information in `Insertion` and the
implementation is shared between the "insert single row" and "insert
multiple rows" cases:
```rust
/// Translate the key and the columns of the insertion.
/// This function is called by both [translate_rows_single] and [translate_rows_multiple],
/// each providing a different [translate_value_fn] implementation, because for multiple rows
/// we need to emit the values in a loop, from either an ephemeral table or a coroutine,
/// whereas for the single row the translation happens in a single pass without looping.
fn translate_rows_base<'short, 'long: 'short>(
program: &mut ProgramBuilder,
insertion: &'short Insertion<'long>,
mut translate_value_fn: impl FnMut(&mut ProgramBuilder, usize, usize) -> Result<()>,
resolver: &Resolver,
) -> Result<()> {
translate_key(program, insertion, &mut translate_value_fn, resolver)?;
for col in insertion.col_mappings.iter() {
translate_column(
program,
col.column,
col.register,
col.value_index,
&mut translate_value_fn,
resolver,
)?;
}
Ok(())
}
```
Which gets rid of the duplication in `populate_columns_single_row` and
`populate_columns_multiple_rows` in the old implementation.
Reviewed-by: Nikita Sivukhin (@sivukhin)
Closes#2687
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.
Found when running simulator in #2641
All indexes store the rowid as the last column, so whenever the rowid of
a given row changes the index entry must also be deleted and reinserted
with the new index.
Reviewed-by: Nikita Sivukhin (@sivukhin)
Closes#2712
UPDATE should skip over the UNIQUE constraint failure if the existing
row it found during the check has the same rowid as the row we are
currently updating
Same deal as #2700, except this time in UPDATE. Nothing tests this on
`main` so not caught.
I will later put #2641 into mergeable condition so it will catch all of
these going forward.
Reviewed-by: Nikita Sivukhin (@sivukhin)
Closes#2710
Previously, we just hardcoded the reserved space with encryption flag.
This patch removes that and sets the reserved space if a key was
specified during a creation of db
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#2706
This PR introduces optional upper_bound for PASSIVE and TRUNCATE
checkpoint modes
This is needed for sync engine where we need to have control over WAL:
1. TRUNCATE with upper_bound used as a way to checkpoint WAL only if
there were no frames written since sync-engine read max_frame_no
2. PASSIVE with upper_bound used to checkpoint only certain prefix of
the WAL
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#2699
Previously, we just hardcoded the reserved space with encryption flag.
This patch removes that and sets the reserved space if a key was
specified during a creation of db
### General idea:
(outside of other optimizations made mostly around concurrency):
**When checkpointing, use pages from the PageCache if we can determine
that they are exactly the page/frame that we want.**
e.g. if the frame_cache has an entry:
`Page ID: 104 -> Frame ID's: [1001, 1002]`
and the OngoingCheckpoint has min_frame of 999 and max_frame of 1020, we
should be able to check the PageCache and see if it has page 104, and
only if it is tagged with frame_id = 1002, can we use that page to
backfill the DB file.
Since using a cached page during checkpoint is purely an optimization,
we can be conservative in terms of when we accept that a cached page is
valid to use. I came up with a `wal_tag` which is the frame_id +
checkpoint_seq, which is set only in the two following places:
1. When explicitly reading a frame from the WAL. (inside
Wall::read_frame)
- read_frame is perhaps the most obvious path of ensuring it's the
exact page + frame combination that we want.
2. When appending a frame to the log during the normal process of
writing (during `[Pager::cacheflush]`)
- cacheflush calls append_frame, and inside the Completion, the dirty
flag is cleared, and the wal_tag flag is set to the frame_id.
Inside `finish_read_page` (which is called for every page we read from
either the DB file or WAL.. the `wal_tag` is cleared along with the
`dirty` flag, so that any re-used `PageRef's` don't contain wal_tag's
from any previous or stale pages.
#### **Proposal**:
(In order to merge and simultaneously be able to sleep at night)
there is this debug assertion:
```rust
#[cfg(debug_assertions)]
{
let mut raw = vec![0u8; self.page_size() as usize + WAL_FRAME_HEADER_SIZE];
self.io.wait_for_completion(self.read_frame_raw(target_frame, &mut raw)?)?;
let (_, wal_page) = sqlite3_ondisk::parse_wal_frame_header(&raw);
let cached = cached_page.get_contents().buffer.as_slice();
// while being horrible for performance, we can ensure that the bytes are identical
// when using the cached page vs what we would otherwise have read from disk.
turso_assert!(wal_page == cached, "cache fast-path returned wrong content for page {page_id} frame {target_frame}");
}
```
Performance
=====================================
Average latency for a checkpoint on my local machine:
#### Before: `7-12ms`
#### After: `2-5ms`
Reviewed-by: Nikita Sivukhin (@sivukhin)
Closes#2568
