diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 4450e61d..2184d296 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -74,16 +74,29 @@ jobs: env: CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }} run: | - if [ -z "$CARGO_REGISTRY_TOKEN" ]; then echo "CARGO_REGISTRY_TOKEN not set; skipping publish"; exit 0; fi - v=$(sed -n 's/^version = "\(.*\)"$/\1/p' Cargo.toml | head -1) - if curl -fsSL "https://index.crates.io/wo/rk/worktable" | sed -n 's/.*"vers":"\([^"]*\)".*/\1/p' | grep -qx "$v"; then - echo "worktable $v is already on crates.io; nothing to publish" - exit 0 - fi - # The main crate pins its codegen twin exactly, so the twin must land - # on the registry first; cargo waits for availability between the two. - cv=$(sed -n 's/^version = "\(.*\)"$/\1/p' codegen/Cargo.toml | head -1) - if ! curl -fsSL "https://index.crates.io/wo/rk/worktable_codegen" | sed -n 's/.*"vers":"\([^"]*\)".*/\1/p' | grep -qx "$cv"; then - cargo publish -p worktable_codegen - fi - cargo publish -p worktable + set -euo pipefail + if [ -z "${CARGO_REGISTRY_TOKEN:-}" ]; then echo "CARGO_REGISTRY_TOKEN not set; skipping publish"; exit 0; fi + + # Publish in dependency order. worktable_codegen depends on + # worktable_dsl and worktable depends on worktable_codegen, both by + # path with an exact version, so each must be on the registry before + # the next is packaged. Omitting worktable_dsl here is what made + # `cargo publish -p worktable_codegen` fail with "no matching package + # named `worktable_dsl` found" the moment the DSL extraction landed. + publish_if_new() { + crate="$1" + manifest="$2" + version=$(sed -n 's/^version = "\(.*\)"$/\1/p' "$manifest" | head -1) + # crates.io index paths: four or more characters is {first two}/{next two}/{name}. + if curl -fsSL "https://index.crates.io/wo/rk/$crate" \ + | sed -n 's/.*"vers":"\([^"]*\)".*/\1/p' | grep -qx "$version"; then + echo "$crate $version is already on crates.io; skipping" + return 0 + fi + echo "publishing $crate $version" + cargo publish -p "$crate" + } + + publish_if_new worktable_dsl dsl/Cargo.toml + publish_if_new worktable_codegen codegen/Cargo.toml + publish_if_new worktable Cargo.toml diff --git a/Cargo.toml b/Cargo.toml index c2788e24..72dfa3aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,9 +1,9 @@ [workspace] -members = ["codegen", "examples", "performance_measurement", "performance_measurement/codegen"] +members = ["codegen", "dsl", "examples", "performance_measurement", "performance_measurement/codegen"] [package] name = "worktable" -version = "1.0.0-beta.16" +version = "1.0.0-beta.17" edition = "2024" authors = ["Handy-caT"] license = "MIT" @@ -40,15 +40,15 @@ convert_case = "0.6.0" crc32fast = "1.5.0" # Already in the dependency graph transitively (indexset's concurrent # structures); used directly for read-side grace periods. -data_bucket = "=0.5.5" +data_bucket = "0.5.5" # data_bucket = { git = "https://github.com/pathscale/DataBucket", branch = "page_cdc_correction", version = "0.2.7" } # data_bucket = { path = "../DataBucket", version = "0.3.14" } derive_more = { version = "2.0.1", features = ["from", "error", "display", "debug", "into"] } eyre = "0.6.12" fastrand = "2.3.0" futures = "0.3.30" -indexset = { package = "WorkTablesIndex", version = "=0.0.9", default-features = false, features = ["concurrent", "cdc", "multimap"] } -vanilla_indexset = { package = "indexset", version = "=0.15.0", features = ["concurrent", "cdc", "multimap"] } +indexset = { package = "WorkTablesIndex", version = "0.0.9", default-features = false, features = ["concurrent", "cdc", "multimap"] } +vanilla_indexset = { package = "indexset", version = "0.15.0", features = ["concurrent", "cdc", "multimap"] } # indexset = { path = "../indexset", version = "0.15.0", features = ["concurrent", "cdc", "multimap"] } # indexset = { package = "wt-indexset", version = "=0.12.12", features = ["concurrent", "cdc", "multimap"] } log = "0.4.29" @@ -60,7 +60,7 @@ prettytable-rs = "^0.10" psc-nanoid = { version = "3.1.1", features = ["rkyv", "packed"] } rkyv = { version = "0.8.17", features = ["uuid-1"] } reqwest = { version = "0.12", optional = true, default-features = false, features = ["rustls-tls-webpki-roots", "charset", "http2"] } -ps-reclaim = "0.1.0" +ps-reclaim = "0.1.1" rustc-hash = "2.1.1" rusty-s3 = { version = "0.10.2", optional = true } smart-default = "0.7.1" @@ -69,7 +69,7 @@ tracing = "0.1" url = { version = "2", optional = true } uuid = { version = "1.24.0", features = ["v4", "v7"] } walkdir = { version = "2", optional = true } -worktable_codegen = { path = "codegen", version = "=1.0.0-beta.15" } +worktable_codegen = { path = "codegen", version = "=1.0.0-beta.17" } [dev-dependencies] chrono = "0.4.43" diff --git a/codegen/Cargo.toml b/codegen/Cargo.toml index 54bdee5b..9ddbf5d7 100644 --- a/codegen/Cargo.toml +++ b/codegen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "worktable_codegen" -version = "1.0.0-beta.15" +version = "1.0.0-beta.17" edition = "2024" license = "MIT" description = "Proc-macro companion crate for worktable: the worktable! macro and its derives." @@ -18,6 +18,9 @@ path = "src/lib.rs" proc-macro = true [dependencies] +# The schema language, extracted so consumers other than this macro can read +# a declaration. See its crate docs for why that needed a separate crate. +worktable_dsl = { path = "../dsl", version = "=1.0.0-beta.17" } rkyv = { version = "0.8.17" } syn = { version = "2.0.74", features = ["full"] } quote = "1.0.36" diff --git a/codegen/src/common/mod.rs b/codegen/src/common/mod.rs index ec73bce8..c7ad9e1d 100644 --- a/codegen/src/common/mod.rs +++ b/codegen/src/common/mod.rs @@ -1,7 +1,13 @@ -pub mod model; +//! What stayed behind when the schema language moved out. +//! +//! `model` and `parser` are `worktable_dsl` now, so anything can read a +//! declaration. `name_generator` is not part of that language: it invents Rust +//! identifiers for generated code, which is this crate's concern and nobody +//! else's. +//! +//! It also could not have gone. Generators here define inherent `impl`s on +//! `WorktableNameGenerator`, and the orphan rule forbids that for a type owned +//! by another crate. The compiler makes the same argument the design does. pub mod name_generator; -pub mod parser; -#[allow(unused_imports)] -pub use model::*; -pub use parser::Parser; +pub use worktable_dsl::{Parser, model, parser}; diff --git a/codegen/src/common/name_generator.rs b/codegen/src/common/name_generator.rs index 715e8535..d671f03a 100644 --- a/codegen/src/common/name_generator.rs +++ b/codegen/src/common/name_generator.rs @@ -120,6 +120,18 @@ impl WorktableNameGenerator { ) } + /// The name of the const carrying the table's own declaration. + /// + /// It follows `get_version_const_ident`'s shape because it answers the + /// question next to it: the version says *which* schema, and this says + /// *what* that schema is. + pub fn get_schema_const_ident(&self) -> Ident { + let upper_snake_case_name = self.name.from_case(Case::Pascal).to_case(Case::UpperSnake); + Ident::new( + format!("{}_SCHEMA", upper_snake_case_name.to_uppercase()).as_str(), + Span::mixed_site(), + ) + } pub fn get_space_secondary_index_ident(&self) -> Ident { Ident::new(format!("{}SpaceSecondaryIndex", self.name).as_str(), Span::mixed_site()) } diff --git a/codegen/src/generators/in_memory/table/impls.rs b/codegen/src/generators/in_memory/table/impls.rs index 6f35ff6c..af20f878 100644 --- a/codegen/src/generators/in_memory/table/impls.rs +++ b/codegen/src/generators/in_memory/table/impls.rs @@ -17,6 +17,7 @@ impl InMemoryGenerator { let select_range_fn = self.gen_table_select_range_fn(); let insert_fn = self.gen_table_insert_fn(); let insert_many_fn = self.gen_table_insert_many_fn(); + let delete_many_fn = self.gen_table_delete_many_fn(); let reinsert_fn = self.gen_table_reinsert_fn(); let upsert_fn = self.gen_table_upsert_fn(); let get_next_fn = self.gen_table_get_next_fn(); @@ -35,6 +36,7 @@ impl InMemoryGenerator { #select_range_fn #insert_fn #insert_many_fn + #delete_many_fn #reinsert_fn #upsert_fn #count_fn @@ -161,6 +163,63 @@ impl InMemoryGenerator { } } + fn gen_table_delete_many_fn(&self) -> TokenStream { + let name_generator = WorktableNameGenerator::from_table_name(self.name.to_string()); + let primary_key_type = name_generator.get_primary_key_type_ident(); + + quote! { + /// Deletes every row named by `pks`, behind one grace marker. + /// + /// `async` although it awaits nothing today. Every other write on + /// this table is async, because cell-level locking makes an update + /// wait on the readers of the cells it touches, and a write surface + /// where the caller has to know which operations happen to need + /// that is a surface where `let _ = table.upsert(row)` silently + /// drops a write. Uniformity is worth more here than the marginal + /// honesty of a sync signature, and the batch paths may need to + /// wait once they take cell locks rather than the striped gate. + /// + /// A delete is a bit flip: the row is marked deleted in place, its + /// index entries are removed, and its storage becomes reusable once + /// no reader can still reach it. `vacuum` is what later compacts + /// pages and hands whole ones back. + /// + /// Batching matters because the per-row cost is dominated by the + /// reclamation bookkeeping each retirement takes, not by the bit + /// flip: `n` deletes take `n` domain advances where a batch takes + /// one. + /// + /// Unlike `insert_many` this is **not** all-or-nothing. A delete + /// that fails partway has already ghosted rows and removed their + /// index entries, and those rows are genuinely gone, so the error + /// reports how many succeeded rather than pretending to rewind. + /// A key that is not present is skipped rather than failing the + /// batch. + /// + /// Returns the keys actually deleted, in the order given. + /// Deletes every row whose primary key falls in `range`. + /// + /// The shape bulk eviction has: a caller dropping a generation + /// knows the span it wants gone rather than the individual keys. + /// The span is collected from the primary index in one ordered + /// walk and then deleted exactly as `delete_many` would, keys + /// still resolved under their mutation guards. + pub async fn delete_range(&self, range: R) + -> core::result::Result, BatchDeleteError<#primary_key_type>> + where R: core::ops::RangeBounds<#primary_key_type> + { + self.0.delete_range(range) + } + + pub async fn delete_many(&self, pks: Vec) + -> core::result::Result, BatchDeleteError<#primary_key_type>> + where #primary_key_type: From + { + self.0.delete_many(pks.into_iter().map(core::convert::Into::into).collect()) + } + } + } + fn gen_table_reinsert_fn(&self) -> TokenStream { let name_generator = WorktableNameGenerator::from_table_name(self.name.to_string()); let row_type = name_generator.get_row_type_ident(); diff --git a/codegen/src/generators/persist/table/impls.rs b/codegen/src/generators/persist/table/impls.rs index f6a1228b..22fb5f69 100644 --- a/codegen/src/generators/persist/table/impls.rs +++ b/codegen/src/generators/persist/table/impls.rs @@ -18,6 +18,7 @@ impl PersistGenerator { let select_range_fn = self.gen_table_select_range_fn(); let insert_fn = self.gen_table_insert_fn(); let insert_many_fn = self.gen_table_insert_many_fn(); + let delete_many_fn = self.gen_table_delete_many_fn(); let reinsert_fn = self.gen_table_reinsert_fn(); let upsert_fn = self.gen_table_upsert_fn(); let get_next_fn = self.gen_table_get_next_fn(); @@ -39,6 +40,7 @@ impl PersistGenerator { #select_range_fn #insert_fn #insert_many_fn + #delete_many_fn #reinsert_fn #upsert_fn #count_fn @@ -483,6 +485,63 @@ impl PersistGenerator { } } + fn gen_table_delete_many_fn(&self) -> TokenStream { + let name_generator = WorktableNameGenerator::from_table_name(self.name.to_string()); + let primary_key_type = name_generator.get_primary_key_type_ident(); + + quote! { + /// Deletes every row named by `pks`, behind one grace marker. + /// + /// `async` although it awaits nothing today. Every other write on + /// this table is async, because cell-level locking makes an update + /// wait on the readers of the cells it touches, and a write surface + /// where the caller has to know which operations happen to need + /// that is a surface where `let _ = table.upsert(row)` silently + /// drops a write. Uniformity is worth more here than the marginal + /// honesty of a sync signature, and the batch paths may need to + /// wait once they take cell locks rather than the striped gate. + /// + /// A delete is a bit flip: the row is marked deleted in place, its + /// index entries are removed, and its storage becomes reusable once + /// no reader can still reach it. `vacuum` is what later compacts + /// pages and hands whole ones back. + /// + /// Batching matters because the per-row cost is dominated by the + /// reclamation bookkeeping each retirement takes, not by the bit + /// flip: `n` deletes take `n` domain advances where a batch takes + /// one. + /// + /// Unlike `insert_many` this is **not** all-or-nothing. A delete + /// that fails partway has already ghosted rows and removed their + /// index entries, and those rows are genuinely gone, so the error + /// reports how many succeeded rather than pretending to rewind. + /// A key that is not present is skipped rather than failing the + /// batch. + /// + /// Returns the keys actually deleted, in the order given. + /// Deletes every row whose primary key falls in `range`. + /// + /// The shape bulk eviction has: a caller dropping a generation + /// knows the span it wants gone rather than the individual keys. + /// The span is collected from the primary index in one ordered + /// walk and then deleted exactly as `delete_many` would, keys + /// still resolved under their mutation guards. + pub async fn delete_range(&self, range: R) + -> core::result::Result, BatchDeleteError<#primary_key_type>> + where R: core::ops::RangeBounds<#primary_key_type> + { + self.0.delete_range(range) + } + + pub async fn delete_many(&self, pks: Vec) + -> core::result::Result, BatchDeleteError<#primary_key_type>> + where #primary_key_type: From + { + self.0.delete_many(pks.into_iter().map(core::convert::Into::into).collect()) + } + } + } + fn gen_table_reinsert_fn(&self) -> TokenStream { let name_generator = WorktableNameGenerator::from_table_name(self.name.to_string()); let row_type = name_generator.get_row_type_ident(); diff --git a/codegen/src/lib.rs b/codegen/src/lib.rs index bc7b2ce9..79545d7a 100644 --- a/codegen/src/lib.rs +++ b/codegen/src/lib.rs @@ -1,3 +1,11 @@ +// `common` is now a thin front for `worktable_dsl`, which holds the schema +// model and parser so that anything other than this macro can read a +// declaration. Kept as a module rather than an alias because the name +// generator stays here: generators define inherent `impl`s on it, which the +// orphan rule allows only in the crate that owns the type. +// +// The 127 `crate::common::` paths across this crate are unchanged, so the diff +// is a move rather than a sweep. mod common; mod generators; mod mem_stat; diff --git a/codegen/src/worktable/mod.rs b/codegen/src/worktable/mod.rs index c4ddfac2..85f9881c 100644 --- a/codegen/src/worktable/mod.rs +++ b/codegen/src/worktable/mod.rs @@ -1,9 +1,17 @@ use proc_macro2::TokenStream; use crate::common::Parser; -use crate::common::model::{Columns, IndexBackend, Persistence}; +use crate::common::name_generator::WorktableNameGenerator; pub fn expand(input: TokenStream) -> syn::Result { + // Keep the tokens. The declaration is read a second time at the end, as + // data, so the generated table can carry its own schema. It happens at the + // end rather than here so that this function's diagnostics are the ones a + // bad declaration produces: both parses reject the same inputs, but only + // one of them knows to say that a separate `attributes` section is not + // part of the 1.0 grammar. + let declaration = input.clone(); + let mut parser = Parser::new(input); let mut columns = None; let mut queries = None; @@ -59,7 +67,12 @@ pub fn expand(input: TokenStream) -> syn::Result { "a separate `attributes` section is not part of the 1.0 grammar; keep `primary_key`, `autoincrement`, `custom`, `optional`, and `using` inline on their column or index declarations", )); } - _ => return Err(syn::Error::new(ident.span(), "Unexpected identifier")), + other => { + return Err(syn::Error::new( + ident.span(), + format!("Unexpected token `{other}`; expected one of `columns`, `indexes`, `queries`, `config`"), + )); + } } } @@ -68,10 +81,10 @@ pub fn expand(input: TokenStream) -> syn::Result { columns.indexes = i } - validate_index_backends(&columns, persistence)?; - validate_page_size(config.as_ref(), persistence)?; + worktable_dsl::validate::validate_index_backends(&columns, persistence)?; + worktable_dsl::validate::validate_page_size(config.as_ref(), persistence)?; if let Some(q) = &queries { - validate_in_place_queries(&columns, q)?; + worktable_dsl::validate::validate_in_place_queries(&columns, q)?; } let mut generated = if persistence.is_persisted() { @@ -84,138 +97,40 @@ pub fn expand(input: TokenStream) -> syn::Result { generated.extend(crate::generators::partitions::expand(&name, &key, persistence)); } - Ok(generated) -} - -/// data_bucket's on-disk layer seeks with its own hardcoded `PAGE_SIZE` of -/// 16384 bytes (`seek_to_page_start`, `seek_by_link`, `persist_page`), while -/// the generated table threads the user's `page_size` through its page-id and -/// length arithmetic. Any other value therefore reads and writes the wrong -/// file offsets as soon as the table persists, silently corrupting it. -/// In-memory tables never seek a file: for them `page_size` only sizes index -/// nodes and stays configurable. -const DATA_BUCKET_PAGE_SIZE: u32 = 16384; - -fn validate_page_size(config: Option<&crate::common::model::Config>, persistence: Persistence) -> syn::Result<()> { - let Some(config) = config else { return Ok(()) }; - let Some(page_size) = config.page_size else { - return Ok(()); - }; - if persistence.is_persisted() && page_size != DATA_BUCKET_PAGE_SIZE { - let span = config.page_size_span.unwrap_or_else(proc_macro2::Span::call_site); - return Err(syn::Error::new( - span, - format!( - "`page_size: {page_size}` cannot be combined with `persist: true`: the on-disk \ - layer (data_bucket) hardcodes {DATA_BUCKET_PAGE_SIZE}-byte pages in every file \ - seek, so a persisted table with any other page size reads and writes the wrong \ - pages and corrupts its files. Remove `page_size` (or set it to \ - {DATA_BUCKET_PAGE_SIZE}); custom page sizes remain available for in-memory \ - tables, where they only size index nodes" - ), - )); - } - Ok(()) -} + generated.extend(gen_schema_const(&worktable_dsl::Schema::from_tokens(declaration)?)); -/// `in_place` queries hand the caller a mutable reference to the archived -/// column bytes and bypass all index maintenance, so a column that any index -/// is built over cannot be mutated in place: the index would keep resolving -/// the old value. -fn validate_in_place_queries(columns: &Columns, queries: &crate::common::model::Queries) -> syn::Result<()> { - for (name, op) in &queries.in_place { - for column in &op.columns { - if columns.indexes.values().any(|index| &index.field == column) { - return Err(syn::Error::new( - column.span(), - format!( - "in_place query `{name}` mutates column `{column}`, which is covered by an index; \ - indexed columns cannot be updated in place because secondary indexes are not \ - maintained on this path. Use an `update` query instead" - ), - )); - } - } - } - Ok(()) + Ok(generated) } -fn validate_index_backends(columns: &Columns, persistence: Persistence) -> syn::Result<()> { - let explicit_backend = if columns.primary_index_backend.requires_explicit_persistence() { - Some(( - columns.primary_index_backend, - columns.primary_keys.first().expect("primary key exists"), - true, - )) - } else { - columns - .indexes - .values() - .find(|index| index.backend.requires_explicit_persistence()) - .map(|index| (index.backend, &index.name, false)) - }; - - if let Some((backend, ident, is_primary)) = explicit_backend { - let kind = if is_primary { "primary index" } else { "index" }; - match persistence { - Persistence::MemoryOnly => {} - Persistence::Omitted => { - return Err(syn::Error::new( - ident.span(), - format!( - "{kind} `{ident}` uses `{}`, which requires an explicit `persist: true` or `persist: false`", - backend.name() - ), - )); - } - Persistence::Persisted => {} - } +/// Bake the declaration into the generated code, as the text it was written in. +/// +/// The point is that a compiled binary should be able to say what schema it was +/// built against, without the source. A migration planner needs it as the +/// "declared" side of a comparison against what is on disk; a designer needs it +/// to draw a diagram of an application it did not build. +/// +/// The stored form is the DSL text rather than a serialised structure. It needs +/// no format decision, no serde in the dependency graph of every user's build, +/// and it is legible in a hex dump; `worktable_dsl` reads it back with the same +/// parser that read the original, and `dsl/tests/round_trip.rs` holds that +/// property against all 116 declarations in this repository. +/// +/// `allow(dead_code)` because a `worktable!` inside a function body puts this +/// const inside that body, where nothing refers to it and `-D warnings` would +/// otherwise fail a user's build over a const they never asked for. +fn gen_schema_const(schema: &worktable_dsl::Schema) -> TokenStream { + let ident = WorktableNameGenerator::from_table_name(schema.name.clone()).get_schema_const_ident(); + let text = schema.to_dsl(); + let doc = format!( + "The `worktable!` declaration `{}` was generated from, as text. Read it with `worktable_dsl::Schema::parse`.", + schema.name + ); + + quote::quote! { + #[doc = #doc] + #[allow(dead_code)] + pub const #ident: &str = #text; } - - for index in columns.indexes.values().filter(|index| !index.is_unique) { - match index.backend { - IndexBackend::WorktablesIndex | IndexBackend::Arctic => {} - IndexBackend::Indexset | IndexBackend::Congee => { - return Err(syn::Error::new( - index.name.span(), - format!( - "non-unique index `{}` cannot use `{}`; non-unique indexes currently require \ - `worktables_index` or `arctic`", - index.name, - index.backend.name() - ), - )); - } - } - } - - for (column, index) in &columns.indexes { - let key_type = columns - .columns_map - .get(column) - .expect("an index always references a validated column") - .to_string(); - let supported = match index.backend { - IndexBackend::Congee => Some(&["u8", "u16", "u32", "u64", "usize"][..]), - IndexBackend::Arctic => Some(&["u16", "u32", "u64", "u128"][..]), - IndexBackend::WorktablesIndex | IndexBackend::Indexset => None, - }; - if let Some(supported) = supported - && !supported.contains(&key_type.as_str()) - { - return Err(syn::Error::new( - index.name.span(), - format!( - "index `{}` uses `{}`, which does not support key type `{key_type}`; supported types: {}", - index.name, - index.backend.name(), - supported.join(", ") - ), - )); - } - } - - Ok(()) } #[cfg(test)] @@ -704,3 +619,253 @@ mod position_tests { ); } } + +/// What a designer needs from the schema IR: a declaration that has been read +/// into [`worktable_dsl::Schema`] and written back out is a declaration this +/// macro accepts. +/// +/// `worktable_dsl` can test its own round trip, which shows nothing was lost +/// between its parser and its emitter. It cannot show that the text it emits +/// is a declaration *this* macro accepts, because it cannot call this macro: +/// that check has to live on the near side of the proc-macro boundary. +/// +/// The stronger claim — that the emitted declaration generates the *same code* +/// — is not asserted here, and cannot be until the generator is deterministic. +/// See `the_same_declaration_expands_the_same_way_twice` below, which is +/// ignored because it currently fails on unmodified code. +#[cfg(test)] +mod emitted_declarations { + use quote::quote; + use worktable_dsl::Schema; + + use super::expand; + + fn survives_the_round_trip(declaration: proc_macro2::TokenStream) { + expand(declaration.clone()).expect("the original expands"); + + let schema = Schema::from_tokens(declaration).expect("the IR reads it"); + let emitted = schema.to_dsl(); + let reparsed: proc_macro2::TokenStream = syn::parse_str(&emitted) + .unwrap_or_else(|error| panic!("emitted text does not tokenise: {error}\n{emitted}")); + + assert_eq!( + Schema::from_tokens(reparsed.clone()).expect("the emitted text reads back"), + schema, + "the emitted declaration describes a different schema\n{emitted}" + ); + expand(reparsed).unwrap_or_else(|error| panic!("the emitted declaration does not expand: {error}\n{emitted}")); + } + + #[test] + fn a_minimal_declaration() { + survives_the_round_trip(quote! { + name: Minimal, + columns: { id: u64 primary_key }, + }); + } + + #[test] + fn a_persisted_declaration_with_indexes_and_queries() { + survives_the_round_trip(quote! { + name: Account, + version: 3, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + tenant: u64, + nickname: String optional, + balance: f64, + }, + indexes: { + email_idx: email unique, + tenant_idx: tenant, + }, + queries: { + update: { + Nickname(nickname) by id, + Email(email) by tenant, + } + delete: { + ById() by id, + } + in_place: { + Balance(balance) by id, + } + } + }); + } + + #[test] + fn a_partitioned_declaration() { + survives_the_round_trip(quote! { + name: Price, + partition_by: symbol_id: u16, + columns: { + exchange_id: u8 primary_key, + bid: f64, + }, + }); + } + + #[test] + fn a_composite_key_keeps_its_column_order() { + // The order of a composite key decides the field order of the + // generated `get_primary_key`, and so the layout of the key type. An + // emitter that wrote the columns back in a `HashMap`'s order would + // change it. + survives_the_round_trip(quote! { + name: CompositeKey, + persist: true, + columns: { + tenant_id: u64 primary_key, + record_id: u64 primary_key, + value: i64, + }, + }); + } + + #[test] + fn an_explicit_backend_and_a_custom_page_size() { + survives_the_round_trip(quote! { + name: Tuned, + persist: false, + columns: { + id: u64 primary_key using congee, + value: u64, + }, + indexes: { + value_idx: value unique using arctic, + }, + config: { + page_size: 1024, + row_derives: Clone, Debug, + } + }); + } +} + +#[cfg(test)] +mod generator_determinism { + use quote::quote; + + use super::expand; + + /// Expanding one declaration twice must produce one program. It does not. + /// + /// `Columns::columns_map` is a `std::collections::HashMap`, and several + /// generators iterate it directly to emit an ordered construct: the + /// `RowFields` enum and the `AvaiableTypes` enum among them. `RandomState` + /// seeds each map instance differently, so two expansions of the same + /// declaration in the same process emit those variants in different + /// orders, and two compilations of the same source can too. + /// + /// This is ignored rather than deleted because it is the evidence. It is + /// ignored rather than failing because the fix — ordering `columns_map`, + /// which `field_positions` already records the order for — changes the + /// generated code of every table and is a change to review on its own, + /// not a side effect of adding an emitter. + /// + /// Run it with `cargo test -p worktable_codegen -- --ignored`. + #[test] + #[ignore = "records a known generator bug: columns_map is a HashMap, so expansion is not deterministic"] + fn the_same_declaration_expands_the_same_way_twice() { + let declaration = quote! { + name: Twice, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + tenant: u64, + balance: f64, + }, + }; + + let first = expand(declaration.clone()).expect("expands").to_string(); + let second = expand(declaration).expect("expands").to_string(); + assert_eq!(first, second); + } +} + +/// The generated table carries its own declaration. +#[cfg(test)] +mod schema_const { + use proc_macro2::{TokenStream, TokenTree}; + use quote::quote; + use worktable_dsl::Schema; + + use super::expand; + + /// Pull the string out of `pub const : &str = "..";` in generated code. + fn baked_schema(generated: TokenStream, const_name: &str) -> String { + let mut trees = generated + .into_iter() + .skip_while(|tree| !matches!(tree, TokenTree::Ident(ident) if ident == const_name)); + assert!(trees.next().is_some(), "no `{const_name}` const in the generated code"); + for tree in trees { + if let TokenTree::Literal(literal) = tree { + let text = literal.to_string(); + return syn::parse_str::(&text).expect("a string literal").value(); + } + } + panic!("`{const_name}` has no value"); + } + + #[test] + fn a_persisted_table_carries_the_declaration_it_was_built_from() { + let declaration = quote! { + name: Account, + version: 3, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + nickname: String optional, + }, + indexes: { email_idx: email unique }, + queries: { update: { Nickname(nickname) by id } } + }; + + let baked = baked_schema(expand(declaration.clone()).expect("expands"), "ACCOUNT_SCHEMA"); + + assert_eq!( + Schema::parse(&baked).expect("the baked text parses"), + Schema::from_tokens(declaration).expect("the declaration parses"), + "the baked declaration is not the one the table was generated from" + ); + } + + #[test] + fn an_in_memory_table_carries_it_too() { + // A designer reading a crate wants every table, not only the persisted + // ones, and the const costs a string either way. + let declaration = quote! { + name: Price, + partition_by: symbol_id: u16, + columns: { exchange_id: u8 primary_key, bid: f64 }, + }; + + let baked = baked_schema(expand(declaration.clone()).expect("expands"), "PRICE_SCHEMA"); + + assert_eq!( + Schema::parse(&baked).expect("the baked text parses"), + Schema::from_tokens(declaration).expect("the declaration parses"), + ); + } + + #[test] + fn the_baked_text_is_a_declaration_the_macro_accepts() { + // Which is what makes it usable as the old table definition a + // migration would otherwise need kept by hand. + let declaration = quote! { + name: Regenerated, + persist: true, + columns: { id: u64 primary_key autoincrement, payload: String }, + indexes: { payload_idx: payload unique } + }; + + let baked = baked_schema(expand(declaration).expect("expands"), "REGENERATED_SCHEMA"); + let reparsed: TokenStream = syn::parse_str(&baked).expect("tokenises"); + expand(reparsed).expect("the baked declaration expands"); + } +} diff --git a/docs/TODO.md b/docs/TODO.md index fa6493b5..242b39b2 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -3,58 +3,74 @@ What is known to be unfinished, and enough context to act on it without the conversation it came from. Ordered by whether it blocks a release. -Last reviewed 2026-09-01, against `feat/ps-reclaim-beta16`. +Last reviewed 2026-09-02, against `deps/caret-not-locked`. -## Blocking beta.16 +## Closed, and how -### Publish `ps-reclaim` 0.1.1 and bump the pin +### `ps-reclaim` 0.1.1 is published and pinned -`Cargo.toml` pins `ps-reclaim = "0.1.0"`, and 0.1.0 has two defects, both fixed -on `pathscale/ps-reclaim` master at `d207d06` and **not yet published**: +Was blocking beta.16 and did not stop it. 0.1.0 had two defects, both fixed on +`pathscale/ps-reclaim` master at `d207d06` and published as 0.1.1: -- **`Guard` is `Send`** in 0.1.0, and its own documentation says it is not. - Nothing enforced it: every field was `Send`, so the auto trait applied. - Dropping a sent guard stores `NO_DOMAIN` into the *originating* thread's pin - slot while that thread may still be reading, and decrements the wrong - thread's `DEPTH`. Both are use-after-free windows. 0.1.1 makes it `!Send` by - construction (the packed field is a raw pointer) with a `compile_fail` - doctest holding it. +- **`Guard` was `Send`**, and its own documentation said it was not. Nothing + enforced it: every field was `Send`, so the auto trait applied. Dropping a + sent guard stores `NO_DOMAIN` into the *originating* thread's pin slot while + that thread may still be reading, and decrements the wrong thread's `DEPTH`. + Both are use-after-free windows. 0.1.1 makes it `!Send` by construction (the + packed field is a raw pointer) with a `compile_fail` doctest holding it. - **`Guard` was three words** (`&Domain`, `&'static Participant`, `usize`) against `crossbeam-epoch`'s one pointer. `partition_ref` returns a `PartRef { guard, &T }` per call, so it paid that size on every lookup: 16 bytes became 32. 0.1.1 packs the entry into the participant pointer's spare alignment bits (`Participant` is `#[repr(align(128))]`) and is one word. -0.1.1 removes `Guard::domain` and `Guard::retire`. That is a breaking change -and would normally take a minor bump, but `worktable` is the only consumer and -it already calls `Domain::retire` directly, which exists in both. Both call -sites here already moved to `self.epoch.retire(...)`, which -works against 0.1.0 and 0.1.1 alike, so this repo is ready for the bump. +`pathscale/ps-reclaim` also had no workflows at all, which is why both defects +were found by hand from a downstream measurement rather than by a check. It now +runs build, test, doctests, fmt, clippy and Miri under strict provenance, and +publishes from master, on Ubicloud runners. -Publishing is irreversible and needs a human. After it lands: -`ps-reclaim = "0.1.1"` in `Cargo.toml`, then re-run the benchmarks below. +### CI has run on this branch + +`.github/workflows/rust.yml` triggers on push to master and on pull requests +targeting master, so a branch with no PR is only ever checked on somebody's +laptop. PR #82 opened, all six jobs passed, and master has been green since. + +## Blocking beta.17 ### Re-measure the partition regression +Unchanged from the beta.16 review, and still the reason to be careful about +what this release claims. + The claim in `82bfdf6` that `crossbeam-epoch` and `ps-reclaim` are "within noise of each other (3.37 against 3.42)" is disputed by an interleaved A/B run: `partition_ref` measured 3.16-3.35 ns on beta.15 and 3.60-3.68 ns here, in both passes, with the two cleanest samples of the run showing the widest gap. The -guard size above is the likely cause and the reason 0.1.1 exists. - -Not yet confirmed. Every attempt so far ran on a machine at load 4 to 24, where -the control (`partition_lookup/cached_handle`, a pure dereference that cannot -differ between versions) varied 3.6x. Re-run on a quiet box, alternate the tree -order between passes, and reject the run if the control moves more than a few -percent. Full brief, including exact commits and setup, at -`~/code/wt-beta16-perf-brief.md`. - -### CI has never run on this branch - -`.github/workflows/rust.yml` triggers on push to master and on pull requests -targeting master. There is no PR, so every green result is somebody's laptop. -`./scripts/ci-local.sh` passes all five jobs (2816 test results, 0 failures) as -of `8699b07`. +guard size, now fixed in 0.1.1, is the likely cause, so this wants re-running +against 0.1.1 rather than re-running the old comparison. + +Not yet confirmed either way. Every attempt so far ran on a machine at load 4 +to 24, where the control (`partition_lookup/cached_handle`, a pure dereference +that cannot differ between versions) varied 3.6x. Re-run on a quiet box, +alternate the tree order between passes, and reject the run if the control +moves more than a few percent. Full brief, including exact commits and setup, +at `~/code/wt-beta16-perf-brief.md`. + +A second set of numbers circulated during the beta.16 release, reporting +`partition_ref` at 7.78 ns on beta.15 falling to 3.31 ns flat. Do not use them. +They were taken at `8699b07`, before `673869c` showed that the benchmark arm +labelled `pinned_get` was calling `partition_ref`, and they were taken under +load. They contradict the interleaved run above by roughly a factor of two on +beta.15, and neither set has been reproduced on a quiet machine. + +### Decide what happens to beta.16 on crates.io + +1.0.0-beta.16 is published and resolves `ps-reclaim ^0.1.0`, so a lockfile +written before 0.1.1 landed keeps the `Send` guard. A fresh resolve now picks +0.1.1 on its own, since the requirement was always a caret and never an exact +pin. The open question is whether to yank ps-reclaim 0.1.0, which is what makes +the unsound version unreachable rather than merely unpreferred, and whether to +yank beta.16 once beta.17 supersedes it. ## Not blocking, but wrong today diff --git a/dsl/Cargo.toml b/dsl/Cargo.toml new file mode 100644 index 00000000..b5b863fc --- /dev/null +++ b/dsl/Cargo.toml @@ -0,0 +1,46 @@ +[package] +name = "worktable_dsl" +version = "1.0.0-beta.17" +edition = "2024" +license = "MIT" +description = "The worktable! schema language: its model and parser, readable outside the proc macro" +repository = "https://github.com/pathscale/WorkTable" + +[dependencies] +# Exactly what `codegen/src/common` already used. The move adds no dependency +# and drops none; anything else would have made it a rewrite rather than a lift. +# `serde` came later, with the IR, and is optional for the reason given below. +syn = { version = "2.0.74", features = ["full"] } +quote = "1.0.36" +proc-macro2 = "1.0.86" +convert_case = "0.6.0" +indexmap = "2" +serde = { version = "1", features = ["derive"], optional = true } + +[dev-dependencies] +# The integration test builds as its own crate, which is what makes it evidence +# that this one is consumable from outside. +proc-macro2 = "1.0.86" +# Already in the workspace lock; used only by the `serde` feature test. +serde_json = "1" + +[features] +# The IR derives serde only on request. `worktable_codegen` depends on this +# crate and is a proc macro, so it is compiled for the host before anything +# else in a dependent's build; adding serde unconditionally would put a derive +# macro in front of every WorkTable user's first compile to serve consumers +# who are not the compiler. The designer and the migration planner turn it on. +serde = ["dep:serde"] +# Byte ranges on diagnostics, for an editor that wants to underline the +# offending token. Off by default for the same reason `serde` is: this crate is +# compiled for the host as part of `worktable_codegen` before anything else in +# a dependent's build, and span tracking is a cost the compiler does not need. +spans = ["proc-macro2/span-locations"] +# Mermaid class diagrams, and the relation guessing they rest on. Off by +# default because it is the one part of this crate that is not about the +# `worktable!` grammar: the language has no foreign keys, so `infer_relations` +# guesses links from a naming convention that WorkTable does not enforce and +# has no opinion about. That mapping belongs to the application that draws the +# diagram, next to whatever it does enforce. Kept here because it is written +# and tested, behind a flag so that turning it on is a decision. +uml = [] diff --git a/dsl/src/check.rs b/dsl/src/check.rs new file mode 100644 index 00000000..c6df529f --- /dev/null +++ b/dsl/src/check.rs @@ -0,0 +1,254 @@ +//! Reading a declaration the way an editor has to read one. +//! +//! [`crate::Schema::parse`] is the right entry point for a tool that has a +//! finished declaration and wants it as data. It is the wrong one for a live +//! editor, for two reasons this module exists to fix. +//! +//! **It reports one problem.** `syn::Result` carries a single error, because +//! the macro stops at the first one: it cannot generate code either way, so +//! finding the rest costs a compile it is not going to do. An editor has the +//! opposite economics. Fixing one error, recompiling, and finding the next is +//! the loop a live checker exists to remove, so [`check`] runs every rule and +//! returns all of them. +//! +//! **It reports no location.** A [`crate::Schema`] deliberately has no spans: +//! an `Ident` cannot be serialised, compared across processes, or sent to a +//! designer over a socket, which is the whole reason the IR is plain data. But +//! an editor that cannot underline the offending token is showing a message +//! about a file rather than about a place in it. +//! +//! [`Diagnostic`] resolves that by keeping the location *outside* the IR, as a +//! byte range into the source text that was parsed. The `Schema` stays plain +//! data; the ranges live next to it, in the result of the call that produced +//! it. A consumer that wants neither pays for neither. +//! +//! # The `spans` feature +//! +//! Byte ranges need `proc-macro2/span-locations`, and `worktable_codegen` +//! depends on this crate. A proc macro is compiled for the host before +//! anything else in a dependent's build, so anything added here is added to +//! every WorkTable user's first compile. Span tracking is therefore behind an +//! off-by-default `spans` feature, for the same reason `serde` is: a designer +//! turns both on, and the compiler pays for neither. +//! +//! Without the feature, [`Diagnostic::span`] is `None`. The messages are +//! identical either way, so a consumer degrades to file-level diagnostics +//! rather than losing them. + +use crate::schema::Schema; + +/// Where a diagnostic points, as a half-open byte range into the source that +/// was handed to [`check`]. +/// +/// Byte offsets rather than line and column on purpose: an editor converts to +/// whichever it needs, and a byte range survives being sent to one that +/// disagrees about what a column is. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct SourceSpan { + /// First byte of the offending text. + pub start: usize, + /// One past the last byte. + pub end: usize, +} + +/// Why a declaration was rejected, and where. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct Diagnostic { + /// The message, identical to the one the macro would print. + pub message: String, + /// The offending text, when this crate was built with the `spans` feature + /// and the error carried a span. `None` otherwise: absence of a location + /// is never absence of a problem. + pub span: Option, + /// Whether the declaration was still readable despite this. + pub stage: Stage, +} + +/// Which half of reading a declaration produced a diagnostic. +/// +/// The distinction is the one an editor acts on. A [`Stage::Grammar`] failure +/// means there is no schema to draw; a [`Stage::Rules`] failure means there is +/// one, it can be rendered, and the macro would refuse to expand it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum Stage { + /// The text is not a declaration. Nothing was produced. + Grammar, + /// The text is a declaration, and it breaks a rule the macro enforces. + Rules, +} + +/// A declaration, everything wrong with it, or both. +#[derive(Debug, Clone, PartialEq)] +pub struct Checked { + /// The declaration, when the grammar accepted it. Present even when + /// `diagnostics` is not empty: a schema that breaks a rule is still a + /// schema, and an editor has to draw it in order to let anyone fix it. + pub schema: Option, + /// Everything wrong, in the order a reader would work through it. + pub diagnostics: Vec, +} + +impl Checked { + /// Whether the macro would accept this declaration. + /// + /// The question `worktable_dsl` could not answer before this module, and + /// the one a designer has to answer on every keystroke to say whether the + /// thing on screen would compile. + pub fn is_acceptable(&self) -> bool { + self.schema.is_some() && self.diagnostics.is_empty() + } +} + +#[cfg(feature = "spans")] +fn span_of(error: &syn::Error) -> Option { + let range = error.span().byte_range(); + // A synthesised span (`Span::call_site` in a non-macro context) reports an + // empty range at zero, which would point an editor at the first character + // for an error that is not there. Reporting no location beats a wrong one. + if range.is_empty() { + None + } else { + Some(SourceSpan { + start: range.start, + end: range.end, + }) + } +} + +#[cfg(not(feature = "spans"))] +fn span_of(_error: &syn::Error) -> Option { + None +} + +/// Read a declaration, and report everything wrong with it. +/// +/// The input is the macro body, `name: Foo, columns: { .. }`, without the +/// macro name or the surrounding braces, exactly as [`Schema::parse`] takes it. +/// +/// ``` +/// use worktable_dsl::check; +/// +/// // A declaration the macro would refuse: `congee` cannot index a `String`. +/// let checked = check( +/// "name: Bad, +/// persist: false, +/// columns: { id: u64 primary_key, label: String }, +/// indexes: { label_idx: label unique using congee }", +/// ); +/// +/// // Readable, and drawable, even though it would not compile. +/// assert!(checked.schema.is_some()); +/// assert!(!checked.is_acceptable()); +/// assert!(checked.diagnostics[0].message.contains("does not support key type")); +/// ``` +pub fn check(source: &str) -> Checked { + let tokens: proc_macro2::TokenStream = match syn::parse_str(source) { + Ok(tokens) => tokens, + Err(error) => { + // Tokenisation failed, which in practice means an unbalanced + // delimiter: the state a declaration is in for most of the time + // somebody is typing one. There is nothing to parse and nothing to + // draw, and saying so is more use than a partial tree that claims + // the missing half does not exist. + return Checked { + schema: None, + diagnostics: vec![Diagnostic { + message: error.to_string(), + span: span_of(&error), + stage: Stage::Grammar, + }], + }; + } + }; + + let schema = match Schema::from_tokens(tokens.clone()) { + Ok(schema) => schema, + Err(error) => { + return Checked { + schema: None, + diagnostics: vec![Diagnostic { + message: error.to_string(), + span: span_of(&error), + stage: Stage::Grammar, + }], + }; + } + }; + + // The rules run against the model rather than the IR, because that is + // where the spans are and because it is the same code the macro runs. A + // second parse is cheap next to a compile, and it is what keeps this + // answering "would the macro accept this?" rather than "would a + // reimplementation of the macro accept this?". + let diagnostics = match model_of(tokens) { + Ok((columns, queries, config, persistence)) => { + crate::validate::all(&columns, queries.as_ref(), config.as_ref(), persistence) + .iter() + .map(|error| Diagnostic { + message: error.to_string(), + span: span_of(error), + stage: Stage::Rules, + }) + .collect() + } + // Unreachable in practice: the same tokens parsed a moment ago. If the + // two dispatches ever disagree, report it rather than panicking in an + // editor's keystroke handler. + Err(error) => vec![Diagnostic { + message: error.to_string(), + span: span_of(&error), + stage: Stage::Grammar, + }], + }; + + Checked { + schema: Some(schema), + diagnostics, + } +} + +type Model = ( + crate::model::Columns, + Option, + Option, + crate::model::Persistence, +); + +/// The macro's own top-level dispatch, kept to the parts the rules read. +fn model_of(tokens: proc_macro2::TokenStream) -> syn::Result { + let mut parser = crate::Parser::new(tokens); + parser.parse_name()?; + parser.parse_version()?; + let persistence = parser.parse_persist()?; + parser.parse_partition_by()?; + + let mut columns = None; + let mut indexes = None; + let mut queries = None; + let mut config = None; + while let Some(ident) = parser.peek_next() { + match ident.to_string().as_str() { + "columns" => columns = Some(parser.parse_columns()?), + "indexes" => indexes = Some(parser.parse_indexes()?), + "queries" => queries = Some(parser.parse_queries()?), + "config" => config = Some(parser.parse_configs()?), + other => { + return Err(syn::Error::new(ident.span(), format!("Unexpected token `{other}`"))); + } + } + } + + let mut columns = columns.ok_or_else(|| { + syn::Error::new( + proc_macro2::Span::call_site(), + "Expected a `columns` block in declaration", + ) + })?; + if let Some(indexes) = indexes { + columns.indexes = indexes; + } + Ok((columns, queries, config, persistence)) +} diff --git a/dsl/src/lib.rs b/dsl/src/lib.rs new file mode 100644 index 00000000..9a54e8b5 --- /dev/null +++ b/dsl/src/lib.rs @@ -0,0 +1,87 @@ +//! The `worktable!` schema language: its model, and the parser that reads it. +//! +//! # Why this is its own crate +//! +//! This was `codegen/src/common`, inside `worktable_codegen`, which is declared +//! `proc-macro = true`. A proc-macro crate can export nothing but macros, so +//! every type here — the columns, the primary key, the indexes, the queries — +//! was unreachable from any other crate no matter how public it was declared. +//! `mod common` was not even public at that crate's root. +//! +//! The consequence was not theoretical. A schema is written down exactly once, +//! in a `worktable!` invocation, and anything that wants to *read* one — a +//! diagram, a migration tool, a documentation generator, an editor — could not +//! reach the parser that already understood it. The available options were to +//! re-implement the grammar and drift from it, or to do without. +//! +//! Nothing here changed in the move. The model and the parser are the ones the +//! macro has always used, and the macro still uses these: `worktable_codegen` +//! depends on this crate, so there is one grammar rather than a copy that can +//! disagree with the compiler about what a schema means. +//! +//! # Reading a declaration +//! +//! ```ignore +//! use worktable_dsl::Parser; +//! use syn::parse_str; +//! +//! let tokens: proc_macro2::TokenStream = parse_str(source)?; +//! let mut parser = Parser::new(tokens); +//! let name = parser.parse_name()?; +//! let columns = parser.parse_columns()?; +//! ``` +//! +//! The parser is token-based rather than textual, so comments and string +//! literals are handled by `proc_macro2` rather than by hand. The schema files +//! this reads are more comment than code, which makes that difference matter. +//! +//! # What is in scope here, and what is not +//! +//! This crate is the WorkTable universe: one `worktable!` declaration, the +//! grammar it must satisfy, and the rules the macro enforces over it. That is +//! [`Parser`] and [`model`], [`Schema`] and its text emitter, [`validate`] and +//! [`check`], and [`schema::Diff`], which prices a schema change in terms of +//! what the storage engine has to do about it. All of those have a single +//! correct answer that WorkTable owns. +//! +//! The mapping *between* tables is a different layer and it is not in scope. +//! The language has no foreign keys: a column called `project_id` is a `u64` +//! like any other, and nothing in WorkTable enforces, records, or checks a +//! relationship between two tables. `infer_relations` guesses those links from +//! a naming convention, and the guess belongs next to whatever application +//! does enforce the convention. It is behind the off-by-default `uml` feature +//! for that reason: available, tested, and not something this crate asserts. +//! +//! An application building on this should treat a `Relation` as a suggestion +//! to confirm, never as a fact recovered from the schema, because there is no +//! fact there to recover. +//! +//! # Features +//! +//! All off by default. `worktable_codegen` depends on this crate and is a proc +//! macro, so it is compiled for the host before anything else in a dependent's +//! build: anything unconditional here is added to every WorkTable user's first +//! compile, to serve consumers who are not the compiler. +//! +//! - `serde` — `Serialize`/`Deserialize` on the IR, for storing a schema next +//! to the data it describes or sending one over a socket. +//! - `spans` — byte ranges on [`check`] diagnostics, for an editor that wants +//! to underline the offending token. +//! - `uml` — Mermaid class diagrams and the relation guessing above. + +pub mod check; +pub mod model; +pub mod parser; +pub mod schema; +pub mod validate; + +pub use check::{Checked, Diagnostic, SourceSpan, Stage, check}; +#[allow(unused_imports)] +pub use model::*; +pub use parser::Parser; +pub use schema::{ + Change, ColumnSpec, ConfigSpec, Cost, Declarations, Diff, IndexSpec, OperationSpec, PartitionKeySpec, QueriesSpec, + Schema, TableChange, TransformReason, TransformRequest, declarations_in_source, declarations_in_tokens, plan, +}; +#[cfg(feature = "uml")] +pub use schema::{Relation, infer_relations, schemas_to_mermaid}; diff --git a/codegen/src/common/model/column.rs b/dsl/src/model/column.rs similarity index 97% rename from codegen/src/common/model/column.rs rename to dsl/src/model/column.rs index 611c9d73..71dbf7b9 100644 --- a/codegen/src/common/model/column.rs +++ b/dsl/src/model/column.rs @@ -1,8 +1,8 @@ use indexmap::IndexMap; use std::collections::HashMap; -use crate::common::model::index::Index; -use crate::common::model::{GeneratorType, IndexBackend}; +use crate::model::index::Index; +use crate::model::{GeneratorType, IndexBackend}; use proc_macro2::{Ident, TokenStream}; use quote::quote; use syn::spanned::Spanned; diff --git a/codegen/src/common/model/config.rs b/dsl/src/model/config.rs similarity index 100% rename from codegen/src/common/model/config.rs rename to dsl/src/model/config.rs diff --git a/codegen/src/common/model/index.rs b/dsl/src/model/index.rs similarity index 92% rename from codegen/src/common/model/index.rs rename to dsl/src/model/index.rs index b133c3b6..53c17be6 100644 --- a/codegen/src/common/model/index.rs +++ b/dsl/src/model/index.rs @@ -6,6 +6,7 @@ use proc_macro2::Ident; /// their current implementation and persistence semantics when `using` is /// absent. Vanilla upstream IndexSet is an explicit, parallel backend. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum IndexBackend { #[default] WorktablesIndex, diff --git a/codegen/src/common/model/mod.rs b/dsl/src/model/mod.rs similarity index 100% rename from codegen/src/common/model/mod.rs rename to dsl/src/model/mod.rs diff --git a/codegen/src/common/model/operation.rs b/dsl/src/model/operation.rs similarity index 100% rename from codegen/src/common/model/operation.rs rename to dsl/src/model/operation.rs diff --git a/codegen/src/common/model/partition.rs b/dsl/src/model/partition.rs similarity index 100% rename from codegen/src/common/model/partition.rs rename to dsl/src/model/partition.rs diff --git a/codegen/src/common/model/persistence.rs b/dsl/src/model/persistence.rs similarity index 87% rename from codegen/src/common/model/persistence.rs rename to dsl/src/model/persistence.rs index bef59fa5..ade72936 100644 --- a/codegen/src/common/model/persistence.rs +++ b/dsl/src/model/persistence.rs @@ -4,6 +4,7 @@ /// explicit `persist: false` acknowledgement before selecting an index backend /// that cannot participate in disk or S3 persistence. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum Persistence { #[default] Omitted, diff --git a/codegen/src/common/model/primary_key.rs b/dsl/src/model/primary_key.rs similarity index 79% rename from codegen/src/common/model/primary_key.rs rename to dsl/src/model/primary_key.rs index bbcb6441..d6d3bcb1 100644 --- a/codegen/src/common/model/primary_key.rs +++ b/dsl/src/model/primary_key.rs @@ -8,6 +8,7 @@ pub struct PrimaryKey { } #[derive(Debug, Clone, Copy, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum GeneratorType { None, Autoincrement, diff --git a/codegen/src/common/model/queries.rs b/dsl/src/model/queries.rs similarity index 86% rename from codegen/src/common/model/queries.rs rename to dsl/src/model/queries.rs index 7ad81643..7c311495 100644 --- a/codegen/src/common/model/queries.rs +++ b/dsl/src/model/queries.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use proc_macro2::Ident; -use crate::common::model::Operation; +use crate::model::Operation; #[derive(Debug, Default)] pub struct Queries { diff --git a/codegen/src/common/parser/attribute.rs b/dsl/src/parser/attribute.rs similarity index 97% rename from codegen/src/common/parser/attribute.rs rename to dsl/src/parser/attribute.rs index 21a1a0b9..a4c6afb4 100644 --- a/codegen/src/common/parser/attribute.rs +++ b/dsl/src/parser/attribute.rs @@ -1,8 +1,8 @@ use proc_macro2::TokenTree; use syn::spanned::Spanned as _; -use crate::common::model::{PARTITION_KEY_TYPES, PartitionKey, Persistence}; -use crate::common::parser::Parser; +use crate::model::{PARTITION_KEY_TYPES, PartitionKey, Persistence}; +use crate::parser::Parser; // TODO: Move this to separate attributes section because now it only parses persist. impl Parser { @@ -97,8 +97,8 @@ impl Parser { mod tests { use quote::quote; - use crate::common::Parser; - use crate::common::model::{PARTITION_KEY_TYPES, PartitionKey, Persistence}; + use crate::Parser; + use crate::model::{PARTITION_KEY_TYPES, PartitionKey, Persistence}; #[test] fn test_empty() { diff --git a/codegen/src/common/parser/columns.rs b/dsl/src/parser/columns.rs similarity index 97% rename from codegen/src/common/parser/columns.rs rename to dsl/src/parser/columns.rs index 65126f64..e9554b41 100644 --- a/codegen/src/common/parser/columns.rs +++ b/dsl/src/parser/columns.rs @@ -1,8 +1,8 @@ use proc_macro2::{Delimiter, TokenTree}; use syn::spanned::Spanned as _; -use crate::common::Parser; -use crate::common::model::{Columns, GeneratorType, Row}; +use crate::Parser; +use crate::model::{Columns, GeneratorType, Row}; impl Parser { pub fn parse_columns(&mut self) -> syn::Result { @@ -129,7 +129,7 @@ mod tests { use quote::quote; - use crate::common::Parser; + use crate::Parser; #[test] fn test_columns_parse() { @@ -321,7 +321,7 @@ mod tests { let mut parser = Parser::new(row_tokens); let row = parser.parse_row().unwrap(); - assert_eq!(row.index_backend, Some(crate::common::model::IndexBackend::Congee)); + assert_eq!(row.index_backend, Some(crate::model::IndexBackend::Congee)); } #[test] diff --git a/codegen/src/common/parser/config.rs b/dsl/src/parser/config.rs similarity index 86% rename from codegen/src/common/parser/config.rs rename to dsl/src/parser/config.rs index 4bf2c95c..464b7c04 100644 --- a/codegen/src/common/parser/config.rs +++ b/dsl/src/parser/config.rs @@ -3,8 +3,8 @@ use std::str::FromStr; use proc_macro2::{Delimiter, TokenTree}; use syn::spanned::Spanned; -use crate::common::Parser; -use crate::common::model::Config; +use crate::Parser; +use crate::model::Config; const CONFIG_FIELD_NAME: &str = "config"; @@ -50,6 +50,14 @@ impl Parser { let mut config = Config::default(); parser.parse_config(&mut config)?; + // `parse_updates`, `parse_indexes` and `parse_queries` have always + // consumed the comma that may follow their block. This one did not, so + // `config: { .. },` left the comma for the top-level dispatch, which + // reported it as an unexpected identifier. `config` happens to be + // written last in every declaration in this repository, which is the + // only reason nobody hit it. + self.try_parse_comma()?; + Ok(config) } @@ -120,7 +128,12 @@ impl Parser { config.row_derives = derives; } - _ => return Err(syn::Error::new(name.span(), "Unexpected identifier")), + other => { + return Err(syn::Error::new( + name.span(), + format!("Unexpected token `{other}` in `config`"), + )); + } } } diff --git a/codegen/src/common/parser/index.rs b/dsl/src/parser/index.rs similarity index 97% rename from codegen/src/common/parser/index.rs rename to dsl/src/parser/index.rs index 1f41bd0f..7dd4adcd 100644 --- a/codegen/src/common/parser/index.rs +++ b/dsl/src/parser/index.rs @@ -1,5 +1,5 @@ -use crate::common::Parser; -use crate::common::model::{Index, IndexBackend}; +use crate::Parser; +use crate::model::{Index, IndexBackend}; use indexmap::IndexMap; use proc_macro2::{Delimiter, Ident, TokenTree}; use syn::spanned::Spanned; @@ -141,8 +141,8 @@ impl Parser { mod tests { use quote::quote; - use crate::common::Parser; - use crate::common::model::IndexBackend; + use crate::Parser; + use crate::model::IndexBackend; #[test] fn absent_using_defaults_to_worktables_index() { diff --git a/codegen/src/common/parser/mod.rs b/dsl/src/parser/mod.rs similarity index 100% rename from codegen/src/common/parser/mod.rs rename to dsl/src/parser/mod.rs diff --git a/codegen/src/common/parser/name.rs b/dsl/src/parser/name.rs similarity index 98% rename from codegen/src/common/parser/name.rs rename to dsl/src/parser/name.rs index 783e853d..b33eb333 100644 --- a/codegen/src/common/parser/name.rs +++ b/dsl/src/parser/name.rs @@ -2,7 +2,7 @@ use proc_macro2::Ident; use proc_macro2::TokenTree; use syn::spanned::Spanned as _; -use crate::common::parser::Parser; +use crate::parser::Parser; impl Parser { pub fn parse_name(&mut self) -> syn::Result { @@ -73,7 +73,7 @@ impl Parser { mod tests { use quote::quote; - use crate::common::Parser; + use crate::Parser; #[test] fn test_name_parse() { diff --git a/codegen/src/common/parser/punct.rs b/dsl/src/parser/punct.rs similarity index 98% rename from codegen/src/common/parser/punct.rs rename to dsl/src/parser/punct.rs index 160956f1..a2ce294b 100644 --- a/codegen/src/common/parser/punct.rs +++ b/dsl/src/parser/punct.rs @@ -1,7 +1,7 @@ use proc_macro2::TokenTree; use syn::spanned::Spanned; -use crate::common::parser::Parser; +use crate::parser::Parser; impl Parser { /// Parses ':' from [`proc_macro2::TokenStream`]. diff --git a/codegen/src/common/parser/queries/delete.rs b/dsl/src/parser/queries/delete.rs similarity index 84% rename from codegen/src/common/parser/queries/delete.rs rename to dsl/src/parser/queries/delete.rs index ffe5aee9..9d2cb80c 100644 --- a/codegen/src/common/parser/queries/delete.rs +++ b/dsl/src/parser/queries/delete.rs @@ -3,8 +3,8 @@ use std::collections::HashMap; use proc_macro2::{Ident, TokenTree}; use syn::spanned::Spanned; -use crate::common::Parser; -use crate::common::model::Operation; +use crate::Parser; +use crate::model::Operation; impl Parser { pub fn parse_deletes(&mut self) -> syn::Result> { @@ -28,7 +28,11 @@ impl Parser { .ok_or(syn::Error::new(self.input.span(), "Expected operation declarations"))?; if let TokenTree::Group(ops) = ops { let mut parser = Parser::new(ops.stream()); - parser.parse_operations() + let operations = parser.parse_operations()?; + // Symmetry with `parse_updates`: consume a comma after the block, + // so a `delete` block is not required to be written last. + self.try_parse_comma()?; + Ok(operations) } else { Err(syn::Error::new(ops.span(), "Expected operation declarations")) } @@ -40,7 +44,7 @@ mod tests { use proc_macro2::{Ident, Span}; use quote::quote; - use crate::common::Parser; + use crate::Parser; #[test] fn test_update() { diff --git a/codegen/src/common/parser/queries/in_place.rs b/dsl/src/parser/queries/in_place.rs similarity index 83% rename from codegen/src/common/parser/queries/in_place.rs rename to dsl/src/parser/queries/in_place.rs index 47fc83f8..2302e3d4 100644 --- a/codegen/src/common/parser/queries/in_place.rs +++ b/dsl/src/parser/queries/in_place.rs @@ -3,8 +3,8 @@ use std::collections::HashMap; use proc_macro2::{Ident, TokenTree}; use syn::spanned::Spanned; -use crate::common::Parser; -use crate::common::model::Operation; +use crate::Parser; +use crate::model::Operation; impl Parser { pub fn parse_in_place(&mut self) -> syn::Result> { @@ -28,7 +28,11 @@ impl Parser { .ok_or(syn::Error::new(self.input.span(), "Expected operation declarations"))?; if let TokenTree::Group(ops) = ops { let mut parser = Parser::new(ops.stream()); - parser.parse_operations() + let operations = parser.parse_operations()?; + // Symmetry with `parse_updates`: consume a comma after the block, + // so a `in_place` block is not required to be written last. + self.try_parse_comma()?; + Ok(operations) } else { Err(syn::Error::new(ops.span(), "Expected operation declarations")) } @@ -40,7 +44,7 @@ mod tests { use proc_macro2::{Ident, Span}; use quote::quote; - use crate::common::Parser; + use crate::Parser; #[test] fn test_update() { diff --git a/codegen/src/common/parser/queries/mod.rs b/dsl/src/parser/queries/mod.rs similarity index 85% rename from codegen/src/common/parser/queries/mod.rs rename to dsl/src/parser/queries/mod.rs index 75d0f3a8..a74140c0 100644 --- a/codegen/src/common/parser/queries/mod.rs +++ b/dsl/src/parser/queries/mod.rs @@ -7,8 +7,8 @@ mod update; use proc_macro2::TokenTree; use syn::spanned::Spanned; -use crate::common::Parser; -use crate::common::model::Queries; +use crate::Parser; +use crate::model::Queries; impl Parser { pub fn parse_queries(&mut self) -> syn::Result { @@ -50,7 +50,12 @@ impl Parser { let in_place = parser.parse_in_place()?; queries.in_place = in_place; } - _ => return Err(syn::Error::new(ident.span(), "Unexpected identifier")), + other => { + return Err(syn::Error::new( + ident.span(), + format!("Unexpected token `{other}`; expected one of `update`, `delete`, `in_place`"), + )); + } } } } else { diff --git a/codegen/src/common/parser/queries/operation.rs b/dsl/src/parser/queries/operation.rs similarity index 97% rename from codegen/src/common/parser/queries/operation.rs rename to dsl/src/parser/queries/operation.rs index a3a0b1e1..c8894ab6 100644 --- a/codegen/src/common/parser/queries/operation.rs +++ b/dsl/src/parser/queries/operation.rs @@ -2,8 +2,8 @@ use proc_macro2::{Ident, TokenTree}; use std::collections::HashMap; use syn::spanned::Spanned; -use crate::common::model::Operation; -use crate::common::parser::Parser; +use crate::model::Operation; +use crate::parser::Parser; impl Parser { pub fn parse_operations(&mut self) -> syn::Result> { @@ -96,7 +96,7 @@ impl Parser { mod tests { use quote::quote; - use crate::common::parser::Parser; + use crate::parser::Parser; #[test] fn test_operation() { diff --git a/codegen/src/common/parser/queries/select.rs b/dsl/src/parser/queries/select.rs similarity index 95% rename from codegen/src/common/parser/queries/select.rs rename to dsl/src/parser/queries/select.rs index 4c2aac84..10ea38f9 100644 --- a/codegen/src/common/parser/queries/select.rs +++ b/dsl/src/parser/queries/select.rs @@ -3,8 +3,8 @@ use std::collections::HashMap; use proc_macro2::{Ident, TokenTree}; use syn::spanned::Spanned; -use crate::common::Parser; -use crate::common::model::Operation; +use crate::Parser; +use crate::model::Operation; impl Parser { pub fn _parse_selects(&mut self) -> syn::Result> { @@ -40,7 +40,7 @@ mod tests { use proc_macro2::{Ident, Span}; use quote::quote; - use crate::common::Parser; + use crate::Parser; #[test] fn test_update() { diff --git a/codegen/src/common/parser/queries/update.rs b/dsl/src/parser/queries/update.rs similarity index 95% rename from codegen/src/common/parser/queries/update.rs rename to dsl/src/parser/queries/update.rs index d51fb6b9..ed3d8f02 100644 --- a/codegen/src/common/parser/queries/update.rs +++ b/dsl/src/parser/queries/update.rs @@ -3,8 +3,8 @@ use std::collections::HashMap; use proc_macro2::{Ident, TokenTree}; use syn::spanned::Spanned; -use crate::common::Parser; -use crate::common::model::Operation; +use crate::Parser; +use crate::model::Operation; impl Parser { pub fn parse_updates(&mut self) -> syn::Result> { @@ -42,7 +42,7 @@ mod tests { use proc_macro2::{Ident, Span}; use quote::quote; - use crate::common::Parser; + use crate::Parser; #[test] fn test_update() { diff --git a/dsl/src/schema/diff.rs b/dsl/src/schema/diff.rs new file mode 100644 index 00000000..f4494b4b --- /dev/null +++ b/dsl/src/schema/diff.rs @@ -0,0 +1,636 @@ +//! What changed between two schemas, and what that costs. +//! +//! # The question this answers +//! +//! A table opens, and the version on disk does not match the version the +//! binary was compiled with. Something has to decide what to do about it, and +//! today that decision is made by a human who wrote `version_tables: { 1 => +//! v1::UserV1WorkTable }` and kept the old table definition by hand, forever, +//! for every version that ever existed. That hand-maintenance is the whole +//! reason migrations get put off. +//! +//! With both schemas as data, the decision can be computed. [`Diff::between`] +//! says what changed; [`Cost`] says what it costs to apply; and +//! [`Diff::transforms_required`] says which parts a human still has to write, +//! because those are the parts that need intent rather than mechanism. +//! +//! # Cost is about links, not about fields +//! +//! A row is addressed by a `Link { page_id, offset, length }`, and every index +//! holds links. So the question that decides the cost of a change is not "how +//! many columns moved" but "is a row still where it was". A change to the row's +//! archived layout invalidates every link in the table at once, and the only +//! way through is to write every row somewhere else. A change to an index +//! invalidates nothing: the rows have not moved, and the index can be rebuilt +//! from them. That is why [`Cost`] has the shape it does, and why adding a +//! column is expensive while adding an index is not. +//! +//! # What it cannot tell you +//! +//! A rename is a drop and an add. Nothing in a declaration distinguishes +//! `email` becoming `email_address` from `email` being deleted while an +//! unrelated `email_address` appears, and guessing from type equality would be +//! wrong exactly when it mattered. The diff reports both changes and asks for a +//! transform, which is where the intent belongs. + +use std::collections::BTreeSet; +use std::fmt::Write as _; + +use super::{ColumnSpec, IndexSpec, PartitionKeySpec, Schema}; +use crate::model::{IndexBackend, Persistence}; + +/// What applying a change costs. +/// +/// Ordered from cheapest to most expensive, so the cost of a whole diff is the +/// maximum over its changes. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum Cost { + /// Nothing on disk changes. The generated code differs and the data does + /// not: new queries, different row derives, a version bump on its own. + Nothing, + /// Indexes are rebuilt from rows that stay where they are. No link is + /// invalidated, so this can be done in place. + RebuildIndexes, + /// Every row is written somewhere else, because its archived layout + /// changed. Every link in the table is invalidated at once, which is why + /// there is no cheaper version of this: it is a copy-forward into a new + /// space, with the old one left untouched until it succeeds. + RewriteRows, + /// Cannot be planned. A person has to say what they meant before anything + /// can be applied. + NeedsIntent, +} + +impl Cost { + /// A short explanation, for a report or an error message. + pub fn describe(self) -> &'static str { + match self { + Self::Nothing => "no change on disk", + Self::RebuildIndexes => "indexes rebuilt in place; rows are not moved", + Self::RewriteRows => "every row is copied forward into a new space", + Self::NeedsIntent => "cannot be planned automatically", + } + } +} + +/// One difference between two schemas. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum Change { + /// The declared version changed. This is the trigger for a migration + /// rather than a cost of one. + Version { + /// The version on disk. + from: u32, + /// The version the binary declares. + to: u32, + }, + /// The table is named differently, which means it is a different space on + /// disk and nothing links the two but a person saying so. + Renamed { + /// The stored name. + from: String, + /// The declared name. + to: String, + }, + /// `persist` changed. + PersistenceChanged { + /// What was stored. + from: Persistence, + /// What is declared. + to: Persistence, + }, + /// The routing key changed. The key is not in the row, so it cannot be + /// recomputed from the data: a row's partition is only knowable from where + /// it already is. + PartitionKeyChanged { + /// What was stored. + from: Option, + /// What is declared. + to: Option, + }, + /// The primary key's columns changed, in membership or in order. Order + /// counts: it decides the layout of the generated key type. + PrimaryKeyChanged { + /// The stored key columns, in order. + from: Vec, + /// The declared key columns, in order. + to: Vec, + }, + /// A column appeared. + ColumnAdded(ColumnSpec), + /// A column is gone, and its values with it. + ColumnDropped(ColumnSpec), + /// A column kept its name and changed its type. + ColumnTypeChanged { + /// Column name. + name: String, + /// The stored type. + from: String, + /// The declared type. + to: String, + }, + /// A column gained or lost `optional`. + ColumnOptionalityChanged { + /// Column name. + name: String, + /// Whether it is optional now. + now_optional: bool, + }, + /// A column moved. Declaration order is the row struct's field order, so + /// moving one changes the archived layout as surely as changing its type. + ColumnMoved { + /// Column name. + name: String, + /// Its stored position. + from: usize, + /// Its declared position. + to: usize, + }, + /// A secondary index appeared. + IndexAdded(IndexSpec), + /// A secondary index is gone. + IndexDropped(IndexSpec), + /// An index of the same name is now built over a different column. + IndexColumnChanged { + /// Index name. + name: String, + /// The stored column. + from: String, + /// The declared column. + to: String, + }, + /// An index gained or lost `unique`. + IndexUniquenessChanged { + /// Index name. + name: String, + /// Whether it is unique now. + now_unique: bool, + }, + /// An index kept its shape and changed its implementation. + IndexBackendChanged { + /// Index name. + name: String, + /// The stored backend. + from: IndexBackend, + /// The declared backend. + to: IndexBackend, + }, + /// The primary index's implementation changed. + PrimaryIndexBackendChanged { + /// The stored backend. + from: IndexBackend, + /// The declared backend. + to: IndexBackend, + }, + /// The generated queries differ. Nothing on disk depends on them. + QueriesChanged, + /// The `config` block differs. `page_size` is pinned to the on-disk page + /// size for persisted tables, so what is left here cannot reach the data. + ConfigChanged, +} + +impl Change { + /// What applying this change costs. + pub fn cost(&self) -> Cost { + match self { + Self::Version { .. } | Self::QueriesChanged | Self::ConfigChanged => Cost::Nothing, + + Self::IndexAdded(_) + | Self::IndexDropped(_) + | Self::IndexColumnChanged { .. } + | Self::IndexUniquenessChanged { .. } + | Self::IndexBackendChanged { .. } + | Self::PrimaryIndexBackendChanged { .. } => Cost::RebuildIndexes, + + Self::ColumnAdded(_) + | Self::ColumnDropped(_) + | Self::ColumnTypeChanged { .. } + | Self::ColumnOptionalityChanged { .. } + | Self::ColumnMoved { .. } => Cost::RewriteRows, + + Self::Renamed { .. } + | Self::PersistenceChanged { .. } + | Self::PartitionKeyChanged { .. } + | Self::PrimaryKeyChanged { .. } => Cost::NeedsIntent, + } + } + + /// What a person has to supply before this change can be applied, if + /// anything. + /// + /// The rule is that the planner can invent a value only when there is + /// exactly one it could be. Widening a column to `optional` has one answer, + /// `Some(old)`. Narrowing it does not: what a `None` should become is a + /// question about the data, not about the schema. + pub fn transform_required(&self) -> Option { + match self { + Self::ColumnAdded(column) if !column.optional => Some(TransformRequest { + column: column.name.clone(), + reason: TransformReason::NoValueToFillItWith { ty: column.ty.clone() }, + }), + Self::ColumnTypeChanged { name, from, to } => Some(TransformRequest { + column: name.clone(), + reason: TransformReason::NoConversionExists { + from: from.clone(), + to: to.clone(), + }, + }), + Self::ColumnOptionalityChanged { + name, + now_optional: false, + } => Some(TransformRequest { + column: name.clone(), + reason: TransformReason::NothingToPutWhereNoneWas, + }), + _ => None, + } + } + + /// Something true about this change that its cost does not say. + pub fn warning(&self) -> Option { + match self { + Self::IndexUniquenessChanged { name, now_unique: true } => Some(format!( + "index `{name}` becomes unique: rebuilding it fails if the existing rows already \ + hold a duplicate, and that is only knowable by reading them" + )), + Self::ColumnDropped(column) => Some(format!( + "column `{}` is dropped: its values are not carried anywhere and are gone once the \ + old space is removed", + column.name + )), + _ => None, + } + } +} + +/// Something a person has to write before a plan can run. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct TransformRequest { + /// The column it concerns. + pub column: String, + /// Why the planner cannot decide it. + pub reason: TransformReason, +} + +/// Why a change needs a human. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum TransformReason { + /// A new column that is not `optional` has no value in any existing row, + /// and nothing in the schema says what it should be. + NoValueToFillItWith { + /// The new column's type. + ty: String, + }, + /// The column's type changed and the schema does not say how one becomes + /// the other. + NoConversionExists { + /// The stored type. + from: String, + /// The declared type. + to: String, + }, + /// A column stopped being `optional`, so every stored `None` needs a value + /// or the row needs dropping. + NothingToPutWhereNoneWas, +} + +impl TransformReason { + /// A one-line explanation, for a report. + pub fn describe(&self) -> String { + match self { + Self::NoValueToFillItWith { ty } => { + format!("new non-optional column of type `{ty}` has no value in existing rows") + } + Self::NoConversionExists { from, to } => { + format!("no conversion from `{from}` to `{to}` is implied by the declaration") + } + Self::NothingToPutWhereNoneWas => { + "stored `None` values need a replacement or the rows need dropping".to_string() + } + } + } +} + +/// Everything that differs between two schemas for one table. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct Diff { + /// The table's stored name. + pub table: String, + /// The differences, in a fixed order: identity, then columns, then + /// indexes, then the parts that cannot reach the data. + pub changes: Vec, +} + +impl Diff { + /// Compare a stored schema against a declared one. + /// + /// `stored` is what is on disk and `declared` is what the binary was + /// compiled with, and the direction matters: "a column was added" means + /// added by the binary, and so absent from every row on disk. + pub fn between(stored: &Schema, declared: &Schema) -> Self { + let mut changes = Vec::new(); + + if stored.name != declared.name { + changes.push(Change::Renamed { + from: stored.name.clone(), + to: declared.name.clone(), + }); + } + if stored.version != declared.version { + changes.push(Change::Version { + from: stored.version, + to: declared.version, + }); + } + if stored.persist != declared.persist { + changes.push(Change::PersistenceChanged { + from: stored.persist, + to: declared.persist, + }); + } + if stored.partition_by != declared.partition_by { + changes.push(Change::PartitionKeyChanged { + from: stored.partition_by.clone(), + to: declared.partition_by.clone(), + }); + } + + let stored_key: Vec = stored.primary_key().iter().map(|c| c.name.clone()).collect(); + let declared_key: Vec = declared.primary_key().iter().map(|c| c.name.clone()).collect(); + if stored_key != declared_key { + changes.push(Change::PrimaryKeyChanged { + from: stored_key, + to: declared_key, + }); + } + + diff_columns(stored, declared, &mut changes); + diff_indexes(stored, declared, &mut changes); + + if stored.primary_index_backend() != declared.primary_index_backend() { + changes.push(Change::PrimaryIndexBackendChanged { + from: stored.primary_index_backend(), + to: declared.primary_index_backend(), + }); + } + if stored.queries != declared.queries { + changes.push(Change::QueriesChanged); + } + if stored.config != declared.config { + changes.push(Change::ConfigChanged); + } + + Self { + table: stored.name.clone(), + changes, + } + } + + /// Whether the two schemas are the same. + pub fn is_empty(&self) -> bool { + self.changes.is_empty() + } + + /// Whether the rows on disk can be read by the declared type as they are. + /// + /// This is the question the fast path asks. A version match plus this + /// returning true is an optimistic load with nothing to do; a version match + /// plus this returning false is a schema changed without a version bump, + /// which is a mistake rather than a migration and should be said so. + pub fn rows_are_readable(&self) -> bool { + self.cost() < Cost::RewriteRows + } + + /// The cost of the whole diff, which is the cost of its worst change. + pub fn cost(&self) -> Cost { + self.changes.iter().map(Change::cost).max().unwrap_or(Cost::Nothing) + } + + /// Everything a person has to write before this can be applied. + pub fn transforms_required(&self) -> Vec { + self.changes.iter().filter_map(Change::transform_required).collect() + } + + /// Everything true about this diff that its cost does not say. + pub fn warnings(&self) -> Vec { + self.changes.iter().filter_map(Change::warning).collect() + } + + /// A report, for an error message or a designer's migration pane. + pub fn describe(&self) -> String { + if self.is_empty() { + return format!("`{}` is unchanged", self.table); + } + + let mut out = format!("`{}`: {}\n", self.table, self.cost().describe()); + for change in &self.changes { + let _ = writeln!(out, " {}", describe_change(change)); + } + let transforms = self.transforms_required(); + if !transforms.is_empty() { + out.push_str(" needs a transform written for:\n"); + for transform in transforms { + let _ = writeln!(out, " {}: {}", transform.column, transform.reason.describe()); + } + } + for warning in self.warnings() { + let _ = writeln!(out, " note: {warning}"); + } + out + } +} + +fn describe_change(change: &Change) -> String { + match change { + Change::Version { from, to } => format!("version {from} -> {to}"), + Change::Renamed { from, to } => format!("table renamed {from} -> {to}"), + Change::PersistenceChanged { from, to } => format!("persistence {from:?} -> {to:?}"), + Change::PartitionKeyChanged { from, to } => { + let name = |key: &Option| match key { + Some(key) => format!("{}: {}", key.name, key.ty), + None => "none".to_string(), + }; + format!("partition key {} -> {}", name(from), name(to)) + } + Change::PrimaryKeyChanged { from, to } => { + format!("primary key ({}) -> ({})", from.join(", "), to.join(", ")) + } + Change::ColumnAdded(column) => format!( + "column added: {}: {}{}", + column.name, + column.ty, + if column.optional { " optional" } else { "" } + ), + Change::ColumnDropped(column) => format!("column dropped: {}: {}", column.name, column.ty), + Change::ColumnTypeChanged { name, from, to } => format!("column {name}: {from} -> {to}"), + Change::ColumnOptionalityChanged { name, now_optional } => { + if *now_optional { + format!("column {name} became optional") + } else { + format!("column {name} stopped being optional") + } + } + Change::ColumnMoved { name, from, to } => format!("column {name} moved from position {from} to {to}"), + Change::IndexAdded(index) => format!("index added: {} over {}", index.name, index.column), + Change::IndexDropped(index) => format!("index dropped: {} over {}", index.name, index.column), + Change::IndexColumnChanged { name, from, to } => format!("index {name}: {from} -> {to}"), + Change::IndexUniquenessChanged { name, now_unique } => { + if *now_unique { + format!("index {name} became unique") + } else { + format!("index {name} stopped being unique") + } + } + Change::IndexBackendChanged { name, from, to } => { + format!("index {name}: {} -> {}", from.name(), to.name()) + } + Change::PrimaryIndexBackendChanged { from, to } => { + format!("primary index: {} -> {}", from.name(), to.name()) + } + Change::QueriesChanged => "queries changed".to_string(), + Change::ConfigChanged => "config changed".to_string(), + } +} + +fn diff_columns(stored: &Schema, declared: &Schema, changes: &mut Vec) { + for (position, column) in declared.columns.iter().enumerate() { + match stored.column(&column.name) { + None => changes.push(Change::ColumnAdded(column.clone())), + Some(before) => { + if before.ty != column.ty { + changes.push(Change::ColumnTypeChanged { + name: column.name.clone(), + from: before.ty.clone(), + to: column.ty.clone(), + }); + } + if before.optional != column.optional { + changes.push(Change::ColumnOptionalityChanged { + name: column.name.clone(), + now_optional: column.optional, + }); + } + let was_at = stored + .columns + .iter() + .position(|c| c.name == column.name) + .expect("the column was just found by name"); + if was_at != position { + changes.push(Change::ColumnMoved { + name: column.name.clone(), + from: was_at, + to: position, + }); + } + } + } + } + for column in &stored.columns { + if declared.column(&column.name).is_none() { + changes.push(Change::ColumnDropped(column.clone())); + } + } +} + +fn diff_indexes(stored: &Schema, declared: &Schema, changes: &mut Vec) { + let find = |schema: &Schema, name: &str| schema.indexes.iter().find(|index| index.name == name).cloned(); + + let names: BTreeSet<&str> = stored + .indexes + .iter() + .chain(declared.indexes.iter()) + .map(|index| index.name.as_str()) + .collect(); + + for name in names { + match (find(stored, name), find(declared, name)) { + (None, Some(added)) => changes.push(Change::IndexAdded(added)), + (Some(dropped), None) => changes.push(Change::IndexDropped(dropped)), + (Some(before), Some(after)) => { + if before.column != after.column { + changes.push(Change::IndexColumnChanged { + name: name.to_string(), + from: before.column, + to: after.column, + }); + } + if before.unique != after.unique { + changes.push(Change::IndexUniquenessChanged { + name: name.to_string(), + now_unique: after.unique, + }); + } + if before.backend != after.backend { + changes.push(Change::IndexBackendChanged { + name: name.to_string(), + from: before.backend, + to: after.backend, + }); + } + } + (None, None) => unreachable!("the name came from one of the two"), + } + } +} + +/// What happened to one table between two sets of schemas. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub enum TableChange { + /// A table the binary declares that is not on disk. There is nothing to + /// migrate: it is created empty. + Created(String), + /// A table on disk that the binary no longer declares. Nothing reads it, + /// and nothing here deletes it either: that is a decision, not a + /// consequence. + Dropped(String), + /// A table that exists on both sides and differs. + Changed(Diff), +} + +impl TableChange { + /// What applying this costs. + pub fn cost(&self) -> Cost { + match self { + // A new table has no rows to move. + Self::Created(_) => Cost::Nothing, + // Whether to delete a table's data is not something a diff can + // decide, however obvious the answer looks from the declaration. + Self::Dropped(_) => Cost::NeedsIntent, + Self::Changed(diff) => diff.cost(), + } + } +} + +/// Compare a stored set of schemas against a declared one. +/// +/// Tables are matched by name, which is also how they are matched on disk: +/// a space's name is its identity. A renamed table therefore reads as one +/// dropped and one created, and saying it was a rename is a person's job. +pub fn plan(stored: &[Schema], declared: &[Schema]) -> Vec { + let mut changes = Vec::new(); + + for schema in declared { + match stored.iter().find(|other| other.name == schema.name) { + None => changes.push(TableChange::Created(schema.name.clone())), + Some(before) => { + let diff = Diff::between(before, schema); + if !diff.is_empty() { + changes.push(TableChange::Changed(diff)); + } + } + } + } + for schema in stored { + if !declared.iter().any(|other| other.name == schema.name) { + changes.push(TableChange::Dropped(schema.name.clone())); + } + } + + changes +} diff --git a/dsl/src/schema/emit_dsl.rs b/dsl/src/schema/emit_dsl.rs new file mode 100644 index 00000000..6ad77160 --- /dev/null +++ b/dsl/src/schema/emit_dsl.rs @@ -0,0 +1,167 @@ +//! Rendering a [`Schema`] back into the declaration text it came from. +//! +//! This is the half of the round trip that makes a designer possible. Reading +//! a schema is enough to draw it; writing one back is what lets the drawing be +//! edited and the result be a file the compiler accepts. +//! +//! The output is the macro body, not the invocation: the caller decides whether +//! it is going inside `worktable! { .. }`, into a `.wt` file, or into a diff. +//! [`Schema::to_macro_invocation`] wraps it when the invocation is what is +//! wanted. +//! +//! Nothing here tries to preserve the input's formatting or its comments. It +//! cannot: the parser is token-based, so comments never reach the model, and a +//! `Schema` is a description of a schema rather than of a file. What is +//! preserved is meaning, and the round-trip test asserts exactly that and +//! nothing more. + +use std::fmt::Write as _; + +use super::{ColumnSpec, IndexSpec, OperationSpec, Schema}; +use crate::model::{GeneratorType, IndexBackend, Persistence}; + +const INDENT: &str = " "; + +impl Schema { + /// Render the declaration body. + pub fn to_dsl(&self) -> String { + let mut out = String::new(); + + let _ = writeln!(out, "name: {},", self.name); + let _ = writeln!(out, "version: {},", self.version); + + match self.persist { + // An omitted `persist` is not the same as `persist: false`: the + // macro requires the acknowledgement before it will accept an + // index backend that cannot be persisted, so writing one in would + // silently answer a question the author left open. + Persistence::Omitted => {} + Persistence::MemoryOnly => { + let _ = writeln!(out, "persist: false,"); + } + Persistence::Persisted => { + let _ = writeln!(out, "persist: true,"); + } + } + + if let Some(key) = &self.partition_by { + let _ = writeln!(out, "partition_by: {}: {},", key.name, key.ty); + } + + let _ = writeln!(out, "columns: {{"); + for column in &self.columns { + let _ = writeln!(out, "{INDENT}{},", column_to_dsl(column)); + } + let _ = writeln!(out, "}},"); + + if !self.indexes.is_empty() { + let _ = writeln!(out, "indexes: {{"); + for index in &self.indexes { + let _ = writeln!(out, "{INDENT}{},", index_to_dsl(index)); + } + let _ = writeln!(out, "}},"); + } + + if !self.queries.is_empty() { + let _ = writeln!(out, "queries: {{"); + write_query_block(&mut out, "update", &self.queries.updates); + write_query_block(&mut out, "delete", &self.queries.deletes); + write_query_block(&mut out, "in_place", &self.queries.in_place); + let _ = writeln!(out, "}},"); + } + + if !self.config.is_empty() { + let _ = writeln!(out, "config: {{"); + if let Some(page_size) = self.config.page_size { + let _ = writeln!(out, "{INDENT}page_size: {page_size},"); + } + if !self.config.row_derives.is_empty() { + // `row_derives` reads identifiers until it meets another config + // key, so it has to be written last of the two. + let _ = writeln!(out, "{INDENT}row_derives: {},", self.config.row_derives.join(", ")); + } + // No comma: `parse_configs` does not consume one after its block, so a + // trailing comma here reaches the top-level dispatch as a `,` token. + // `config` is emitted last, so nothing needs to follow it. + let _ = writeln!(out, "}}"); + } + + out + } + + /// Render the declaration as a complete `worktable!` invocation, ready to + /// be written into a Rust file. + pub fn to_macro_invocation(&self) -> String { + let mut out = String::from("worktable! {\n"); + for line in self.to_dsl().lines() { + if line.is_empty() { + out.push('\n'); + } else { + let _ = writeln!(out, "{INDENT}{line}"); + } + } + out.push_str("}\n"); + out + } +} + +fn column_to_dsl(column: &ColumnSpec) -> String { + let mut out = format!("{}: {}", column.name, column.ty); + + if column.primary_key { + out.push_str(" primary_key"); + match column.generator { + GeneratorType::None => {} + GeneratorType::Autoincrement => out.push_str(" autoincrement"), + GeneratorType::Custom => out.push_str(" custom"), + } + } + + if column.optional { + out.push_str(" optional"); + } + + // A primary-key column always carries a backend once parsed, because the + // model fills the default in. Writing the default back out would be + // correct but noisy, and the point of this emitter is text a person will + // read, so only a deliberate choice is written. + if let Some(backend) = column.index_backend + && backend != IndexBackend::default() + { + let _ = write!(out, " using {}", backend.name()); + } + + out +} + +fn index_to_dsl(index: &IndexSpec) -> String { + let mut out = format!("{}: {}", index.name, index.column); + if index.unique { + out.push_str(" unique"); + } + if index.backend != IndexBackend::default() { + let _ = write!(out, " using {}", index.backend.name()); + } + out +} + +fn write_query_block(out: &mut String, kind: &str, operations: &[OperationSpec]) { + if operations.is_empty() { + return; + } + let _ = writeln!(out, "{INDENT}{kind}: {{"); + for operation in operations { + let _ = writeln!( + out, + "{INDENT}{INDENT}{}({}) by {},", + operation.name, + operation.columns.join(", "), + operation.by + ); + } + // No comma after the closing brace. All six block parsers now consume one + // if it is there, so both forms parse; omitting it is kept because it is + // also what versions before 1.0.0-beta.17 accept, and emitted text is + // routinely fed to a macro older than the emitter that wrote it. + let _ = writeln!(out, "{INDENT}}}"); +} diff --git a/dsl/src/schema/emit_uml.rs b/dsl/src/schema/emit_uml.rs new file mode 100644 index 00000000..25bef40c --- /dev/null +++ b/dsl/src/schema/emit_uml.rs @@ -0,0 +1,216 @@ +//! Rendering schemas as UML, for a designer to draw. +//! +//! The target is Mermaid's `classDiagram`, which is UML class notation and +//! renders anywhere Markdown does: a GitHub comment, a docs page, an editor +//! preview, and the designer itself. Emitting text rather than a picture keeps +//! this crate free of a rendering dependency and keeps the output diffable, +//! which matters when the diagram is generated from a schema in version +//! control. +//! +//! # The mapping +//! +//! A table is a class. Columns are attributes, carrying their markers in +//! brackets: `[PK]`, `[UK ]` for a unique index, `[IX ]` for a +//! non-unique one, and the backend name when a non-default one was selected. +//! Queries are operations, since that is what they are: a named thing the +//! table can be asked to do, with the columns it touches as parameters and the +//! column it selects by as the qualifier. An `optional` column is written +//! `Option~T~`, Mermaid's spelling of a generic. +//! +//! The partition key is not a column and is not drawn as one. It appears in a +//! note, because it describes the table rather than a row: it is stored once +//! per partition and no query can reference it. +//! +//! # Relations +//! +//! The schema language has no foreign keys, so there is nothing to draw an +//! association from. [`infer_relations`] guesses instead, by a single rule +//! stated in its own documentation, and returns what it guessed so a caller +//! can show the user rather than assert it. [`schemas_to_mermaid`] draws the +//! guesses as dependencies (`..>`) rather than associations, because a dashed +//! arrow is the honest notation for a link the declaration does not make. + +use std::fmt::Write as _; + +use convert_case::{Case, Casing as _}; + +use super::{ColumnSpec, Schema}; +use crate::model::{GeneratorType, IndexBackend, Persistence}; + +impl Schema { + /// Render this schema as a Mermaid `classDiagram`. + pub fn to_mermaid(&self) -> String { + let mut out = String::from("classDiagram\n"); + self.write_mermaid_class(&mut out); + out + } + + fn write_mermaid_class(&self, out: &mut String) { + let _ = writeln!(out, " class {} {{", self.name); + let _ = writeln!(out, " <<{}>>", self.stereotype()); + + for column in &self.columns { + let _ = writeln!(out, " +{}", self.column_member(column)); + } + + for (kind, operations) in [ + ("update", &self.queries.updates), + ("delete", &self.queries.deletes), + ("in_place", &self.queries.in_place), + ] { + for operation in operations { + let _ = writeln!( + out, + " +{kind}_{}({}) by_{}", + operation.name, + operation.columns.join(", "), + operation.by + ); + } + } + + let _ = writeln!(out, " }}"); + + if let Some(key) = &self.partition_by { + let _ = writeln!( + out, + " note for {} \"partitioned by {}: {}\"", + self.name, key.name, key.ty + ); + } + } + + fn stereotype(&self) -> String { + let persistence = match self.persist { + Persistence::Persisted => "persisted", + Persistence::MemoryOnly => "in-memory", + Persistence::Omitted => "in-memory by default", + }; + format!("v{} {persistence}", self.version) + } + + fn column_member(&self, column: &ColumnSpec) -> String { + let ty = if column.optional { + format!("Option~{}~", column.ty) + } else { + column.ty.clone() + }; + let mut member = format!("{} : {ty}", column.name); + + let mut markers = Vec::new(); + if column.primary_key { + markers.push("PK".to_string()); + match column.generator { + GeneratorType::None => {} + GeneratorType::Autoincrement => markers.push("autoincrement".to_string()), + GeneratorType::Custom => markers.push("custom".to_string()), + } + if let Some(backend) = column.index_backend + && backend != IndexBackend::default() + { + markers.push(backend.name().to_string()); + } + } + for index in self.indexes.iter().filter(|index| index.column == column.name) { + let kind = if index.unique { "UK" } else { "IX" }; + let mut marker = format!("{kind} {}", index.name); + if index.backend != IndexBackend::default() { + let _ = write!(marker, " {}", index.backend.name()); + } + markers.push(marker); + } + + if !markers.is_empty() { + let _ = write!(member, " [{}]", markers.join(", ")); + } + member + } +} + +/// A link one schema appears to make to another. +/// +/// Appears because nothing in the declaration says so: see [`infer_relations`]. +#[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct Relation { + /// The table holding the referring column. + pub from: String, + /// The referring column. + pub column: String, + /// The table it appears to refer to. + pub to: String, + /// The primary-key column it appears to match. + pub to_column: String, +} + +/// Guess the links between schemas from their column names. +/// +/// The schema language has no foreign keys, so there is no declared answer to +/// recover and this is a heuristic, deliberately a narrow one. A column of +/// table `A` is taken to refer to table `B` when all of the following hold: +/// +/// - `B` has exactly one primary-key column. A composite key has no single +/// column to point at, and guessing which part was meant is worse than +/// drawing nothing. +/// - The column is named `_`, where `` is `B`'s name in snake_case: +/// `project_id` for `Project { id }`. +/// - The two types are identical, ignoring `optional`. A `String project_id` +/// against a `u64 Project::id` is a name collision, not a reference. +/// - The column is not itself part of `A`'s primary key. Those are usually a +/// composite key's own parts rather than a reference outward, and drawing +/// them as references clutters the diagram where it is already busiest. +/// +/// It will miss links written under any other convention, and it can be wrong. +/// Callers showing this to a user should show it as a suggestion. +pub fn infer_relations(schemas: &[Schema]) -> Vec { + let targets: Vec<(&Schema, &ColumnSpec)> = schemas + .iter() + .filter_map(|schema| { + let key = schema.primary_key(); + match key.as_slice() { + [single] => Some((schema, *single)), + _ => None, + } + }) + .collect(); + + let mut relations = Vec::new(); + for schema in schemas { + for column in &schema.columns { + if column.primary_key { + continue; + } + for (target, key) in &targets { + if target.name == schema.name { + continue; + } + let expected = format!("{}_{}", target.name.to_case(Case::Snake), key.name); + if column.name == expected && column.ty == key.ty { + relations.push(Relation { + from: schema.name.clone(), + column: column.name.clone(), + to: target.name.clone(), + to_column: key.name.clone(), + }); + } + } + } + } + relations +} + +/// Render several schemas as one Mermaid `classDiagram`, with the links from +/// [`infer_relations`] drawn as dependencies. +/// +/// The arrow is `..>` rather than an association because the link is inferred. +/// A solid line would claim the declaration says something it does not. +pub fn schemas_to_mermaid(schemas: &[Schema]) -> String { + let mut out = String::from("classDiagram\n"); + for schema in schemas { + schema.write_mermaid_class(&mut out); + } + for relation in infer_relations(schemas) { + let _ = writeln!(out, " {} ..> {} : {}", relation.from, relation.to, relation.column); + } + out +} diff --git a/dsl/src/schema/mod.rs b/dsl/src/schema/mod.rs new file mode 100644 index 00000000..56f5ee9b --- /dev/null +++ b/dsl/src/schema/mod.rs @@ -0,0 +1,410 @@ +//! A schema as plain data, and the emitters that render one. +//! +//! # Why a second representation +//! +//! [`crate::model`] is the macro's representation. It is built out of +//! `proc_macro2::Ident` and `TokenStream`, which is exactly right for a thing +//! whose job is to become Rust code: an `Ident` carries a span, and a span is +//! what turns a schema mistake into an error pointing at the offending line. +//! +//! It is the wrong representation for everything else. An `Ident` cannot be +//! serialised, cannot be compared across processes, cannot be sent to a +//! designer over a socket or written into a data file, and cannot be +//! constructed at all outside a proc-macro context without a `Span::call_site` +//! that lies about where it came from. A `TokenStream` is not `PartialEq`, so +//! two schemas cannot even be asked whether they differ, which is the one +//! question a migration planner exists to answer. +//! +//! [`Schema`] is the same declaration with the compiler's concerns removed: +//! `String` where the model has `Ident`, ordered `Vec`s where the model has +//! `HashMap`, and no spans. It derives `PartialEq`, so two of them can be +//! diffed, and (under the `serde` feature) `Serialize`, so one can be stored +//! next to the data it describes and read back by a process that has never +//! seen the Rust type. +//! +//! # What it is not +//! +//! Building a `Schema` runs the *parser*, not the *validator*. The rules that +//! reject, say, a `congee` index over a `String` key live in `worktable_codegen` +//! next to the code that would have been generated, because that is where the +//! explanation belongs. A `Schema` can therefore describe a declaration that +//! the macro would refuse to expand. That is deliberate: a designer needs to +//! hold a half-finished schema while the user is still typing it, and a +//! migration planner needs to read an old one whose rules have since changed. +//! +//! # Determinism +//! +//! Every collection here is ordered, and the order is the one written in the +//! declaration. This matters more than it sounds: [`crate::model::Columns`] +//! stores columns in a `HashMap`, whose iteration order Rust randomises per +//! process, so a consumer walking it draws a different diagram on every run. +//! `field_positions` carries the declaration order and is what this sorts by. +//! The query maps have no such field, so those are sorted by name, which is at +//! least stable. + +use proc_macro2::TokenStream; +use syn::spanned::Spanned as _; + +use crate::model::{Columns, GeneratorType, IndexBackend, Persistence, Queries}; +use crate::parser::Parser; + +mod diff; +mod emit_dsl; +#[cfg(feature = "uml")] +mod emit_uml; +mod scan; + +pub use diff::{Change, Cost, Diff, TableChange, TransformReason, TransformRequest, plan}; +#[cfg(feature = "uml")] +pub use emit_uml::{Relation, infer_relations, schemas_to_mermaid}; +pub use scan::{Declarations, declarations_in_source, declarations_in_tokens}; + +/// One `worktable!` declaration, as data. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct Schema { + /// The table name, as written. This is the Rust type name, so it is + /// `UpperCamel` by convention but the parser does not enforce that. + pub name: String, + /// Schema version. Absent in the declaration means 1, and this stores the + /// resolved value rather than the absence, because a consumer comparing an + /// on-disk version against a declared one wants a number either way. + pub version: u32, + /// Whether persistence was selected, and whether it was selected at all. + pub persist: Persistence, + /// The routing key of a partitioned table. Not a column: it is stored once + /// per partition rather than once per row, and no query can name it. + pub partition_by: Option, + /// Columns in declaration order. + pub columns: Vec, + /// Secondary indexes in declaration order. + pub indexes: Vec, + /// Generated queries, sorted by name within each kind. + pub queries: QueriesSpec, + /// The `config` block. + pub config: ConfigSpec, +} + +/// A column declaration: `name: Type [primary_key] [autoincrement|custom] [optional] [using backend]`. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct ColumnSpec { + /// Field name. + pub name: String, + /// The type as written, with any `optional` wrapper removed. The grammar + /// accepts a single identifier here, so this is never a path or a generic. + pub ty: String, + /// Whether `optional` was written, making the field `Option`. + pub optional: bool, + /// Whether this column is part of the primary key. + pub primary_key: bool, + /// The primary-key generator. Only meaningful when `primary_key` is set, + /// and shared by every column of a composite key. + pub generator: GeneratorType, + /// The primary index backend. `Some` on primary-key columns, carrying the + /// declared backend or the default when `using` was omitted; `None` + /// elsewhere, because `using` on a non-key column is a parse error. + pub index_backend: Option, +} + +/// A secondary index declaration: `name: column [unique] [using backend]`. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct IndexSpec { + /// Index name. + pub name: String, + /// The column it is built over. + pub column: String, + /// Whether the index rejects duplicate keys. + pub unique: bool, + /// The physical implementation. + pub backend: IndexBackend, +} + +/// The `partition_by` key. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct PartitionKeySpec { + /// Key name, used for generated argument names. + pub name: String, + /// Unsigned integer type. See [`crate::model::PARTITION_KEY_TYPES`]. + pub ty: String, +} + +/// The `queries` block. +#[derive(Debug, Clone, Default, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serde", serde(default))] +pub struct QueriesSpec { + /// `update:` operations. + pub updates: Vec, + /// `delete:` operations. + pub deletes: Vec, + /// `in_place:` operations. + pub in_place: Vec, +} + +impl QueriesSpec { + /// Whether any query was declared. An empty block and an absent one are + /// the same thing to the macro, so the emitter writes neither. + pub fn is_empty(&self) -> bool { + self.updates.is_empty() && self.deletes.is_empty() && self.in_place.is_empty() + } +} + +/// One generated query: `Name(columns) by key`. +#[derive(Debug, Clone, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct OperationSpec { + /// Query name, which becomes part of the generated method name. + pub name: String, + /// Columns the query touches. Empty for a delete. + pub columns: Vec, + /// The column the query selects rows by. + pub by: String, +} + +/// The `config` block. +#[derive(Debug, Clone, Default, PartialEq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serde", serde(default))] +pub struct ConfigSpec { + /// `page_size`, in bytes. + pub page_size: Option, + /// Extra derives placed on the generated row type. + pub row_derives: Vec, +} + +impl ConfigSpec { + /// Whether anything was configured. + pub fn is_empty(&self) -> bool { + self.page_size.is_none() && self.row_derives.is_empty() + } +} + +impl Schema { + /// Read a declaration from the text between a `worktable!`'s braces. + /// + /// The input is the body only: `name: Foo, columns: { .. }`, without the + /// macro name or the surrounding braces. + pub fn parse(source: &str) -> syn::Result { + let tokens: TokenStream = syn::parse_str(source)?; + Self::from_tokens(tokens) + } + + /// Read a declaration from tokens. + /// + /// This mirrors the macro's own top-level dispatch, including the + /// diagnostics for keywords written in the wrong position, because those + /// are properties of the grammar rather than of code generation. It stops + /// short of the macro's semantic validation: see the module docs. + pub fn from_tokens(input: TokenStream) -> syn::Result { + let mut parser = Parser::new(input); + + let name = parser.parse_name()?; + let version = parser.parse_version()?.unwrap_or(1); + let persist = parser.parse_persist()?; + let partition_by = parser.parse_partition_by()?.map(|key| PartitionKeySpec { + name: key.name.to_string(), + ty: key.ty.to_string(), + }); + + let mut columns: Option = None; + let mut indexes = None; + let mut queries: Option = None; + let mut config = None; + + while let Some(ident) = parser.peek_next() { + match ident.to_string().as_str() { + "columns" => columns = Some(parser.parse_columns()?), + "indexes" => indexes = Some(parser.parse_indexes()?), + "queries" => queries = Some(parser.parse_queries()?), + "config" => config = Some(parser.parse_configs()?), + "version" => { + return Err(syn::Error::new( + ident.span(), + "version must be specified before columns/indexes/queries/config", + )); + } + "persist" | "partition_by" => { + return Err(syn::Error::new( + ident.span(), + "`persist` and `partition_by` are positional; the required order is: \ + name, version, persist, partition_by, then columns/indexes/queries/config", + )); + } + other => { + return Err(syn::Error::new( + ident.span(), + format!( + "Unexpected token `{other}`; expected one of `columns`, `indexes`, `queries`, `config`" + ), + )); + } + } + } + + let mut model = + columns.ok_or_else(|| syn::Error::new(parser.input.span(), "Expected a `columns` block in declaration"))?; + if let Some(indexes) = indexes { + model.indexes = indexes; + } + + Ok(Self { + name: name.to_string(), + version, + persist, + partition_by, + columns: columns_from_model(&model)?, + indexes: indexes_from_model(&model), + queries: queries.map(queries_from_model).unwrap_or_default(), + config: config + .map(|config| ConfigSpec { + page_size: config.page_size, + row_derives: config.row_derives.iter().map(ToString::to_string).collect(), + }) + .unwrap_or_default(), + }) + } + + /// The columns forming the primary key, in declaration order. + pub fn primary_key(&self) -> Vec<&ColumnSpec> { + self.columns.iter().filter(|column| column.primary_key).collect() + } + + /// Look a column up by name. + pub fn column(&self, name: &str) -> Option<&ColumnSpec> { + self.columns.iter().find(|column| column.name == name) + } +} + +fn columns_from_model(model: &Columns) -> syn::Result> { + let mut ordered: Vec<_> = model.field_positions.iter().collect(); + ordered.sort_by_key(|(_, position)| **position); + + ordered + .into_iter() + .map(|(name, _)| { + let ty = model.columns_map.get(name).expect("every positioned column has a type"); + let (ty, optional) = split_optional(ty)?; + let primary_key = model.primary_keys.contains(name); + Ok(ColumnSpec { + name: name.to_string(), + ty, + optional, + primary_key, + generator: if primary_key { + model.generator_type + } else { + GeneratorType::None + }, + index_backend: primary_key.then_some(model.primary_index_backend), + }) + }) + .collect() +} + +/// Recover `optional` from the type the model stores. +/// +/// `Columns::try_from_rows` folds the `optional` keyword into the type, so by +/// the time a column reaches the model there is no flag left to read: the type +/// is literally `core::option::Option`. Going back out means undoing that, +/// and it has to be done on the parsed type rather than on the token text, +/// because `TokenStream::to_string` spaces punctuation in a way that makes +/// string matching a guess. +fn split_optional(ty: &TokenStream) -> syn::Result<(String, bool)> { + let parsed: syn::Type = syn::parse2(ty.clone())?; + let syn::Type::Path(path) = &parsed else { + return Err(syn::Error::new(ty.span(), "Expected a named column type")); + }; + let last = path + .path + .segments + .last() + .ok_or_else(|| syn::Error::new(ty.span(), "Expected a named column type"))?; + + if last.ident == "Option" + && let syn::PathArguments::AngleBracketed(args) = &last.arguments + && let Some(syn::GenericArgument::Type(inner)) = args.args.first() + { + return Ok((type_name(inner)?, true)); + } + + Ok((last.ident.to_string(), false)) +} + +fn type_name(ty: &syn::Type) -> syn::Result { + let syn::Type::Path(path) = ty else { + return Err(syn::Error::new(ty.span(), "Expected a named column type")); + }; + path.path + .segments + .last() + .map(|segment| segment.ident.to_string()) + .ok_or_else(|| syn::Error::new(ty.span(), "Expected a named column type")) +} + +fn indexes_from_model(model: &Columns) -> Vec { + model + .indexes + .values() + .map(|index| IndexSpec { + name: index.name.to_string(), + column: index.field.to_string(), + unique: index.is_unique, + backend: index.backend, + }) + .collect() +} + +fn queries_from_model(queries: Queries) -> QueriesSpec { + fn convert( + operations: std::collections::HashMap, + ) -> Vec { + let mut converted: Vec<_> = operations + .into_values() + .map(|operation| OperationSpec { + name: operation.name.to_string(), + columns: operation.columns.iter().map(ToString::to_string).collect(), + by: operation.by.to_string(), + }) + .collect(); + // The model stores these in a `HashMap`, so this is the only place an + // order can be imposed at all. Sorted by name is not the declaration + // order, but it is the same on every run, which is what a consumer + // rendering them needs. + converted.sort_by(|a, b| a.name.cmp(&b.name)); + converted + } + + QueriesSpec { + updates: convert(queries.updates), + deletes: convert(queries.deletes), + in_place: convert(queries.in_place), + } +} + +/// Whether the declaration selected persistence, for callers that only care +/// about the answer rather than about whether it was written down. +impl Schema { + /// Whether the table persists to disk. + pub fn is_persisted(&self) -> bool { + self.persist.is_persisted() + } +} + +impl Schema { + /// The implementation backing the primary index. + /// + /// Every primary-key column carries the same one: the parser rejects a + /// composite key whose parts disagree. A table with no primary key cannot + /// be declared, so the fallback is unreachable through the parser and is + /// here for a `Schema` built by hand. + pub fn primary_index_backend(&self) -> IndexBackend { + self.columns + .iter() + .find(|column| column.primary_key) + .and_then(|column| column.index_backend) + .unwrap_or_default() + } +} diff --git a/dsl/src/schema/scan.rs b/dsl/src/schema/scan.rs new file mode 100644 index 00000000..4b3546f6 --- /dev/null +++ b/dsl/src/schema/scan.rs @@ -0,0 +1,114 @@ +//! Finding the declarations in Rust source. +//! +//! A designer opening a project, a documentation generator, and a migration +//! tool comparing a checkout against a running database all start from the same +//! problem: a schema is written inside a `worktable!` invocation somewhere in a +//! crate, and there is no index of where. +//! +//! This walks tokens rather than `syn`'s item tree. An invocation inside a +//! function body is not an item, and real code puts them there, so an item walk +//! would quietly miss a table and the caller would never know a table existed to +//! be missed. +//! +//! # What gets set aside +//! +//! Some invocations are not declarations. A `macro_rules!` body writing +//! `name: $name, ... using $backend` is a template: the metavariables stand for +//! text that only exists once the outer macro expands, and no parser for this +//! grammar can accept them. Those are counted rather than reported as errors, +//! because they are not mistakes. +//! +//! Everything else that fails to parse *is* reported, with the text that failed, +//! because a designer that silently drops a table the compiler accepts is worse +//! than one that says it could not read it. + +use proc_macro2::{Delimiter, TokenStream, TokenTree}; + +use super::Schema; + +/// What was found in one piece of source. +#[derive(Debug, Default)] +pub struct Declarations { + /// The schemas, in the order they appear. + pub schemas: Vec, + /// Invocations inside a `macro_rules!` body, which are templates rather + /// than declarations. The text of each, for a caller that wants to say so. + pub templates: Vec, + /// Invocations that did not parse: the text, and why. + pub rejected: Vec<(String, syn::Error)>, +} + +impl Declarations { + /// Whether every invocation found was read as a schema. + pub fn is_complete(&self) -> bool { + self.rejected.is_empty() + } + + /// How many invocations were found, read or not. + pub fn found(&self) -> usize { + self.schemas.len() + self.templates.len() + self.rejected.len() + } +} + +/// Read every `worktable!` declaration in a Rust source file. +/// +/// The error case is the file not tokenising at all, which is a broken file +/// rather than a broken declaration. A declaration that does not parse lands in +/// [`Declarations::rejected`] and does not stop the rest. +pub fn declarations_in_source(source: &str) -> syn::Result { + let tokens: TokenStream = syn::parse_str(source)?; + Ok(declarations_in_tokens(tokens)) +} + +/// Read every `worktable!` declaration in a token stream. +pub fn declarations_in_tokens(tokens: TokenStream) -> Declarations { + let mut bodies = Vec::new(); + collect(tokens, &mut bodies); + + let mut found = Declarations::default(); + for body in bodies { + let text = body.to_string(); + if is_macro_template(&body) { + found.templates.push(text); + continue; + } + match Schema::from_tokens(body) { + Ok(schema) => found.schemas.push(schema), + Err(error) => found.rejected.push((text, error)), + } + } + found +} + +/// Both delimiter forms appear in real code: `worktable! { .. }` and +/// `worktable!( .. )`. Either is accepted, and so is `[ .. ]`, because rustc +/// accepts it and a reader that did not would be wrong about the language. +fn collect(tokens: TokenStream, found: &mut Vec) { + let trees: Vec = tokens.into_iter().collect(); + let mut index = 0; + while index < trees.len() { + if let TokenTree::Ident(ident) = &trees[index] + && ident == "worktable" + && let Some(TokenTree::Punct(bang)) = trees.get(index + 1) + && bang.as_char() == '!' + && let Some(TokenTree::Group(body)) = trees.get(index + 2) + && body.delimiter() != Delimiter::None + { + found.push(body.stream()); + index += 3; + continue; + } + if let TokenTree::Group(group) = &trees[index] { + collect(group.stream(), found); + } + index += 1; + } +} + +fn is_macro_template(tokens: &TokenStream) -> bool { + tokens.clone().into_iter().any(|tree| match tree { + TokenTree::Punct(punct) => punct.as_char() == '$', + TokenTree::Group(group) => is_macro_template(&group.stream()), + _ => false, + }) +} diff --git a/dsl/src/validate.rs b/dsl/src/validate.rs new file mode 100644 index 00000000..dd097c41 --- /dev/null +++ b/dsl/src/validate.rs @@ -0,0 +1,187 @@ +//! The rules a declaration must satisfy beyond being grammatical. +//! +//! These lived in `worktable_codegen`, next to the code they would have +//! generated. That is the right home for the *explanation* and the wrong home +//! for the *check*: a proc-macro crate exports nothing but macros, so the only +//! way to ask "would the macro accept this?" was to expand it. A designer +//! cannot expand a proc macro, and an editor that finds out by compiling is not +//! an editor. +//! +//! They are moved here unchanged, still operating on [`crate::model`] types, +//! which carry `proc_macro2` spans. `worktable_codegen` calls these functions +//! and its diagnostics are identical, down to the span each error points at. +//! Same rule the parser follows: one implementation, two callers, no second +//! copy to drift. +//! +//! [`all`] is the addition. The macro stops at the first error because it will +//! not generate code either way; an editor has the opposite economics, where +//! fix-recompile-find-the-next is the loop a live checker exists to remove. + +use crate::model::{Columns, IndexBackend, Persistence}; + +/// data_bucket's on-disk layer seeks with its own hardcoded `PAGE_SIZE` of +/// 16384 bytes (`seek_to_page_start`, `seek_by_link`, `persist_page`), while +/// the generated table threads the user's `page_size` through its page-id and +/// length arithmetic. Any other value therefore reads and writes the wrong +/// file offsets as soon as the table persists, silently corrupting it. +/// In-memory tables never seek a file: for them `page_size` only sizes index +/// nodes and stays configurable. +const DATA_BUCKET_PAGE_SIZE: u32 = 16384; + +pub fn validate_page_size(config: Option<&crate::model::Config>, persistence: Persistence) -> syn::Result<()> { + let Some(config) = config else { return Ok(()) }; + let Some(page_size) = config.page_size else { + return Ok(()); + }; + if persistence.is_persisted() && page_size != DATA_BUCKET_PAGE_SIZE { + let span = config.page_size_span.unwrap_or_else(proc_macro2::Span::call_site); + return Err(syn::Error::new( + span, + format!( + "`page_size: {page_size}` cannot be combined with `persist: true`: the on-disk \ + layer (data_bucket) hardcodes {DATA_BUCKET_PAGE_SIZE}-byte pages in every file \ + seek, so a persisted table with any other page size reads and writes the wrong \ + pages and corrupts its files. Remove `page_size` (or set it to \ + {DATA_BUCKET_PAGE_SIZE}); custom page sizes remain available for in-memory \ + tables, where they only size index nodes" + ), + )); + } + Ok(()) +} + +/// `in_place` queries hand the caller a mutable reference to the archived +/// column bytes and bypass all index maintenance, so a column that any index +/// is built over cannot be mutated in place: the index would keep resolving +/// the old value. +pub fn validate_in_place_queries(columns: &Columns, queries: &crate::model::Queries) -> syn::Result<()> { + for (name, op) in &queries.in_place { + for column in &op.columns { + if columns.indexes.values().any(|index| &index.field == column) { + return Err(syn::Error::new( + column.span(), + format!( + "in_place query `{name}` mutates column `{column}`, which is covered by an index; \ + indexed columns cannot be updated in place because secondary indexes are not \ + maintained on this path. Use an `update` query instead" + ), + )); + } + } + } + Ok(()) +} + +/// Every backend rule. The `syn::Result` form below is what the macro calls. +/// +/// The three checks here are independent, so a declaration with an unsupported +/// key type *and* a non-unique congee index has two things wrong with it, not +/// one thing and a surprise after the fix. +fn index_backends_into(columns: &Columns, persistence: Persistence, errors: &mut Vec) { + let explicit_backend = if columns.primary_index_backend.requires_explicit_persistence() { + Some(( + columns.primary_index_backend, + columns.primary_keys.first().expect("primary key exists"), + true, + )) + } else { + columns + .indexes + .values() + .find(|index| index.backend.requires_explicit_persistence()) + .map(|index| (index.backend, &index.name, false)) + }; + + if let Some((backend, ident, is_primary)) = explicit_backend { + let kind = if is_primary { "primary index" } else { "index" }; + match persistence { + Persistence::MemoryOnly => {} + Persistence::Omitted => { + errors.push(syn::Error::new( + ident.span(), + format!( + "{kind} `{ident}` uses `{}`, which requires an explicit `persist: true` or `persist: false`", + backend.name() + ), + )); + } + Persistence::Persisted => {} + } + } + + for index in columns.indexes.values().filter(|index| !index.is_unique) { + match index.backend { + IndexBackend::WorktablesIndex | IndexBackend::Arctic => {} + IndexBackend::Indexset | IndexBackend::Congee => { + errors.push(syn::Error::new( + index.name.span(), + format!( + "non-unique index `{}` cannot use `{}`; non-unique indexes currently require \ + `worktables_index` or `arctic`", + index.name, + index.backend.name() + ), + )); + } + } + } + + for (column, index) in &columns.indexes { + let key_type = columns + .columns_map + .get(column) + .expect("an index always references a validated column") + .to_string(); + let supported = match index.backend { + IndexBackend::Congee => Some(&["u8", "u16", "u32", "u64", "usize"][..]), + IndexBackend::Arctic => Some(&["u16", "u32", "u64", "u128"][..]), + IndexBackend::WorktablesIndex | IndexBackend::Indexset => None, + }; + if let Some(supported) = supported + && !supported.contains(&key_type.as_str()) + { + errors.push(syn::Error::new( + index.name.span(), + format!( + "index `{}` uses `{}`, which does not support key type `{key_type}`; supported types: {}", + index.name, + index.backend.name(), + supported.join(", ") + ), + )); + } + } +} + +/// The first backend rule that fails, which is all the macro can act on. +pub fn validate_index_backends(columns: &Columns, persistence: Persistence) -> syn::Result<()> { + let mut errors = Vec::new(); + index_backends_into(columns, persistence, &mut errors); + match errors.into_iter().next() { + Some(error) => Err(error), + None => Ok(()), + } +} + +/// Every rule, collecting all failures rather than stopping at the first. +/// +/// Ordered backends, page size, then in-place queries: the order a reader +/// would work through them. +pub fn all( + columns: &Columns, + queries: Option<&crate::model::Queries>, + config: Option<&crate::model::Config>, + persistence: Persistence, +) -> Vec { + let mut errors = Vec::new(); + index_backends_into(columns, persistence, &mut errors); + if let Err(error) = validate_page_size(config, persistence) { + errors.push(error); + } + if let Some(queries) = queries + && let Err(error) = validate_in_place_queries(columns, queries) + { + errors.push(error); + } + errors +} diff --git a/dsl/tests/check.rs b/dsl/tests/check.rs new file mode 100644 index 00000000..ae391c29 --- /dev/null +++ b/dsl/tests/check.rs @@ -0,0 +1,124 @@ +//! The question `worktable_dsl` could not answer before `check`: would the +//! macro accept this? +//! +//! Every case here was run against the parser before `check` existed. The +//! rule failures came back as a `Schema` that parsed cleanly, because +//! `Schema::parse` runs the parser and not the validator, so a designer had no +//! way to know the declaration would not compile short of compiling it. + +use worktable_dsl::{Stage, check}; + +/// A declaration that is both grammatical and acceptable. +#[test] +fn a_good_declaration_has_nothing_to_report() { + let checked = check("name: Good, columns: { id: u64 primary_key, label: String }"); + + assert!(checked.is_acceptable(), "unexpected: {:?}", checked.diagnostics); + assert_eq!(checked.schema.expect("parsed").name, "Good"); +} + +/// The case the module exists for: parses, would not compile. +/// +/// `Schema::parse` returns `Ok` here, which is correct and is the documented +/// contract: the IR holds declarations the macro refuses, so an editor can +/// render what somebody is halfway through typing. It is also useless on its +/// own, because nothing then says the thing on screen will not build. +#[test] +fn a_rule_failure_still_yields_a_drawable_schema() { + // `persist: false` is required, and stated first: `congee` refuses to be + // used at all until the declaration commits either way, and that rule + // fires before the key type is looked at. Leaving it out tests the + // persistence rule while claiming to test the key-type one. + let checked = check( + "name: Bad, + persist: false, + columns: { id: u64 primary_key, label: String }, + indexes: { label_idx: label unique using congee }", + ); + + assert!(checked.schema.is_some(), "a rule failure must still be drawable"); + assert!(!checked.is_acceptable()); + assert_eq!(checked.diagnostics.len(), 1); + assert_eq!(checked.diagnostics[0].stage, Stage::Rules); + assert!( + checked.diagnostics[0].message.contains("does not support key type"), + "got: {}", + checked.diagnostics[0].message + ); +} + +/// Every problem at once, not the first one. +/// +/// The macro stops at the first because it cannot generate code either way. +/// An editor has the opposite economics: fix, recompile, find the next is the +/// loop a live checker removes. +#[test] +fn every_broken_rule_is_reported() { + let checked = check( + "name: Several, + persist: true, + columns: { id: u64 primary_key, label: String }, + indexes: { label_idx: label unique using congee }, + config: { page_size: 4096 }", + ); + + assert!(checked.schema.is_some()); + assert!( + checked.diagnostics.len() >= 2, + "expected the backend and the page size, got: {:?}", + checked.diagnostics.iter().map(|d| &d.message).collect::>() + ); + assert!( + checked + .diagnostics + .iter() + .any(|d| d.message.contains("does not support key type")) + ); + assert!(checked.diagnostics.iter().any(|d| d.message.contains("page_size"))); +} + +/// A grammar failure says so, and produces nothing. +#[test] +fn a_grammar_failure_is_distinguished_from_a_rule_failure() { + let checked = check("name: Half, columns: { id: u64 primary_key, x: }"); + + assert!(checked.schema.is_none(), "there is no schema to draw"); + assert_eq!(checked.diagnostics.len(), 1); + assert_eq!(checked.diagnostics[0].stage, Stage::Grammar); +} + +/// An unbalanced brace, which is the state a declaration is in for most of the +/// time somebody is typing one, is reported rather than panicking. +#[test] +fn an_unclosed_brace_is_a_diagnostic() { + let checked = check("name: Typing, columns: { id: u64 primary_key"); + + assert!(checked.schema.is_none()); + assert_eq!(checked.diagnostics[0].stage, Stage::Grammar); +} + +/// Without the `spans` feature the location is absent, never wrong. +#[cfg(not(feature = "spans"))] +#[test] +fn a_diagnostic_without_the_spans_feature_carries_no_location() { + let checked = check("name: Bad, columns: { id: u64 primary_key, x: }"); + assert!(checked.diagnostics[0].span.is_none()); +} + +/// With `spans`, the range points at the offending text in the input. +/// +/// Asserted by slicing the source with the range rather than by comparing +/// offsets: an off-by-one in either direction produces a plausible-looking +/// number and a wrong underline, and only the slice catches that. +#[cfg(feature = "spans")] +#[test] +fn a_diagnostic_points_at_the_offending_text() { + let source = "name: Bad, + persist: false, + columns: { id: u64 primary_key, label: String }, + indexes: { label_idx: label unique using congee }"; + let checked = check(source); + + let span = checked.diagnostics[0].span.expect("the spans feature is on"); + assert_eq!(&source[span.start..span.end], "label_idx"); +} diff --git a/dsl/tests/diff.rs b/dsl/tests/diff.rs new file mode 100644 index 00000000..812acba8 --- /dev/null +++ b/dsl/tests/diff.rs @@ -0,0 +1,387 @@ +//! What the migration planner promises, one claim per test. + +use worktable_dsl::{Change, Cost, Diff, Schema, TableChange, TransformReason, plan}; + +fn parse(source: &str) -> Schema { + Schema::parse(source).unwrap_or_else(|error| panic!("{error}\n{source}")) +} + +fn base() -> Schema { + parse( + " + name: User, + version: 1, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + age: u8, + }, + indexes: { email_idx: email unique } + ", + ) +} + +#[test] +fn a_schema_does_not_differ_from_itself() { + let diff = Diff::between(&base(), &base()); + assert!(diff.is_empty()); + assert_eq!(diff.cost(), Cost::Nothing); + assert!(diff.rows_are_readable()); + assert_eq!(diff.describe(), "`User` is unchanged"); +} + +#[test] +fn a_version_bump_on_its_own_costs_nothing() { + // The version is what triggers a migration, not what it costs. A binary + // that bumped the version and changed nothing else has nothing to do, and + // saying so is what keeps the bump cheap enough to be habitual. + let declared = parse( + " + name: User, + version: 2, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + age: u8, + }, + indexes: { email_idx: email unique } + ", + ); + let diff = Diff::between(&base(), &declared); + assert_eq!(diff.changes, vec![Change::Version { from: 1, to: 2 }]); + assert_eq!(diff.cost(), Cost::Nothing); + assert!(diff.rows_are_readable()); +} + +#[test] +fn an_added_index_leaves_the_rows_where_they_are() { + // Every index holds links, and rebuilding one reads rows that have not + // moved. Nothing is invalidated, so this is the cheap kind of change. + let declared = parse( + " + name: User, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + age: u8, + }, + indexes: { email_idx: email unique, age_idx: age } + ", + ); + let diff = Diff::between(&base(), &declared); + assert_eq!(diff.cost(), Cost::RebuildIndexes); + assert!(diff.rows_are_readable()); + assert!(diff.transforms_required().is_empty()); +} + +#[test] +fn an_added_optional_column_rewrites_rows_but_needs_no_decision() { + // The archived layout changes, so every link is invalidated and every row + // is copied forward. There is only one value the new column could hold in + // an existing row, so nobody has to be asked. + let declared = parse( + " + name: User, + version: 2, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + age: u8, + nickname: String optional, + }, + indexes: { email_idx: email unique } + ", + ); + let diff = Diff::between(&base(), &declared); + assert_eq!(diff.cost(), Cost::RewriteRows); + assert!(!diff.rows_are_readable()); + assert!(diff.transforms_required().is_empty()); +} + +#[test] +fn an_added_required_column_has_to_be_asked_about() { + let declared = parse( + " + name: User, + version: 2, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + age: u8, + nickname: String, + }, + indexes: { email_idx: email unique } + ", + ); + let diff = Diff::between(&base(), &declared); + assert_eq!(diff.cost(), Cost::RewriteRows); + let transforms = diff.transforms_required(); + assert_eq!(transforms.len(), 1); + assert_eq!(transforms[0].column, "nickname"); + assert_eq!( + transforms[0].reason, + TransformReason::NoValueToFillItWith { + ty: "String".to_string() + } + ); +} + +#[test] +fn a_type_change_has_to_be_asked_about() { + let declared = parse( + " + name: User, + version: 2, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + age: u32, + }, + indexes: { email_idx: email unique } + ", + ); + let diff = Diff::between(&base(), &declared); + let transforms = diff.transforms_required(); + assert_eq!(transforms.len(), 1); + assert_eq!( + transforms[0].reason, + TransformReason::NoConversionExists { + from: "u8".to_string(), + to: "u32".to_string(), + } + ); +} + +#[test] +fn widening_to_optional_is_decided_but_narrowing_is_not() { + // There is exactly one thing an existing value becomes when a column gains + // `optional`. There is no one thing a stored `None` becomes when it loses + // it, and that is a question about the data rather than about the schema. + let widened = parse( + " + name: User, version: 2, persist: true, + columns: { id: u64 primary_key autoincrement, email: String, age: u8 optional }, + indexes: { email_idx: email unique } + ", + ); + assert!(Diff::between(&base(), &widened).transforms_required().is_empty()); + + let narrowed = Diff::between(&widened, &base()); + assert_eq!(narrowed.transforms_required().len(), 1); + assert_eq!( + narrowed.transforms_required()[0].reason, + TransformReason::NothingToPutWhereNoneWas + ); +} + +#[test] +fn reordering_columns_is_a_layout_change() { + // Declaration order is the generated row struct's field order, so moving a + // column changes the archived layout exactly as changing its type does. + // It is the change most likely to be made by accident and least likely to + // look like one. + let declared = parse( + " + name: User, version: 2, persist: true, + columns: { id: u64 primary_key autoincrement, age: u8, email: String }, + indexes: { email_idx: email unique } + ", + ); + let diff = Diff::between(&base(), &declared); + assert_eq!(diff.cost(), Cost::RewriteRows); + assert!(diff.changes.iter().any(|change| matches!( + change, + Change::ColumnMoved { name, from: 1, to: 2 } if name == "email" + ))); +} + +#[test] +fn a_dropped_column_says_the_data_goes_with_it() { + let declared = parse( + " + name: User, version: 2, persist: true, + columns: { id: u64 primary_key autoincrement, email: String }, + indexes: { email_idx: email unique } + ", + ); + let diff = Diff::between(&base(), &declared); + assert_eq!(diff.cost(), Cost::RewriteRows); + assert!( + diff.warnings() + .iter() + .any(|warning| warning.contains("`age` is dropped")) + ); +} + +#[test] +fn a_renamed_column_reads_as_a_drop_and_an_add() { + // Nothing in a declaration distinguishes a rename from a deletion next to + // an unrelated addition, and guessing by type would be wrong exactly when + // it mattered. The transform is where the intent goes. + let declared = parse( + " + name: User, version: 2, persist: true, + columns: { id: u64 primary_key autoincrement, email_address: String, age: u8 }, + indexes: { email_idx: email_address unique } + ", + ); + let diff = Diff::between(&base(), &declared); + assert!( + diff.changes + .iter() + .any(|c| matches!(c, Change::ColumnAdded(column) if column.name == "email_address")) + ); + assert!( + diff.changes + .iter() + .any(|c| matches!(c, Change::ColumnDropped(column) if column.name == "email")) + ); + assert_eq!(diff.transforms_required().len(), 1); +} + +#[test] +fn a_changed_primary_key_needs_a_person() { + let declared = parse( + " + name: User, version: 2, persist: true, + columns: { id: u64 primary_key, email: String primary_key, age: u8 }, + indexes: { email_idx: email unique } + ", + ); + let diff = Diff::between(&base(), &declared); + assert_eq!(diff.cost(), Cost::NeedsIntent); + assert!(!diff.rows_are_readable()); +} + +#[test] +fn a_changed_partition_key_needs_a_person() { + // The routing key is not in the row, so which partition a row belongs to + // cannot be recomputed from the row: it is only knowable from where the + // row already is. + let stored = parse("name: Price, columns: { id: u64 primary_key, bid: f64 }"); + let declared = + parse("name: Price, version: 2, partition_by: shard: u32, columns: { id: u64 primary_key, bid: f64 }"); + assert_eq!(Diff::between(&stored, &declared).cost(), Cost::NeedsIntent); +} + +#[test] +fn making_an_index_unique_says_it_can_still_fail() { + let stored = parse( + "name: User, persist: true, columns: { id: u64 primary_key, email: String }, indexes: { email_idx: email }", + ); + let declared = parse( + "name: User, version: 2, persist: true, columns: { id: u64 primary_key, email: String }, indexes: { email_idx: email unique }", + ); + let diff = Diff::between(&stored, &declared); + assert_eq!(diff.cost(), Cost::RebuildIndexes); + assert!(diff.warnings().iter().any(|warning| warning.contains("duplicate"))); +} + +#[test] +fn a_schema_change_without_a_version_bump_is_still_visible() { + // This is the middle branch of the load state machine: the versions agree + // and the schemas do not, which is a forgotten bump rather than a + // migration. Catching it costs one comparison of two small structs and no + // row access at all. + let declared = parse( + " + name: User, version: 1, persist: true, + columns: { id: u64 primary_key autoincrement, email: String, age: u32 }, + indexes: { email_idx: email unique } + ", + ); + let diff = Diff::between(&base(), &declared); + assert!( + !diff + .changes + .iter() + .any(|change| matches!(change, Change::Version { .. })) + ); + assert!(!diff.is_empty()); + assert!(!diff.rows_are_readable()); +} + +#[test] +fn queries_and_config_never_reach_the_data() { + let declared = parse( + " + name: User, version: 2, persist: true, + columns: { id: u64 primary_key autoincrement, email: String, age: u8 }, + indexes: { email_idx: email unique }, + queries: { update: { Age(age) by id } }, + config: { row_derives: Clone } + ", + ); + let diff = Diff::between(&base(), &declared); + assert_eq!(diff.cost(), Cost::Nothing); + assert!(diff.rows_are_readable()); + assert!(diff.changes.contains(&Change::QueriesChanged)); + assert!(diff.changes.contains(&Change::ConfigChanged)); +} + +#[test] +fn a_plan_sorts_tables_into_created_changed_and_dropped() { + let stored = vec![ + base(), + parse("name: Legacy, persist: true, columns: { id: u64 primary_key }"), + ]; + let declared = vec![ + parse( + " + name: User, version: 2, persist: true, + columns: { id: u64 primary_key autoincrement, email: String, age: u8, nickname: String optional }, + indexes: { email_idx: email unique } + ", + ), + parse("name: Session, persist: true, columns: { id: u64 primary_key }"), + ]; + + let plan = plan(&stored, &declared); + assert_eq!(plan.len(), 3); + assert!(plan.contains(&TableChange::Created("Session".to_string()))); + assert!(plan.contains(&TableChange::Dropped("Legacy".to_string()))); + assert!(plan.iter().any(|change| matches!( + change, + TableChange::Changed(diff) if diff.table == "User" && diff.cost() == Cost::RewriteRows + ))); +} + +#[test] +fn a_new_table_costs_nothing_and_a_dropped_one_is_never_assumed() { + // A table the binary declares and disk does not is created empty: there is + // nothing to move. A table on disk the binary no longer declares is a + // different matter, because deleting data is a decision rather than a + // consequence of a declaration. + assert_eq!(TableChange::Created("New".to_string()).cost(), Cost::Nothing); + assert_eq!(TableChange::Dropped("Old".to_string()).cost(), Cost::NeedsIntent); +} + +#[test] +fn an_unchanged_table_is_absent_from_the_plan() { + assert!(plan(&[base()], &[base()]).is_empty()); +} + +#[test] +fn the_report_names_the_cost_the_changes_and_what_is_still_needed() { + let declared = parse( + " + name: User, version: 2, persist: true, + columns: { id: u64 primary_key autoincrement, email: String, nickname: String }, + indexes: { email_idx: email unique } + ", + ); + let report = Diff::between(&base(), &declared).describe(); + assert!(report.contains("every row is copied forward")); + assert!(report.contains("version 1 -> 2")); + assert!(report.contains("column added: nickname: String")); + assert!(report.contains("column dropped: age: u8")); + assert!(report.contains("needs a transform written for:")); + assert!(report.contains("nickname: new non-optional column")); + assert!(report.contains("note: column `age` is dropped")); +} diff --git a/dsl/tests/readable_from_outside.rs b/dsl/tests/readable_from_outside.rs new file mode 100644 index 00000000..6886f809 --- /dev/null +++ b/dsl/tests/readable_from_outside.rs @@ -0,0 +1,100 @@ +//! A schema can be read by a crate that is not the macro. +//! +//! This is the whole point of the extraction, so it gets an explicit test +//! rather than trusting that the code moved. Before it, every type here lived +//! in a `proc-macro = true` crate, which can export nothing but macros: the +//! parser existed, understood the grammar exactly as the compiler does, and was +//! unreachable. Anything wanting to read a declaration — a diagram, a migration +//! tool, a documentation generator — had to re-implement the grammar and drift +//! from it. +//! +//! An integration test is the right shape for that claim, because it compiles +//! as a separate crate. If `worktable_dsl` ever became a proc-macro crate +//! again, or stopped exporting these types, this file would fail to build, +//! which is a louder failure than an assertion. + +use worktable_dsl::Parser; + +/// Parse the shape of a real declaration, from outside. +#[test] +fn a_declaration_parses_into_a_model() { + let tokens: proc_macro2::TokenStream = r#" + name: Question, + columns: { + id: String primary_key, + project_id: String, + answered: bool, + } + "# + .parse() + .expect("the fixture is valid tokens"); + + let mut parser = Parser::new(tokens); + + let name = parser.parse_name().expect("a name is declared"); + assert_eq!(name.to_string(), "Question"); + + let columns = parser.parse_columns().expect("columns are declared"); + + let mut declared: Vec = columns.columns_map.keys().map(ToString::to_string).collect(); + declared.sort(); + assert_eq!(declared, ["answered", "id", "project_id"]); + + // The primary key is recognised as one rather than read as part of the + // type, which is the parse most likely to be silently wrong. + assert_eq!( + columns.primary_keys.first().map(ToString::to_string), + Some("id".to_owned()), + "the primary key should be identified: {:?}", + columns.primary_keys + ); +} + +/// Declaration order comes from `field_positions`, never from `columns_map`. +/// +/// `columns_map` is a `std::collections::HashMap`, whose iteration order Rust +/// randomises per process. Running the suite twice produced +/// `["answered", "project_id", "id"]` and then +/// `["project_id", "answered", "id"]` from the same input, so a consumer that +/// iterates it renders a different table on every run. +/// +/// Nothing in this repository had noticed, and nothing needed to: the macro +/// does not care what order it sees columns in, and the parser's own tests +/// collect `columns_map` into another `HashMap` and assert membership. The +/// property was never specified because no caller existed to depend on it. +/// +/// `field_positions` is the answer and is already there — it maps each column +/// to its position in the declaration. A diagram, a documentation page, or an +/// editor should sort by it. This test exists so the next consumer finds that +/// out here rather than by shipping a table that reorders itself. +#[test] +fn declaration_order_is_recovered_from_field_positions() { + let tokens: proc_macro2::TokenStream = r#" + name: Question, + columns: { + id: String primary_key, + project_id: String, + answered: bool, + } + "# + .parse() + .expect("the fixture is valid tokens"); + + let mut parser = Parser::new(tokens); + parser.parse_name().expect("a name is declared"); + let columns = parser.parse_columns().expect("columns are declared"); + + let mut ordered: Vec<(usize, String)> = columns + .field_positions + .iter() + .map(|(name, position)| (*position, name.to_string())) + .collect(); + ordered.sort(); + + let names: Vec = ordered.into_iter().map(|(_, name)| name).collect(); + assert_eq!( + names, + ["id", "project_id", "answered"], + "field_positions should recover the order the columns were declared in" + ); +} diff --git a/dsl/tests/round_trip.rs b/dsl/tests/round_trip.rs new file mode 100644 index 00000000..6fc04d55 --- /dev/null +++ b/dsl/tests/round_trip.rs @@ -0,0 +1,118 @@ +//! The round trip, checked against every schema the project actually declares. +//! +//! A hand-written fixture proves the emitter handles the cases its author +//! thought of. The repository already contains 128 `worktable!` invocations +//! written by people who were not thinking about this crate at all, which is a +//! better corpus than anything written here would be: they use the grammar the +//! way it is really used, including the corners. +//! +//! The property is `parse(emit(parse(source))) == parse(source)`. It is stated +//! on the parsed form rather than the text because the emitter does not +//! reproduce formatting or comments and is not trying to: what has to survive +//! is the meaning. Comparing text would fail on whitespace and would say +//! nothing about whether anything was lost. +//! +//! This runs through `declarations_in_source`, so it is also the evidence that +//! the scanner finds what is there. + +use std::fs; +use std::path::{Path, PathBuf}; + +use worktable_dsl::{Schema, declarations_in_source}; + +fn rust_files(root: &Path, out: &mut Vec) { + let Ok(entries) = fs::read_dir(root) else { return }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + rust_files(&path, out); + } else if path.extension().is_some_and(|extension| extension == "rs") { + out.push(path); + } + } +} + +#[test] +fn every_declaration_in_the_repository_survives_a_round_trip() { + let repository = Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .expect("crate is in the workspace"); + + let mut files = Vec::new(); + for directory in ["src", "tests", "benches", "examples", "codegen"] { + rust_files(&repository.join(directory), &mut files); + } + files.sort(); + assert!(!files.is_empty(), "found no sources to read"); + + let mut checked = 0; + let mut templates = 0; + let mut rejected = Vec::new(); + + for file in &files { + let Ok(contents) = fs::read_to_string(file) else { + continue; + }; + if !contents.contains("worktable!") { + continue; + } + let Ok(found) = declarations_in_source(&contents) else { + continue; + }; + + templates += found.templates.len(); + for (source, error) in found.rejected { + rejected.push(format!(" {}: {error}\n {source}", file.display())); + } + + for schema in found.schemas { + let emitted = schema.to_dsl(); + let reparsed = Schema::parse(&emitted).unwrap_or_else(|error| { + panic!( + "emitted declaration for `{}` from {} does not parse: {error}\n{emitted}", + schema.name, + file.display() + ) + }); + assert_eq!( + schema, + reparsed, + "round trip changed `{}` from {}\n{emitted}", + schema.name, + file.display() + ); + checked += 1; + } + } + + assert!( + rejected.is_empty(), + "{} declaration(s) the parser rejected:\n{}", + rejected.len(), + rejected.join("\n") + ); + assert!(checked >= 100, "only {checked} declarations were checked"); + assert!( + templates >= 12, + "the `macro_rules!` templates stopped being found, so the filter is now hiding \ + something else: {templates}" + ); +} + +#[test] +fn reading_the_same_declaration_twice_gives_the_same_schema() { + // `Columns::columns_map` and the query maps are `HashMap`s, whose iteration + // order Rust randomises per process. Within one process that randomisation + // is fixed, so this catches an ordering mistake only if the schema is built + // from two independently-hashed maps; the ordering guarantee that matters + // across processes is the one `columns_are_in_declaration_order` states. + let source = " + name: Repeatable, + columns: { id: u64 primary_key, a: u64, b: u64, c: String }, + queries: { update: { A(a) by id, B(b) by id, C(c) by id } } + "; + let first = Schema::parse(source).expect("parses"); + let second = Schema::parse(source).expect("parses"); + assert_eq!(first, second); + assert_eq!(first.to_dsl(), second.to_dsl()); +} diff --git a/dsl/tests/schema.rs b/dsl/tests/schema.rs new file mode 100644 index 00000000..1d9c85ec --- /dev/null +++ b/dsl/tests/schema.rs @@ -0,0 +1,200 @@ +//! What the IR and the emitters promise, stated one claim per test. + +use worktable_dsl::{Schema, declarations_in_source}; + +fn parse(source: &str) -> Schema { + Schema::parse(source).unwrap_or_else(|error| panic!("{error}\n{source}")) +} + +#[test] +fn columns_are_in_declaration_order() { + // The model stores columns in a `HashMap`, whose iteration order Rust + // randomises per process: the same input has been observed producing + // `["answered", "project_id", "id"]` and `["project_id", "answered", "id"]` + // on two runs. A consumer walking that draws a different table every time. + // `field_positions` carries the declaration order, and this is the claim + // that the IR sorts by it. + let schema = parse( + " + name: Answer, + columns: { + id: u64 primary_key autoincrement, + project_id: u64, + answered: bool, + } + ", + ); + let names: Vec<&str> = schema.columns.iter().map(|column| column.name.as_str()).collect(); + assert_eq!(names, ["id", "project_id", "answered"]); +} + +#[test] +fn queries_are_sorted_because_the_model_cannot_order_them() { + // Unlike columns, queries have no recorded declaration order to recover: + // the model holds them in a `HashMap` and nothing else. Sorted by name is + // not the order they were written in, but it is the same on every run, + // which is what a consumer rendering them needs. + let schema = parse( + " + name: Sorted, + columns: { id: u64 primary_key, a: u64, b: u64, c: u64 }, + queries: { update: { Charlie(c) by id, Alpha(a) by id, Bravo(b) by id } } + ", + ); + let names: Vec<&str> = schema.queries.updates.iter().map(|q| q.name.as_str()).collect(); + assert_eq!(names, ["Alpha", "Bravo", "Charlie"]); +} + +#[test] +fn optional_is_recovered_from_the_type_the_model_stores() { + // `optional` is not kept as a flag past the parser: `try_from_rows` folds + // it into the type, which becomes `core::option::Option`. Reading it + // back out is the only way the emitter can write the keyword again. + let schema = parse( + " + name: Optionals, + columns: { + id: u64 primary_key, + nickname: String optional, + age: u8, + } + ", + ); + let nickname = schema.column("nickname").expect("declared"); + assert_eq!(nickname.ty, "String"); + assert!(nickname.optional); + assert!(!schema.column("age").expect("declared").optional); + assert!(schema.to_dsl().contains("nickname: String optional,")); +} + +#[test] +fn an_omitted_persist_is_not_written_back() { + // `Omitted` and `MemoryOnly` are different answers. The macro requires an + // explicit `persist: false` before it will accept an index backend that + // cannot persist, so writing one in would answer a question the author + // deliberately left open. + let omitted = parse("name: Omitted, columns: { id: u64 primary_key }"); + assert!(!omitted.to_dsl().contains("persist")); + + let explicit = parse("name: Explicit, persist: false, columns: { id: u64 primary_key }"); + assert!(explicit.to_dsl().contains("persist: false,")); +} + +#[test] +fn only_a_deliberate_backend_choice_is_written_back() { + // A primary key always carries a backend once parsed, because the model + // fills the default in. Emitting `using worktables_index` everywhere would + // round-trip correctly and read like noise. + let default = parse("name: Default, columns: { id: u64 primary_key }"); + assert!(!default.to_dsl().contains("using")); + + let chosen = parse("name: Chosen, persist: false, columns: { id: u64 primary_key using congee }"); + assert!(chosen.to_dsl().contains("id: u64 primary_key using congee,")); +} + +#[test] +fn the_emitted_body_wraps_into_an_invocation() { + let schema = parse("name: Wrapped, columns: { id: u64 primary_key }"); + let invocation = schema.to_macro_invocation(); + assert!(invocation.starts_with("worktable! {\n")); + assert!(invocation.ends_with("}\n")); + assert!(invocation.contains(" name: Wrapped,")); +} + +#[cfg(feature = "serde")] +#[test] +fn a_schema_survives_a_trip_through_serde() { + // This is the property the migration planner rests on: a schema written + // beside the data it describes has to come back the same, in a process + // that never saw the Rust type it was generated from. + let schema = parse( + " + name: Stored, + version: 4, + persist: true, + partition_by: shard: u32, + columns: { + id: u64 primary_key autoincrement using congee, + payload: String optional, + }, + indexes: { payload_idx: payload unique }, + queries: { update: { Payload(payload) by id } }, + config: { page_size: 16384, row_derives: Clone, Debug } + ", + ); + let encoded = serde_json::to_string(&schema).expect("serialises"); + let decoded: Schema = serde_json::from_str(&encoded).expect("deserialises"); + assert_eq!(schema, decoded); + assert_eq!(schema.to_dsl(), decoded.to_dsl()); +} + +#[test] +fn the_scanner_reads_both_delimiter_forms_and_nested_invocations() { + // An invocation inside a function body is not an item, and real code puts + // them there, so an item walk would quietly miss a table. + let source = r#" + worktable!( + name: Braced, + columns: { id: u64 primary_key }, + ); + + mod inner { + worktable! { + name: Nested, + columns: { id: u64 primary_key }, + } + } + + fn in_a_body() { + worktable! { + name: InABody, + columns: { id: u64 primary_key }, + } + } + "#; + + let found = declarations_in_source(source).expect("the source tokenises"); + let names: Vec<&str> = found.schemas.iter().map(|schema| schema.name.as_str()).collect(); + assert_eq!(names, ["Braced", "Nested", "InABody"]); + assert!(found.is_complete()); + assert_eq!(found.found(), 3); +} + +#[test] +fn the_scanner_sets_templates_aside_and_reports_real_failures() { + // A `macro_rules!` body is not a declaration and its metavariables are not + // mistakes. A declaration the compiler would accept but this cannot read is + // a different matter, and dropping it silently would be worse than saying + // so. + let source = r#" + macro_rules! table_for { + ($name:ident, $backend:ident) => { + worktable! { + name: $name, + columns: { id: u64 primary_key using $backend }, + } + }; + } + + worktable! { + name: Broken, + columns: { id: u64 primary_key }, + nonsense: { whatever: 1 }, + } + "#; + + let found = declarations_in_source(source).expect("the source tokenises"); + assert!(found.schemas.is_empty()); + assert_eq!(found.templates.len(), 1); + assert_eq!(found.rejected.len(), 1); + assert!(!found.is_complete()); + // The rejection names the token it tripped on. The old text said + // "Unexpected identifier" for anything at all, including a `,`, which + // named the wrong category of token and sent the reader hunting for a + // misspelled keyword. + let reason = found.rejected[0].1.to_string(); + assert!( + reason.contains("nonsense"), + "the rejection should name the block it did not recognise, got: {reason}" + ); +} diff --git a/dsl/tests/trailing_commas.rs b/dsl/tests/trailing_commas.rs new file mode 100644 index 00000000..e6f27674 --- /dev/null +++ b/dsl/tests/trailing_commas.rs @@ -0,0 +1,95 @@ +//! Every block accepts a trailing comma, and every block accepts its absence. +//! +//! Three of the six block parsers consumed the comma that may follow their +//! block and three did not, so `config: { .. },` reached the top-level +//! dispatch as a `,` token and died as "Unexpected identifier". `config`, +//! `delete` and `in_place` happen to be written last in every declaration in +//! this repository, which is the only reason it had not been hit. +//! +//! It matters for the designer specifically: the emitter writes a declaration +//! and the parser reads it back, so an asymmetry here is a round trip that +//! fails on the tool's own output the moment block order changes. +//! +//! Each case below fails on the pre-fix parser. That was checked by reverting +//! the three `try_parse_comma()` calls and watching this file go red, rather +//! than by assuming a new test tests something. + +use worktable_dsl::Schema; + +/// The block that must not be last: everything after `config` was unreachable. +#[test] +fn a_comma_after_config_is_accepted() { + let schema = Schema::parse( + "name: Trailing, + columns: { id: u64 primary_key, payload: String }, + config: { page_size: 4096 },", + ) + .expect("a comma after the `config` block is a comma, not an identifier"); + + assert_eq!(schema.name, "Trailing"); + assert_eq!(schema.config.page_size, Some(4096)); +} + +/// `config` written before `queries`, which the comma asymmetry forbade. +#[test] +fn config_does_not_have_to_be_written_last() { + let schema = Schema::parse( + "name: Ordered, + columns: { id: u64 primary_key, name: String }, + config: { page_size: 8192 }, + queries: { update: { Renamed(name) by id, } }", + ) + .expect("block order should not depend on which parser eats a comma"); + + assert_eq!(schema.config.page_size, Some(8192)); + assert_eq!(schema.queries.updates.len(), 1); +} + +/// The same asymmetry inside `queries`, where `delete` and `in_place` sat. +#[test] +fn a_comma_after_delete_or_in_place_is_accepted() { + let schema = Schema::parse( + "name: Inner, + columns: { id: u64 primary_key, name: String }, + queries: { + delete: { ByName() by name, }, + in_place: { SetName(name) by id, }, + update: { Renamed(name) by id, } + }", + ) + .expect("`delete` and `in_place` should not have to be written last either"); + + assert_eq!(schema.queries.deletes.len(), 1); + assert_eq!(schema.queries.in_place.len(), 1); + assert_eq!(schema.queries.updates.len(), 1); +} + +/// Omitting the comma stays valid. The fix is permissive, not a new rule. +#[test] +fn omitting_the_comma_is_still_accepted() { + let schema = Schema::parse( + "name: NoComma, + columns: { id: u64 primary_key }, + config: { page_size: 4096 }", + ) + .expect("the form the emitter writes must keep parsing"); + + assert_eq!(schema.config.page_size, Some(4096)); +} + +/// A genuinely unexpected token names itself now. +/// +/// The old message said "Unexpected identifier" for a `,`, which is what made +/// this class of bug cost an afternoon: the text names the wrong category of +/// token and sends you looking for a misspelled keyword. +#[test] +fn an_unexpected_token_is_named() { + let error = + Schema::parse("name: Bad, columns: { id: u64 primary_key }, wat: { x: 1 }").expect_err("`wat` is not a block"); + + let message = error.to_string(); + assert!( + message.contains("wat"), + "the error should name the token it saw, got: {message}" + ); +} diff --git a/dsl/tests/uml.rs b/dsl/tests/uml.rs new file mode 100644 index 00000000..b584e3ad --- /dev/null +++ b/dsl/tests/uml.rs @@ -0,0 +1,144 @@ +//! The Mermaid emitter and the relation guessing it rests on. +//! +//! Separate from `schema.rs` because this is the one part of `worktable_dsl` +//! that is not about the `worktable!` grammar. The language has no foreign +//! keys, so `infer_relations` guesses links from a naming convention that +//! WorkTable does not enforce and has no opinion about; that mapping belongs +//! to whatever application draws the diagram. Hence the `uml` feature, and +//! hence these tests living behind it rather than beside the ones that test +//! what the compiler actually enforces. +#![cfg(feature = "uml")] + +use worktable_dsl::{Schema, infer_relations, schemas_to_mermaid}; + +fn parse(source: &str) -> Schema { + Schema::parse(source).unwrap_or_else(|error| panic!("{error}\n{source}")) +} +#[test] +fn mermaid_marks_the_key_the_generator_and_the_indexes() { + let schema = parse( + " + name: Account, + version: 3, + persist: true, + columns: { + id: u64 primary_key autoincrement, + email: String, + tenant: u64, + nickname: String optional, + }, + indexes: { + email_idx: email unique, + tenant_idx: tenant, + } + ", + ); + let diagram = schema.to_mermaid(); + + assert!(diagram.starts_with("classDiagram\n")); + assert!(diagram.contains("class Account {")); + assert!(diagram.contains("<>")); + assert!(diagram.contains("+id : u64 [PK, autoincrement]")); + assert!(diagram.contains("+email : String [UK email_idx]")); + assert!(diagram.contains("+tenant : u64 [IX tenant_idx]")); + // Mermaid spells a generic with tildes. + assert!(diagram.contains("+nickname : Option~String~")); +} + +#[test] +fn mermaid_draws_queries_as_operations() { + let schema = parse( + " + name: Ledger, + columns: { id: u64 primary_key, balance: f64, note: String }, + queries: { + update: { Balance(balance) by id } + delete: { ById() by id } + } + ", + ); + let diagram = schema.to_mermaid(); + assert!(diagram.contains("+update_Balance(balance) by_id")); + assert!(diagram.contains("+delete_ById() by_id")); +} + +#[test] +fn mermaid_puts_the_partition_key_in_a_note_not_a_column() { + // The routing key is stored once per partition rather than once per row, + // and no query can name it, so drawing it as an attribute would be a lie + // about where the data lives. + let schema = parse( + " + name: Price, + partition_by: symbol_id: u16, + columns: { exchange_id: u8 primary_key, bid: f64 } + ", + ); + let diagram = schema.to_mermaid(); + assert!(diagram.contains("note for Price \"partitioned by symbol_id: u16\"")); + assert!(!diagram.contains("symbol_id : u16")); +} + +fn related() -> Vec { + vec![ + parse("name: Project, columns: { id: u64 primary_key autoincrement, title: String }"), + parse( + " + name: Answer, + columns: { + id: u64 primary_key autoincrement, + project_id: u64, + body: String, + } + ", + ), + ] +} + +#[test] +fn a_reference_is_inferred_from_the_naming_convention() { + let relations = infer_relations(&related()); + assert_eq!(relations.len(), 1); + assert_eq!(relations[0].from, "Answer"); + assert_eq!(relations[0].column, "project_id"); + assert_eq!(relations[0].to, "Project"); + assert_eq!(relations[0].to_column, "id"); +} + +#[test] +fn an_inferred_reference_is_drawn_as_a_dependency() { + // Dashed, because the declaration does not say this. A solid association + // would claim the schema language has foreign keys, and it does not. + let diagram = schemas_to_mermaid(&related()); + assert!(diagram.contains("Answer ..> Project : project_id")); +} + +#[test] +fn a_name_collision_on_a_different_type_is_not_a_reference() { + let schemas = vec![ + parse("name: Project, columns: { id: u64 primary_key, title: String }"), + parse("name: Answer, columns: { id: u64 primary_key, project_id: String }"), + ]; + assert!(infer_relations(&schemas).is_empty()); +} + +#[test] +fn a_composite_key_is_not_guessed_at() { + // There is no single column to point the arrow at, and picking one part of + // the key would be worse than drawing nothing. + let schemas = vec![ + parse("name: Project, columns: { tenant_id: u64 primary_key, id: u64 primary_key }"), + parse("name: Answer, columns: { id: u64 primary_key, project_id: u64 }"), + ]; + assert!(infer_relations(&schemas).is_empty()); +} + +#[test] +fn a_key_column_is_not_read_as_a_reference() { + // `project_id` here is half of this table's own identity, not a link out. + let schemas = vec![ + parse("name: Project, columns: { id: u64 primary_key, title: String }"), + parse("name: Answer, columns: { project_id: u64 primary_key, seq: u64 primary_key }"), + ]; + assert!(infer_relations(&schemas).is_empty()); +} diff --git a/src/in_memory/pages.rs b/src/in_memory/pages.rs index 80c36805..bdbd58bd 100644 --- a/src/in_memory/pages.rs +++ b/src/in_memory/pages.rs @@ -315,19 +315,44 @@ where /// thread can later collect it; it executes only after every reader /// pinned right now has unpinned. fn retire(&self, item: Retired) { + self.retire_many(std::iter::once(item)); + } + + /// Queue several retired items behind one grace marker. + /// + /// Retiring `n` items one at a time takes `n` domain advances, and an + /// advance is the expensive half: it is the only operation here that has to + /// decide what every reader can still reach. Retiring a batch behind a + /// single marker is the same guarantee, because the marker is stamped after + /// the whole batch is queued and therefore covers all of it. + /// + /// The lock is taken once for the batch rather than once per item, which + /// also stops a bulk delete interleaving its queue pushes with concurrent + /// mutations for no reason. + fn retire_many(&self, items: impl IntoIterator>) { + let mut queued = 0usize; let len = { let mut retired = self.retired.lock(); - retired.push_back(item); + for item in items { + retired.push_back(item); + queued += 1; + } retired.len() }; - self.pending_retirements.fetch_add(1, Ordering::Release); + if queued == 0 { + return; + } + self.pending_retirements.fetch_add(queued, Ordering::Release); if len >= RETIREMENT_BACKLOG_WARN_AT && len.is_power_of_two() { tracing::warn!(len, "versioned publication retirement backlog is growing"); } let reclaimable = Arc::clone(&self.reclaimable); let guard = self.epoch.pin(); + // One marker for the whole batch. It is stamped now, so it expires only + // after every reader pinned now has unpinned, which is exactly the + // condition each item would have waited for individually. self.epoch.retire(move || { - reclaimable.fetch_add(1, Ordering::Release); + reclaimable.fetch_add(queued, Ordering::Release); }); drop(guard); self.epoch.advance(); @@ -849,6 +874,60 @@ where Ok(()) } + /// Ghost every link in `links`, behind one grace marker. + /// + /// Same per-row effect as calling [`Self::delete`] in a loop: each row is + /// marked deleted in place and its link is queued for reuse once no reader + /// can still reach it. The difference is that the batch takes one domain + /// advance and one reclaim pass instead of one of each per row, and an + /// advance is the expensive half of a retirement. + /// + /// Ghosting is done first, for all links, and the batch is retired only + /// after. A link must not become reusable while a later row in the same + /// batch is still being marked, or a concurrent insert could claim it and + /// be ghosted by this call. + /// + /// On error the links ghosted so far are still retired: they are genuinely + /// deleted, and dropping them from the queue would leak their storage for + /// the life of the table. The caller learns which link failed and how many + /// preceded it. + pub fn delete_many(&self, links: &[Link]) -> Result<(), ExecutionError> + where + Row: Archive + for<'a> Serialize, Share>, rkyv::rancor::Error>>, + ::WrappedRow: + Archive + for<'a> Serialize, Share>, rkyv::rancor::Error>>, + <::WrappedRow as Archive>::Archived: ArchivedRowWrapper + + Portable + + Deserialize<::WrappedRow, HighDeserializer>, + { + if links.is_empty() { + return Ok(()); + } + + let mut ghosted = 0usize; + let mut failure = None; + for link in links { + match unsafe { self.with_mut_ref(*link, |r| r.delete()) } { + Ok(()) => ghosted += 1, + Err(error) => { + failure = Some(error); + break; + } + } + } + + if ghosted > 0 { + self.row_count.fetch_sub(ghosted as u64, Ordering::Relaxed); + self.retire_many(links[..ghosted].iter().map(|link| Retired::Link(*link))); + self.reclaim_retired(); + } + + match failure { + Some(error) => Err(error), + None => Ok(()), + } + } + pub fn select_raw(&self, link: Link) -> Result, ExecutionError> { let pages = self.pages.read(); let page = pages diff --git a/src/lib.rs b/src/lib.rs index 021fa5d2..71d37247 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -52,12 +52,12 @@ pub mod prelude { pub use crate::table::system_info::{IndexInfo, IndexKind, SystemInfo}; pub use crate::util::{OffsetEqLink, OrderedF32Def, OrderedF64Def}; pub use crate::{ - ArcticIndex, ArcticKey, ArcticMultiIndex, AvailableIndex, BatchInsertError, CongeeIndex, CongeeKey, Difference, - IndexError, IndexMap, IndexMultiMap, MultiPairRecreate, PersistentArcticIndex, PersistentArcticMultiIndex, - PersistentArtIndex, PersistentCongeeIndex, PersistentWtiIndex, PrimaryIndex, TableIndex, TableIndexCdc, - TableRow, TableSecondaryIndex, TableSecondaryIndexCdc, TableSecondaryIndexEventsOps, TableSecondaryIndexInfo, - UniqueIndex, UnsizedNode, UpstreamIndexMap, UpstreamIndexPair, WorkTable, WorkTableError, - vacuum::EmptyDataVacuum, vacuum::VacuumPersistence, vacuum::WorkTableVacuum, + ArcticIndex, ArcticKey, ArcticMultiIndex, AvailableIndex, BatchDeleteError, BatchInsertError, CongeeIndex, + CongeeKey, Difference, IndexError, IndexMap, IndexMultiMap, MultiPairRecreate, PersistentArcticIndex, + PersistentArcticMultiIndex, PersistentArtIndex, PersistentCongeeIndex, PersistentWtiIndex, PrimaryIndex, + TableIndex, TableIndexCdc, TableRow, TableSecondaryIndex, TableSecondaryIndexCdc, TableSecondaryIndexEventsOps, + TableSecondaryIndexInfo, UniqueIndex, UnsizedNode, UpstreamIndexMap, UpstreamIndexPair, WorkTable, + WorkTableError, vacuum::EmptyDataVacuum, vacuum::VacuumPersistence, vacuum::WorkTableVacuum, }; pub use data_bucket::{ DATA_VERSION, DataPage, GENERAL_HEADER_SIZE, GeneralHeader, GeneralPage, INNER_PAGE_SIZE, IndexPage, Interval, diff --git a/src/partition/mod.rs b/src/partition/mod.rs index 7f52c705..306fa9d7 100644 --- a/src/partition/mod.rs +++ b/src/partition/mod.rs @@ -292,13 +292,37 @@ impl PartitionSet { /// /// So a tick loop should not pin per lookup. Pin once, read many: /// - /// ```ignore + /// ``` + /// use worktable::prelude::*; + /// use worktable::worktable; + /// + /// worktable!( + /// name: Price, + /// partition_by: symbol_id: u16, + /// columns: { + /// exchange_id: u8 primary_key, + /// bid: f64 + /// } + /// ); + /// + /// let prices = PricePartitions::new(); + /// for symbol in [7u16, 9, 11] { + /// prices + /// .partition_or_create(symbol) + /// .unwrap() + /// .insert(PriceRow { exchange_id: 1, bid: symbol as f64 }) + /// .unwrap(); + /// } + /// + /// // One pin for the whole batch, then three dependent loads per lookup. /// let pinned = prices.pinned(); - /// for tick in batch { - /// if let Some(book) = pinned.get(tick.symbol_id) { - /// book.insert(tick.into())?; + /// let mut total = 0.0; + /// for symbol in [7u16, 9, 11] { + /// if let Some(book) = pinned.get(symbol) { + /// total += book.select(1).unwrap().bid; /// } /// } + /// assert_eq!(total, 27.0); /// ``` /// /// The pin is held for the whole scope, so nothing retired during it is diff --git a/src/table/mod.rs b/src/table/mod.rs index 7f290da9..d93cb54c 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -290,6 +290,184 @@ where Ok(pk) } + /// Deletes every row named by `pks`, ghosting them behind one grace marker. + /// + /// Deleting is already a bit flip rather than a move: the row is marked + /// deleted in place, its index entries are removed, and its storage becomes + /// reusable once no reader can still reach it. Vacuum is what later + /// compacts the pages and hands whole ones back. This batches that, and the + /// batching is worth having for one specific reason: the per-row cost is + /// dominated by the domain advance each retirement takes, not by the bit + /// flip, so `n` deletes cost `n` advances where a batch costs one. + /// + /// Unlike [`Self::insert_many`] this is **not** all-or-nothing, and the + /// difference is deliberate. A rejected insert has published nothing, so + /// unwinding restores a state that was real. A delete that fails partway + /// has already removed index entries and ghosted rows, and those rows are + /// genuinely gone; resurrecting them would mean re-publishing index entries + /// for storage that is queued for reuse. So the batch reports what it + /// deleted and stops at the first failure, rather than pretending it can + /// rewind. + /// + /// A primary key that is not present is skipped rather than failing the + /// batch. Callers evicting a generation do not generally know which of its + /// keys a concurrent writer has already removed, and making them find out + /// first would be a race they cannot win. + /// + /// Returns the keys actually deleted, in the order given. + pub fn delete_many(&self, pks: Vec) -> Result, BatchDeleteError> + where + Row: Archive + + Clone + + for<'a> Serialize, Share>, rkyv::rancor::Error>>, + ::WrappedRow: + Archive + for<'a> Serialize, Share>, rkyv::rancor::Error>>, + <::WrappedRow as Archive>::Archived: ArchivedRowWrapper + + Portable + + Deserialize<::WrappedRow, HighDeserializer>, + PrimaryKey: Clone, + AvailableTypes: 'static, + AvailableIndexes: AvailableIndex, + SecondaryIndexes: TableSecondaryIndex, + LockType: 'static, + { + if pks.is_empty() { + return Ok(Vec::new()); + } + // Stripe-ordered, exactly as `insert_many` takes them, so a batch + // delete and a batch insert cannot deadlock against each other. + let _mutation_guards = self.lock_manager.mutation_guards(pks.iter()); + + let mut deleted: Vec = Vec::with_capacity(pks.len()); + let mut links: Vec = Vec::with_capacity(pks.len()); + + for pk in &pks { + let Some(link) = self.primary_index.pk_map.get_value(pk).map(Into::into) else { + // Already gone. Not an error: see above. + continue; + }; + // Read at the link rather than by key: the link is already in + // hand, and `select` would spend a second primary-key lookup to + // find it again. `select_non_ghosted` keeps the check that matters, + // which is that low-level staged or hydrated state can publish + // index reachability before clearing a row's ghost bit. + let Ok(row) = self.data.select_non_ghosted(link) else { + continue; + }; + + // Index removals run BEFORE the rows are ghosted, and for the same + // reason the single-row path gives: insert publishes data first and + // indexes second, so tearing down in the reverse order guarantees + // no index entry ever resolves to storage that has been freed or + // reused. + if let Err(source) = self.indexes.delete_row(row, link) { + return Err(BatchDeleteError::Key { + key: pk.clone(), + deleted: deleted.len(), + source: WorkTableError::from(source), + }); + } + self.primary_index.remove(pk, link); + links.push(link); + deleted.push(pk.clone()); + } + + // One ghosting pass, one grace marker, one reclaim. + if let Err(source) = self.data.delete_many(&links) { + return Err(BatchDeleteError::Table(WorkTableError::PagesError(source))); + } + + Ok(deleted) + } + + /// Deletes every row whose primary key falls in `range`. + /// + /// The shape bulk eviction actually has. A caller dropping a generation + /// knows the span it wants gone, not the individual keys, and making them + /// enumerate the span first means walking the primary index by hand and + /// then handing the result straight back. + /// + /// The span is collected from the primary index in one ordered walk, and + /// the delete then runs exactly as [`Self::delete_many`]: keys are still + /// resolved under their mutation guards, because the set can change between + /// the walk and the delete and a key that has since gone is skipped rather + /// than failing the batch. So this is one walk instead of the caller's, not + /// a way to skip the per-key work that keeps the delete correct. + /// + /// Returns the keys actually deleted, in key order. + pub fn delete_range(&self, range: R) -> Result, BatchDeleteError> + where + R: std::ops::RangeBounds, + Row: Archive + + Clone + + for<'a> Serialize, Share>, rkyv::rancor::Error>>, + ::WrappedRow: + Archive + for<'a> Serialize, Share>, rkyv::rancor::Error>>, + <::WrappedRow as Archive>::Archived: ArchivedRowWrapper + + Portable + + Deserialize<::WrappedRow, HighDeserializer>, + PrimaryKey: Clone, + AvailableTypes: 'static, + AvailableIndexes: AvailableIndex, + SecondaryIndexes: TableSecondaryIndex, + LockType: 'static, + { + // Owned bounds, because the span is walked twice and `R` is consumed. + let start = range.start_bound().cloned(); + let end = range.end_bound().cloned(); + + // First walk: learn which keys the span holds. Nothing is trusted from + // this pass except the key set, which is what the guards are for. + let keys: Vec = self + .primary_index + .pk_map + .range_values((start.clone(), end.clone())) + .map(|(key, _)| key) + .collect(); + if keys.is_empty() { + return Ok(Vec::new()); + } + let _mutation_guards = self.lock_manager.mutation_guards(keys.iter()); + + // Second walk, under the guards, so these links cannot move and can be + // used directly. This is the whole point: `k` individual lookups cost + // `k` times `O(log n)` in table size, one walk costs `O(log n + k)`, so + // the saving grows with both the batch and the table. A first version + // walked and then looked every key up again, which is strictly more + // work than `delete_many` and gave this no reason to exist. + let pinned: Vec<(PrimaryKey, Link)> = self + .primary_index + .pk_map + .range_values((start, end)) + .map(|(key, link)| (key, link.into())) + .collect(); + + let mut deleted: Vec = Vec::with_capacity(pinned.len()); + let mut links: Vec = Vec::with_capacity(pinned.len()); + for (key, link) in pinned { + // Read at the link, as above: the walk already produced it. + let Ok(row) = self.data.select_non_ghosted(link) else { + continue; + }; + if let Err(source) = self.indexes.delete_row(row, link) { + return Err(BatchDeleteError::Key { + key, + deleted: deleted.len(), + source: WorkTableError::from(source), + }); + } + self.primary_index.remove(&key, link); + links.push(link); + deleted.push(key); + } + + if let Err(source) = self.data.delete_many(&links) { + return Err(BatchDeleteError::Table(WorkTableError::PagesError(source))); + } + + Ok(deleted) + } + /// Inserts every row of `rows`, all or nothing. /// /// Rows are first staged ghosted (invisible to lock-free readers), every @@ -1051,6 +1229,28 @@ pub enum BatchInsertError { Table(WorkTableError), } +/// Why a [`WorkTable::delete_many`] stopped. +/// +/// It carries how many keys were deleted before the failure, because a bulk +/// delete does not roll back: those rows are gone, and a caller retrying the +/// batch needs to know the prefix already succeeded rather than assume nothing +/// happened. +#[derive(Debug, Display, Error)] +pub enum BatchDeleteError { + /// One key could not be deleted. Everything before it was. + #[display("batch delete stopped at {key:?} after {deleted} deleted: {source}")] + Key { + /// The key that failed. + key: PrimaryKey, + /// How many keys were deleted before this one. + deleted: usize, + source: WorkTableError, + }, + /// The batch failed for a reason not attributable to a single key. + #[display("{_0}")] + Table(WorkTableError), +} + #[derive(Debug, Display, Error, From)] pub enum WorkTableError { NotFound, diff --git a/tests/worktable/concurrency.rs b/tests/worktable/concurrency.rs new file mode 100644 index 00000000..ac917074 --- /dev/null +++ b/tests/worktable/concurrency.rs @@ -0,0 +1,426 @@ +//! Concurrent writes, across every index backend, above the writer count the +//! rest of the suite reaches. +//! +//! Concurrency coverage existed before this file but was scattered and +//! backend-specific: `nonunique_arctic` races four writers against an arctic +//! non-unique index, `index_backends` recovers concurrent same-row updates, +//! `vacuum` runs a vacuum thread beside sequential inserts. Nothing raced +//! writers across all three backends, and **nothing went above four writers**. +//! +//! That mattered. Insert throughput on this engine is flat to four writers and +//! collapses at eight, so four is precisely the last thread count at which +//! everything looks fine. See `insert_throughput_should_scale_past_four_writers` +//! at the end of this file. +//! +//! Congee appears only where a unique index is enough: it has no non-unique +//! backend, and `worktable_codegen` rejects the declaration rather than letting +//! it fail later. + +/// Shape of the concurrent workload, tunable without editing the file. +/// +/// Hardcoding a writer count is what let this whole class of problem hide: the +/// suite stopped at four writers, four is the last count at which this engine +/// still behaves, and nobody could turn the dial without a recompile. Every +/// number below is an environment variable with a default sized for CI. +/// +/// ```sh +/// WT_CONC_WRITERS=32 WT_CONC_PER_WRITER=2000 cargo test --test mod concurrency +/// WT_CONC_SWEEP=1,2,4,8,16,32,64 cargo test --test mod insert_throughput -- --ignored --nocapture +/// ``` +mod params { + /// Reads an integer from the environment, falling back to `default`. + /// + /// A malformed value is a hard error rather than a silent fallback: a typo + /// in `WT_CONC_WRITERS` that quietly runs the default is a run you believe + /// tested something it did not. + pub fn env_u64(name: &str, default: u64) -> u64 { + match std::env::var(name) { + Ok(raw) => raw + .trim() + .parse() + .unwrap_or_else(|_| panic!("{name} must be an integer, got {raw:?}")), + Err(_) => default, + } + } + + pub fn env_f64(name: &str, default: f64) -> f64 { + match std::env::var(name) { + Ok(raw) => raw + .trim() + .parse() + .unwrap_or_else(|_| panic!("{name} must be a number, got {raw:?}")), + Err(_) => default, + } + } + + /// Concurrent writers. Defaults to eight because four is where the rest of + /// the suite stops and where this engine still looks healthy. + pub fn writers() -> u64 { + env_u64("WT_CONC_WRITERS", 8) + } + + /// Rows each writer inserts. + pub fn per_writer() -> u64 { + env_u64("WT_CONC_PER_WRITER", 500) + } + + /// Rows seeded before a race that also deletes. + pub fn seed_rows() -> u64 { + env_u64("WT_CONC_SEED_ROWS", 2_000) + } + + /// Reader threads running beside the writers. + pub fn readers() -> u64 { + env_u64("WT_CONC_READERS", 4) + } + + /// Rows inserted per arm of the throughput sweep. + pub fn scale_rows() -> u64 { + env_u64("WT_SCALE_ROWS", 200_000) + } + + /// Writer counts the throughput sweep visits. + pub fn scale_sweep() -> Vec { + match std::env::var("WT_SCALE_SWEEP") { + Ok(raw) => raw + .split(',') + .map(|part| { + part.trim() + .parse() + .unwrap_or_else(|_| panic!("WT_SCALE_SWEEP must be comma-separated integers, got {raw:?}")) + }) + .collect(), + Err(_) => vec![2, 4, 8, 16], + } + } + + /// Share of single-writer throughput a run must keep. + pub fn scale_floor() -> f64 { + env_f64("WT_SCALE_FLOOR", 0.6) + } + + /// Whether this is a release build. + /// + /// A function rather than `cfg!(..)` inline, so the assertion that uses it + /// is not a compile-time constant. `assert!(!cfg!(debug_assertions))` is + /// rejected by clippy as a constant assertion, and `#[cfg] panic!` makes + /// the rest of the function unreachable and its imports unused. This keeps + /// one code path in both profiles. + pub fn is_release_build() -> bool { + !cfg!(debug_assertions) + } +} + +use std::collections::HashMap; +use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::Instant; + +use worktable::prelude::*; +use worktable::worktable; + +/// One table per backend. Separate modules because the generated idents would +/// otherwise collide, and a macro because three hand-written copies drift. +macro_rules! backend_suite { + ($module:ident, $backend:ident, $label:literal) => { + mod $module { + use super::*; + + worktable!( + name: Conc, + persist: false, + columns: { + id: u64 primary_key, + payload: u64, + bucket: u32, + }, + indexes: { + payload_idx: payload unique using $backend, + bucket_idx: bucket using worktables_index, + }, + ); + + fn row(id: u64) -> ConcRow { + ConcRow { id, payload: 1_000_000 + id, bucket: (id % 16) as u32 } + } + + /// Eight writers on disjoint key ranges: every row lands, exactly + /// once, reachable through both the unique and the non-unique + /// index. + /// + /// Eight rather than four on purpose. Four is where the existing + /// tests stop and where this engine still behaves; the interesting + /// interleavings start above it. + #[test] + fn eight_writers_lose_no_rows() { + let (writers, per_writer) = (params::writers(), params::per_writer()); + + let table = Arc::new(ConcWorkTable::default()); + std::thread::scope(|scope| { + for w in 0..writers { + let table = Arc::clone(&table); + scope.spawn(move || { + for n in 0..per_writer { + let id = w * per_writer + n; + table.insert(row(id)).expect("insert"); + // Read while the others write, so the scan and + // the mutations actually overlap. + let _ = table.select(id); + } + }); + } + }); + + assert_eq!(table.count(), (writers * per_writer) as usize, "{} lost rows", $label); + for id in 0..(writers * per_writer) { + assert!(table.select(id).is_some(), "{}: row {id} missing by primary key", $label); + assert!( + table.select_by_payload(1_000_000 + id).is_some(), + "{}: row {id} missing from the unique index", + $label + ); + } + } + + /// Writers inserting while other threads delete, so inserts race + /// storage reuse rather than only each other. + /// + /// A link freed by a delete becomes reusable once no reader can + /// reach it, so this is the interleaving where an insert can claim + /// a slot another thread is still finishing with. + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn inserts_racing_deletes_leave_a_consistent_index() { + let seed = params::seed_rows(); + let (writers, per_writer) = (params::writers(), params::per_writer()); + + let table = Arc::new(ConcWorkTable::default()); + for id in 0..seed { + table.insert(row(id)).expect("seed"); + } + + let deleter = { + let table = Arc::clone(&table); + tokio::spawn(async move { + for id in 0..seed { + table.delete(id).await.expect("delete"); + } + }) + }; + + std::thread::scope(|scope| { + for w in 0..writers { + let table = Arc::clone(&table); + scope.spawn(move || { + for n in 0..per_writer { + let id = seed + w * per_writer + n; + table.insert(row(id)).expect("insert"); + } + }); + } + }); + deleter.await.expect("deleter did not panic"); + + // Every inserted row survived, and nothing the deleter removed + // came back through an index. + for w in 0..writers { + for n in 0..per_writer { + let id = seed + w * per_writer + n; + assert!(table.select(id).is_some(), "{}: inserted row {id} lost", $label); + } + } + for id in 0..seed { + assert!(table.select(id).is_none(), "{}: deleted row {id} still readable", $label); + assert!( + table.select_by_payload(1_000_000 + id).is_none(), + "{}: deleted row {id} still in the unique index", + $label + ); + } + } + + /// A unique collision under contention rejects exactly one writer. + /// + /// Every writer races to claim the same payload. Exactly one must + /// win: two winners is a broken unique index, zero is a broken + /// insert. + #[test] + fn a_contended_unique_key_admits_exactly_one_writer() { + let writers = params::writers(); + + let table = Arc::new(ConcWorkTable::default()); + let winners = Arc::new(AtomicU64::new(0)); + std::thread::scope(|scope| { + for w in 0..writers { + let table = Arc::clone(&table); + let winners = Arc::clone(&winners); + scope.spawn(move || { + // Distinct primary keys, one shared payload. + let contended = ConcRow { id: w, payload: 42, bucket: 0 }; + if table.insert(contended).is_ok() { + winners.fetch_add(1, Ordering::Release); + } + }); + } + }); + + assert_eq!( + winners.load(Ordering::Acquire), + 1, + "{}: a contended unique key admitted more than one writer", + $label + ); + assert_eq!(table.count(), 1, "{}", $label); + } + + /// Concurrent readers see a consistent non-unique group while it is + /// being written. + #[test] + fn readers_see_consistent_groups_during_writes() { + let (writers, per_writer) = (params::writers(), params::per_writer()); + let readers = params::readers(); + + let table = Arc::new(ConcWorkTable::default()); + std::thread::scope(|scope| { + for w in 0..writers { + let table = Arc::clone(&table); + scope.spawn(move || { + for n in 0..per_writer { + table.insert(row(w * per_writer + n)).expect("insert"); + } + }); + } + // Readers running throughout: a group read must never + // return a row that is not in that group. + for _ in 0..readers { + let table = Arc::clone(&table); + scope.spawn(move || { + for _ in 0..2_000 { + for bucket in 0..16u32 { + for row in table.select_by_bucket(bucket).execute().unwrap() { + assert_eq!(row.bucket, bucket, "{}: row in the wrong group", $label); + } + } + } + }); + } + }); + + let mut counts: HashMap = HashMap::new(); + for id in 0..(writers * per_writer) { + let row = table.select(id).expect("row present"); + *counts.entry(row.bucket).or_default() += 1; + } + for bucket in 0..16u32 { + let selected = table.select_by_bucket(bucket).execute().unwrap(); + assert_eq!( + selected.len(), + *counts.get(&bucket).unwrap_or(&0), + "{}: group {bucket} disagrees with the rows", + $label + ); + } + } + } + }; +} + +backend_suite!(wti, worktables_index, "wti"); +backend_suite!(arctic, arctic, "arctic"); +backend_suite!(congee, congee, "congee"); + +worktable!( + name: Scale, + persist: false, + columns: { + id: u64 primary_key, + payload: u64, + }, + indexes: { payload_idx: payload unique }, +); + +/// Insert throughput must not collapse as writers are added. +/// +/// **Ignored because it fails on current code, deliberately.** It records a +/// defect rather than guarding against one, in the same way +/// `worktable_codegen`'s `generator_determinism` does. Run it with +/// `cargo test --test mod insert_throughput -- --ignored --nocapture`. +/// +/// Measured on an M4 Max, best of three, 200,000 inserts: +/// +/// | writers | throughput | vs 1 writer | +/// | ---: | ---: | ---: | +/// | 1 | 1.20 M/s | 1.00x | +/// | 2 | 1.20 M/s | 1.00x | +/// | 4 | 1.11 M/s | 0.92x | +/// | 8 | 297 K/s | **0.25x** | +/// | 16 | 266 K/s | 0.22x | +/// +/// Eight concurrent writers are four times slower **in aggregate** than one. +/// It is not the index: all three backends collapse to the same ~300 K/s, and +/// arctic and congee are 1.3x faster than WTI single-threaded before hitting +/// the identical wall. A `sample` of the eight-writer run puts the time in +/// `parking_lot::RawRwLock::lock_exclusive_slow` and `DataPages`, which is +/// `pages.rs`: every insert takes an exclusive write lock on the *one* page +/// named by `current_page_id`, so appends serialise by construction, and +/// `EmptyLinkRegistry::pop_max` takes a global mutex on every insert even when +/// the free list is empty. +/// +/// The threshold is 0.6x rather than 1.0x: some loss is expected from cache +/// traffic and allocation, and a benchmark-shaped assertion on a shared machine +/// has to leave room. At 0.25x this is not a threshold question. +#[test] +#[ignore = "records the concurrent-insert collapse; fails until pages.rs stops serialising appends"] +fn insert_throughput_should_scale_past_four_writers() { + // A debug build makes this test lie, and lie reassuringly. Per-operation + // overhead swamps the lock contention, so the collapse disappears and the + // sweep reports 8 writers as *faster* than 1 (measured: 2.79x). A + // throughput assertion that passes for the wrong reason is worse than none, + // so refuse rather than mislead. + assert!( + params::is_release_build(), + "run this in release: `cargo test --release --test mod insert_throughput -- --ignored --nocapture`. \ + In a debug build the per-operation overhead hides the contention and this test passes for the wrong reason." + ); + + let n = params::scale_rows(); + let floor = params::scale_floor(); + + let throughput = |writers: u64| -> f64 { + let mut best = f64::MAX; + for _ in 0..3 { + let table = Arc::new(ScaleWorkTable::default()); + let per = n / writers; + let start = Instant::now(); + std::thread::scope(|scope| { + for w in 0..writers { + let table = Arc::clone(&table); + scope.spawn(move || { + for i in (w * per)..((w + 1) * per) { + let _ = table.insert(ScaleRow { + id: i, + payload: 1_000_000 + i, + }); + } + }); + } + }); + let ns = start.elapsed().as_nanos() as f64 / n as f64; + if ns < best { + best = ns; + } + } + 1e9 / best + }; + + let single = throughput(1); + println!(" 1 writer : {single:>12.0}/s 1.00x (baseline)"); + for writers in params::scale_sweep() { + let scaled = throughput(writers); + println!("{writers:>3} writers: {scaled:>12.0}/s {:.2}x", scaled / single); + assert!( + scaled / single >= floor, + "{writers} writers reached {:.2}x of single-writer throughput, below the {floor:.2}x floor", + scaled / single + ); + } +} diff --git a/tests/worktable/delete_many.rs b/tests/worktable/delete_many.rs new file mode 100644 index 00000000..959fc765 --- /dev/null +++ b/tests/worktable/delete_many.rs @@ -0,0 +1,248 @@ +//! Bulk delete: what it removes, what it leaves reachable, and what it reuses. +//! +//! The single-row `delete` already ghosts rather than moves: the row is marked +//! deleted in place, its index entries come out, and the storage becomes +//! reusable once no reader can still reach it. `delete_many` is that, batched, +//! and the batching is not cosmetic. The per-row cost is dominated by the +//! reclamation bookkeeping each retirement takes rather than by the bit flip, +//! so a loop of `n` deletes pays `n` domain advances where a batch pays one. +//! +//! These tests are about behaviour rather than speed. The property that would +//! actually bite a consumer is the one in +//! `deleted_rows_are_unreachable_through_every_index`: a bulk delete that +//! removed rows from the primary index but left a secondary index pointing at +//! their storage would still pass a naive `select` test, and would resolve to +//! reused storage the moment an insert claimed the link. + +use worktable::prelude::*; +use worktable::worktable; + +worktable! ( + name: Evict, + columns: { + id: u64 primary_key autoincrement, + unique_value: u64, + generation: u32, + }, + indexes: { + unique_value_idx: unique_value unique, + generation_idx: generation, + }, +); + +fn row(id: u64, unique_value: u64, generation: u32) -> EvictRow { + EvictRow { + id, + unique_value, + generation, + } +} + +fn table_with(rows: u64) -> EvictWorkTable { + let table = EvictWorkTable::default(); + let batch: Vec<_> = (0..rows).map(|i| row(i, 1_000 + i, (i % 4) as u32)).collect(); + table.insert_many(batch).expect("fixture inserts"); + table +} + +#[tokio::test] +async fn delete_many_removes_exactly_the_named_keys() { + let table = table_with(20); + + let deleted = table.delete_many((0..5u64).collect()).await.expect("bulk delete"); + + let expected: Vec = (0..5u64).map(Into::into).collect(); + assert_eq!(deleted, expected); + assert_eq!(table.count(), 15); + for id in 0..5u64 { + assert!(table.select(id).is_none(), "row {id} should be gone"); + } + for id in 5..20u64 { + assert!(table.select(id).is_some(), "row {id} should survive"); + } +} + +/// The property a bulk delete is most likely to get wrong. +/// +/// Removing rows from the primary index while leaving a secondary index +/// pointing at their storage leaves a dangling entry: the link is queued for +/// reuse, so a later insert can claim it and the stale entry then resolves to a +/// live, unrelated row. +/// +/// Reading it back is **not** enough to catch that, and this test asserted only +/// that at first. A ghosted row is filtered out of reads, so a stale index +/// entry is invisible through `select` and the test passed with secondary +/// removal deleted entirely. What catches it is claiming the key again: a +/// unique index that still holds the deleted row's value rejects the insert. +#[tokio::test] +async fn deleted_rows_are_unreachable_through_every_index() { + let table = table_with(20); + + table.delete_many((0..5u64).collect()).await.expect("bulk delete"); + + for id in 0..5u64 { + assert!( + table.select_by_unique_value(1_000 + id).is_none(), + "unique index still resolves deleted row {id}" + ); + } + // The non-unique index must have lost exactly the deleted members of each + // group, not the whole group. + let generation_zero = table.select_by_generation(0).execute().expect("non-unique read"); + let surviving: Vec = generation_zero.iter().map(|r| r.id).collect(); + assert!( + surviving.iter().all(|id| *id >= 5), + "non-unique index still resolves deleted rows: {surviving:?}" + ); + assert!( + surviving.contains(&8) && surviving.contains(&12), + "non-unique index lost rows it should have kept: {surviving:?}" + ); + + // The assertion with teeth: reclaiming a deleted row's unique value must + // succeed. If the unique index still holds the entry, this is rejected. + for id in 0..5u64 { + table + .insert(row(100 + id, 1_000 + id, 9)) + .unwrap_or_else(|error| panic!("unique value {} was not released by the delete: {error}", 1_000 + id)); + } +} + +/// A key that is not there is skipped, not an error. +/// +/// A caller evicting a generation does not know which of its keys a concurrent +/// writer already removed, and making them find out first is a race they cannot +/// win. The return value is what was actually deleted, so the caller can tell. +#[tokio::test] +async fn absent_keys_are_skipped_rather_than_failing_the_batch() { + let table = table_with(10); + + let deleted = table + .delete_many(vec![1u64, 999, 3, 1_000, 5]) + .await + .expect("absent keys must not fail the batch"); + + let expected: Vec = vec![1u64, 3, 5].into_iter().map(Into::into).collect(); + assert_eq!(deleted, expected); + assert_eq!(table.count(), 7); +} + +/// Deleting the same key twice in one batch is not a double free. +#[tokio::test] +async fn a_repeated_key_is_deleted_once() { + let table = table_with(10); + + let deleted = table.delete_many(vec![2u64, 2, 2]).await.expect("repeats must be safe"); + + let expected: Vec = vec![2u64.into()]; + assert_eq!(deleted, expected, "a key already ghosted in this batch is skipped"); + assert_eq!(table.count(), 9); +} + +#[tokio::test] +async fn an_empty_batch_is_a_no_op() { + let table = table_with(4); + assert_eq!( + table.delete_many(Vec::::new()).await.expect("empty batch"), + Vec::::new() + ); + assert_eq!(table.count(), 4); +} + +/// Storage from a bulk delete is reused, which is the point of the exercise. +/// +/// Not asserted as an exact byte figure: reuse happens once no reader can +/// reach the links, so the observable property is that a delete-then-insert +/// cycle does not grow the table without bound. A table that never reused a +/// link would grow by the full batch on every cycle. +#[tokio::test] +async fn bulk_delete_then_insert_reuses_storage() { + let table = table_with(200); + let before = table.count(); + + for cycle in 0..10u64 { + let keys: Vec = (0..100).collect(); + table.delete_many(keys).await.expect("bulk delete"); + assert_eq!(table.count(), before - 100); + + let refill: Vec<_> = (0..100) + .map(|i| row(i, 500_000 + cycle * 1_000 + i, (i % 4) as u32)) + .collect(); + table.insert_many(refill).expect("refill"); + assert_eq!(table.count(), before); + } + + // Every row is still readable through both indexes after ten cycles of + // ghosting and reclaiming the same links. + for id in 0..200u64 { + assert!(table.select(id).is_some(), "row {id} lost after reuse cycles"); + } +} + +/// A bulk delete and the single-row path agree. +/// +/// Cheap to state and the thing most likely to drift: if `delete_many` ever +/// stops doing exactly what a loop of `delete` does, this is where it shows. +#[tokio::test] +async fn delete_many_matches_a_loop_of_delete() { + let batched = table_with(30); + let looped = table_with(30); + + let keys: Vec = (0..30).filter(|i| i % 3 == 0).collect(); + batched.delete_many(keys.clone()).await.expect("bulk delete"); + for key in &keys { + looped.delete(*key).await.expect("single delete"); + } + + assert_eq!(batched.count(), looped.count()); + for id in 0..30u64 { + assert_eq!( + batched.select(id).is_some(), + looped.select(id).is_some(), + "row {id} disagrees between the batched and looped paths" + ); + } +} + +/// Eviction by span, which is the shape a caller dropping a generation has. +#[tokio::test] +async fn delete_range_removes_the_span_and_nothing_else() { + let table = table_with(20); + + let deleted = table + .delete_range(EvictPrimaryKey::from(5u64)..EvictPrimaryKey::from(10u64)) + .await; + let deleted = deleted.expect("range delete"); + + let expected: Vec = (5u64..10).map(Into::into).collect(); + assert_eq!(deleted, expected, "half-open: 10 is not included"); + assert_eq!(table.count(), 15); + for id in 5..10u64 { + assert!(table.select(id).is_none(), "row {id} should be gone"); + assert!( + table.select_by_unique_value(1_000 + id).is_none(), + "unique index still resolves deleted row {id}" + ); + } + assert!(table.select(4u64).is_some(), "the row below the span survives"); + assert!(table.select(10u64).is_some(), "the row at the exclusive end survives"); +} + +/// An inclusive end, and a range that matches nothing. +#[tokio::test] +async fn delete_range_honours_its_bounds() { + let table = table_with(20); + + let deleted = table + .delete_range(EvictPrimaryKey::from(0u64)..=EvictPrimaryKey::from(2u64)) + .await + .expect("inclusive range"); + assert_eq!(deleted.len(), 3, "0, 1 and 2"); + + let empty = table + .delete_range(EvictPrimaryKey::from(500u64)..EvictPrimaryKey::from(600u64)) + .await + .expect("a range matching nothing is not an error"); + assert!(empty.is_empty()); + assert_eq!(table.count(), 17); +} diff --git a/tests/worktable/mod.rs b/tests/worktable/mod.rs index 723f1aaf..3d4e22e7 100644 --- a/tests/worktable/mod.rs +++ b/tests/worktable/mod.rs @@ -3,10 +3,12 @@ mod base; mod bench; mod borrowed_primary_key; mod cancel_safety; +mod concurrency; mod config; mod count; mod custom_pk; mod delete; +mod delete_many; mod float; mod in_place; mod index;