From 8ba799a0d5a0335284b7d17d3e7fb3a258209ad1 Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 1 Sep 2026 14:04:51 +0700 Subject: [PATCH 1/6] refactor: extract the schema language into worktable_dsl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A schema is written down once, in a `worktable!` invocation, and the parser that understands it lived in `worktable_codegen`, which is `proc-macro = true`. A proc-macro crate can export nothing but macros, so every type describing a schema — the columns, the primary key, the indexes, the queries — was unreachable from any other crate however public it was declared. `mod common` was not public at that crate's root either. So anything wanting to *read* a declaration had two options: re-implement the grammar and drift from it, or do without. A diagram, a migration tool, a documentation generator and an editor all want to read one. `lib.rs` has carried `// TODO: Refactor this codegen stuff because it's now too strange.` `model` and `parser` move to `worktable_dsl`, a plain library. Nothing in them changed; the dependencies are the five they already used, none added, none dropped. `worktable_codegen` now depends on it, so there is one grammar rather than a copy that can disagree with the compiler about what a schema means. `name_generator` stays in codegen. It invents Rust identifiers for generated code, which is not the schema language, and generators here define inherent `impl`s on `WorktableNameGenerator` — the orphan rule allows that only in the crate owning the type. I had it in the extracted crate first and the compiler made the same argument the design does. `crate::common::` still resolves, through a thin module that re-exports the new crate, so the 127 paths across 67 files are untouched and the diff stays a move rather than a sweep. An integration test reads a declaration from outside, which is the claim worth pinning: it compiles as its own crate, so it stops building if this ever becomes a proc-macro crate again. It also records a property no caller existed to depend on before. `Columns::columns_map` is a `std::collections::HashMap`, whose iteration order Rust randomises per process; two runs of the same input gave `["answered", "project_id", "id"]` and `["project_id", "answered", "id"]`. The macro never cared, and the parser's own tests collect it into another `HashMap` and assert membership, so nothing noticed. A consumer rendering columns in that order draws a different table every run. `field_positions` already carries the declaration order and is the field to sort by; the test asserts that, and says so, so the next consumer learns it here rather than by shipping the bug. --- Cargo.toml | 2 +- codegen/Cargo.toml | 3 + codegen/src/common/mod.rs | 17 ++- codegen/src/lib.rs | 8 ++ dsl/Cargo.toml | 21 ++++ dsl/src/lib.rs | 43 ++++++++ .../src/common => dsl/src}/model/column.rs | 4 +- .../src/common => dsl/src}/model/config.rs | 0 .../src/common => dsl/src}/model/index.rs | 0 {codegen/src/common => dsl/src}/model/mod.rs | 0 .../src/common => dsl/src}/model/operation.rs | 0 .../src/common => dsl/src}/model/partition.rs | 0 .../common => dsl/src}/model/persistence.rs | 0 .../common => dsl/src}/model/primary_key.rs | 0 .../src/common => dsl/src}/model/queries.rs | 2 +- .../common => dsl/src}/parser/attribute.rs | 8 +- .../src/common => dsl/src}/parser/columns.rs | 8 +- .../src/common => dsl/src}/parser/config.rs | 4 +- .../src/common => dsl/src}/parser/index.rs | 8 +- {codegen/src/common => dsl/src}/parser/mod.rs | 0 .../src/common => dsl/src}/parser/name.rs | 4 +- .../src/common => dsl/src}/parser/punct.rs | 2 +- .../src}/parser/queries/delete.rs | 6 +- .../src}/parser/queries/in_place.rs | 6 +- .../common => dsl/src}/parser/queries/mod.rs | 4 +- .../src}/parser/queries/operation.rs | 6 +- .../src}/parser/queries/select.rs | 6 +- .../src}/parser/queries/update.rs | 6 +- dsl/tests/readable_from_outside.rs | 100 ++++++++++++++++++ src/index/arctic_multi.rs | 23 ++-- src/index/persistent_art.rs | 5 +- src/index/table_index/mod.rs | 5 +- src/lib.rs | 15 ++- src/partition/mod.rs | 2 +- src/persistence/mod.rs | 8 +- src/persistence/space/art_index.rs | 9 +- tests/worktable/nonunique_arctic.rs | 4 +- tests/worktable/partitioned.rs | 6 +- 38 files changed, 269 insertions(+), 76 deletions(-) create mode 100644 dsl/Cargo.toml create mode 100644 dsl/src/lib.rs rename {codegen/src/common => dsl/src}/model/column.rs (97%) rename {codegen/src/common => dsl/src}/model/config.rs (100%) rename {codegen/src/common => dsl/src}/model/index.rs (100%) rename {codegen/src/common => dsl/src}/model/mod.rs (100%) rename {codegen/src/common => dsl/src}/model/operation.rs (100%) rename {codegen/src/common => dsl/src}/model/partition.rs (100%) rename {codegen/src/common => dsl/src}/model/persistence.rs (100%) rename {codegen/src/common => dsl/src}/model/primary_key.rs (100%) rename {codegen/src/common => dsl/src}/model/queries.rs (86%) rename {codegen/src/common => dsl/src}/parser/attribute.rs (97%) rename {codegen/src/common => dsl/src}/parser/columns.rs (97%) rename {codegen/src/common => dsl/src}/parser/config.rs (98%) rename {codegen/src/common => dsl/src}/parser/index.rs (97%) rename {codegen/src/common => dsl/src}/parser/mod.rs (100%) rename {codegen/src/common => dsl/src}/parser/name.rs (98%) rename {codegen/src/common => dsl/src}/parser/punct.rs (98%) rename {codegen/src/common => dsl/src}/parser/queries/delete.rs (95%) rename {codegen/src/common => dsl/src}/parser/queries/in_place.rs (94%) rename {codegen/src/common => dsl/src}/parser/queries/mod.rs (97%) rename {codegen/src/common => dsl/src}/parser/queries/operation.rs (97%) rename {codegen/src/common => dsl/src}/parser/queries/select.rs (95%) rename {codegen/src/common => dsl/src}/parser/queries/update.rs (95%) create mode 100644 dsl/tests/readable_from_outside.rs diff --git a/Cargo.toml b/Cargo.toml index 6de183fe..962873c0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -members = ["codegen", "examples", "performance_measurement", "performance_measurement/codegen"] +members = ["codegen", "dsl", "examples", "performance_measurement", "performance_measurement/codegen"] [package] name = "worktable" diff --git a/codegen/Cargo.toml b/codegen/Cargo.toml index d4269682..356709e7 100644 --- a/codegen/Cargo.toml +++ b/codegen/Cargo.toml @@ -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.14" } 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..298d1f4c 100644 --- a/codegen/src/common/mod.rs +++ b/codegen/src/common/mod.rs @@ -1,7 +1,14 @@ -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, *}; +pub use worktable_dsl::{model, parser}; 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/dsl/Cargo.toml b/dsl/Cargo.toml new file mode 100644 index 00000000..9f23067b --- /dev/null +++ b/dsl/Cargo.toml @@ -0,0 +1,21 @@ +[package] +name = "worktable_dsl" +version = "1.0.0-beta.14" +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 make this a rewrite rather than a lift. +syn = { version = "2.0.74", features = ["full"] } +quote = "1.0.36" +proc-macro2 = "1.0.86" +convert_case = "0.6.0" +indexmap = "2" + +[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" diff --git a/dsl/src/lib.rs b/dsl/src/lib.rs new file mode 100644 index 00000000..b044c841 --- /dev/null +++ b/dsl/src/lib.rs @@ -0,0 +1,43 @@ +//! 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. + +pub mod model; +pub mod parser; + +#[allow(unused_imports)] +pub use model::*; +pub use parser::Parser; 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 100% rename from codegen/src/common/model/index.rs rename to dsl/src/model/index.rs 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 100% rename from codegen/src/common/model/persistence.rs rename to dsl/src/model/persistence.rs diff --git a/codegen/src/common/model/primary_key.rs b/dsl/src/model/primary_key.rs similarity index 100% rename from codegen/src/common/model/primary_key.rs rename to dsl/src/model/primary_key.rs 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 98% rename from codegen/src/common/parser/config.rs rename to dsl/src/parser/config.rs index 4bf2c95c..08a54b39 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"; 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 95% rename from codegen/src/common/parser/queries/delete.rs rename to dsl/src/parser/queries/delete.rs index ffe5aee9..757d7797 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> { @@ -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/in_place.rs b/dsl/src/parser/queries/in_place.rs similarity index 94% rename from codegen/src/common/parser/queries/in_place.rs rename to dsl/src/parser/queries/in_place.rs index 47fc83f8..c9809592 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> { @@ -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/mod.rs b/dsl/src/parser/queries/mod.rs similarity index 97% rename from codegen/src/common/parser/queries/mod.rs rename to dsl/src/parser/queries/mod.rs index 75d0f3a8..b6525ad1 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 { 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/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/src/index/arctic_multi.rs b/src/index/arctic_multi.rs index a3f4b174..37cc8776 100644 --- a/src/index/arctic_multi.rs +++ b/src/index/arctic_multi.rs @@ -142,7 +142,9 @@ where self.remove_dead_slot(&raw); continue; } - links.links.push(value.take().expect("reclaimed above or never consumed")); + links + .links + .push(value.take().expect("reclaimed above or never consumed")); self.len.fetch_add(1, Ordering::Relaxed); return; } @@ -232,7 +234,12 @@ where while let Some((key, slot)) = entries.lend() { let raw = ::insert_to_key(key); let links = slot.read(); - pairs.extend(links.links.iter().map(|value| (K::from_arctic(raw), value.clone()))); + pairs.extend( + links + .links + .iter() + .map(|value| (K::from_arctic(raw), value.clone())), + ); } pairs }}; @@ -318,10 +325,7 @@ mod tests { index.insert_pair(3, value); } assert_eq!(index.remove_pair(&3, &2), Some(2)); - assert_eq!( - index.get(&3).map(|(_, v)| v).collect::>(), - vec![0, 1, 3, 4] - ); + assert_eq!(index.get(&3).map(|(_, v)| v).collect::>(), vec![0, 1, 3, 4]); } #[test] @@ -338,12 +342,7 @@ mod tests { // Degenerate excluded bounds are empty, not unbounded. assert_eq!(index.range(..0).count(), 0); - assert_eq!( - index - .range((Bound::Excluded(u64::MAX), Bound::Unbounded)) - .count(), - 0 - ); + assert_eq!(index.range((Bound::Excluded(u64::MAX), Bound::Unbounded)).count(), 0); assert_eq!(index.iter().count(), 30); } diff --git a/src/index/persistent_art.rs b/src/index/persistent_art.rs index 26a568d9..eda5a690 100644 --- a/src/index/persistent_art.rs +++ b/src/index/persistent_art.rs @@ -278,7 +278,10 @@ where fn insert_cdc(&self, value: T, link: Link) -> (Option, Vec>>) { let _sequence_guard = self.mutation_stripe(&value).lock(); self.inner.insert_pair(value.clone(), OffsetEqLink(link)); - let pair = Pair { key: value, value: link }; + let pair = Pair { + key: value, + value: link, + }; let event = ChangeEvent::InsertAt { event_id: self.next_event_id(), max_value: pair.clone(), diff --git a/src/index/table_index/mod.rs b/src/index/table_index/mod.rs index cc13e002..f5afeec0 100644 --- a/src/index/table_index/mod.rs +++ b/src/index/table_index/mod.rs @@ -10,9 +10,8 @@ use vanilla_indexset::core::pair::Pair as VanillaPair; use crate::util::OffsetEqLink; use crate::{ - ArcticIndex, ArcticKey, ArcticMultiIndex, CongeeIndex, CongeeKey, IndexMap, IndexMultiMap, - PersistentArcticIndex, PersistentArcticMultiIndex, PersistentCongeeIndex, PersistentWtiIndex, UniqueIndex, - UpstreamIndexMap, + ArcticIndex, ArcticKey, ArcticMultiIndex, CongeeIndex, CongeeKey, IndexMap, IndexMultiMap, PersistentArcticIndex, + PersistentArcticMultiIndex, PersistentCongeeIndex, PersistentWtiIndex, UniqueIndex, UpstreamIndexMap, }; mod cdc; diff --git a/src/lib.rs b/src/lib.rs index 79a73261..021fa5d2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -39,9 +39,9 @@ pub mod prelude { AcknowledgeOperation, ArtPersistenceKey, DeleteOperation, DiskConfig, DiskPersistenceEngine, IndexTableOfContents, InsertOperation, LoadMode, Operation, OperationId, PersistedWorkTable, PersistenceConfig, PersistenceEngine, PersistenceError, PersistenceIndexCorruption, PersistenceLoadError, PersistenceMonitor, - PersistenceResult, PersistenceState, PersistenceTask, ReadOnlyPersistenceEngine, SpaceArcticIndex, SpaceArcticMultiIndex, - SpaceCongeeIndex, SpaceData, SpaceDataOps, SpaceIndex, SpaceIndexOps, SpaceIndexUnsized, SpaceLogicalIndex, - SpaceLogicalIndexUnsized, SpaceSecondaryIndexOps, TocEntryOversizedError, UpdateOperation, + PersistenceResult, PersistenceState, PersistenceTask, ReadOnlyPersistenceEngine, SpaceArcticIndex, + SpaceArcticMultiIndex, SpaceCongeeIndex, SpaceData, SpaceDataOps, SpaceIndex, SpaceIndexOps, SpaceIndexUnsized, + SpaceLogicalIndex, SpaceLogicalIndexUnsized, SpaceSecondaryIndexOps, TocEntryOversizedError, UpdateOperation, load_persisted_state, map_index_pages_to_toc_and_general, map_unsized_index_pages_to_toc_and_general, reconstruct_multi_index_nodes, validate_events, }; @@ -54,11 +54,10 @@ pub mod prelude { 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, + 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 4d718747..a7f598d0 100644 --- a/src/partition/mod.rs +++ b/src/partition/mod.rs @@ -72,9 +72,9 @@ use std::sync::Arc; #[cfg(not(wt_loom))] use std::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; +use crate::mem_stat::MemStat; #[cfg(not(wt_loom))] use crate::util::epoch::EpochDomain; -use crate::mem_stat::MemStat; /// Most retired partitions one opportunistic collect frees inline. #[cfg(not(wt_loom))] diff --git a/src/persistence/mod.rs b/src/persistence/mod.rs index f3f754d9..acfe1cf7 100644 --- a/src/persistence/mod.rs +++ b/src/persistence/mod.rs @@ -16,10 +16,10 @@ pub use operation::{ }; pub use readonly_engine::ReadOnlyPersistenceEngine; pub use space::{ - ArtPersistenceKey, IndexTableOfContents, SpaceArcticIndex, SpaceArcticMultiIndex, SpaceCongeeIndex, SpaceData, SpaceDataOps, SpaceIndex, - SpaceIndexOps, SpaceIndexUnsized, SpaceLogicalIndex, SpaceLogicalIndexUnsized, SpaceSecondaryIndexOps, - TocEntryOversizedError, map_index_pages_to_toc_and_general, map_unsized_index_pages_to_toc_and_general, - reconstruct_multi_index_nodes, + ArtPersistenceKey, IndexTableOfContents, SpaceArcticIndex, SpaceArcticMultiIndex, SpaceCongeeIndex, SpaceData, + SpaceDataOps, SpaceIndex, SpaceIndexOps, SpaceIndexUnsized, SpaceLogicalIndex, SpaceLogicalIndexUnsized, + SpaceSecondaryIndexOps, TocEntryOversizedError, map_index_pages_to_toc_and_general, + map_unsized_index_pages_to_toc_and_general, reconstruct_multi_index_nodes, }; pub use task::{PersistenceMonitor, PersistenceTask}; diff --git a/src/persistence/space/art_index.rs b/src/persistence/space/art_index.rs index 1bd47a58..d8766fc7 100644 --- a/src/persistence/space/art_index.rs +++ b/src/persistence/space/art_index.rs @@ -1215,7 +1215,10 @@ mod tests { let bytes = encode_multi_pairs(pairs.iter().cloned()); assert_eq!(decode_multi_pairs::(&bytes).unwrap(), pairs); assert!(decode_multi_pairs::(&bytes[..bytes.len() - 1]).is_err()); - assert_eq!(decode_multi_pairs::(&encode_multi_pairs(std::iter::empty::<(u64, Link)>())).unwrap(), vec![]); + assert_eq!( + decode_multi_pairs::(&encode_multi_pairs(std::iter::empty::<(u64, Link)>())).unwrap(), + vec![] + ); } #[test] @@ -1259,7 +1262,9 @@ mod tests { assert_eq!(index.get(&9).map(|(_, link)| link.0).collect::>(), vec![link(4)]); space.compact().await.unwrap(); - let image = ArtFile::::read_image(&path, Backend::ArcticMulti, 5).await.unwrap(); + let image = ArtFile::::read_image(&path, Backend::ArcticMulti, 5) + .await + .unwrap(); assert!(image.wal.is_empty()); drop(space); diff --git a/tests/worktable/nonunique_arctic.rs b/tests/worktable/nonunique_arctic.rs index 5d9019fb..ea0da930 100644 --- a/tests/worktable/nonunique_arctic.rs +++ b/tests/worktable/nonunique_arctic.rs @@ -135,7 +135,9 @@ fn range_select_over_non_unique_arctic_keys() { let table = ArcticAdjacencyWorkTable::default(); for key in 0..10u64 { for copy in 0..3u128 { - table.insert(row(&table, key as u128, key as u128 * 100 + copy, key)).unwrap(); + table + .insert(row(&table, key as u128, key as u128 * 100 + copy, key)) + .unwrap(); } } diff --git a/tests/worktable/partitioned.rs b/tests/worktable/partitioned.rs index 2f0f2359..e98d38fa 100644 --- a/tests/worktable/partitioned.rs +++ b/tests/worktable/partitioned.rs @@ -313,7 +313,11 @@ fn a_removed_partition_waits_out_its_readers_then_frees_through_a_shared_handle( let reader = prices.partition_ref(4).expect("present"); let taken = prices.remove(4).expect("was present"); assert_eq!(prices.len(), 0); - assert_eq!(prices.retired_len(), 1, "removal must wait out the reader's grace period"); + assert_eq!( + prices.retired_len(), + 1, + "removal must wait out the reader's grace period" + ); // All three handles still work: this is the reader-mid-query case. assert_eq!(reader.select(2).unwrap().bid, 9.0); From dfe8ef6495090893d7a364ffed8e9f105c31dac0 Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 1 Sep 2026 20:23:50 +0700 Subject: [PATCH 2/6] Drop the redundant glob from the codegen shim `pub use worktable_dsl::{Parser, *};` names `Parser` and then re-exports it again through the glob, and rustc reports the glob as unused: nothing reaches the shim that way, because every caller goes through `crate::common::model::` or `crate::common::parser::`. That is only a warning locally, which is why it survived the extraction. CI runs `cargo clippy --workspace --all-targets -- -D warnings`, where it is a build failure. --- codegen/src/common/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/codegen/src/common/mod.rs b/codegen/src/common/mod.rs index 298d1f4c..c7ad9e1d 100644 --- a/codegen/src/common/mod.rs +++ b/codegen/src/common/mod.rs @@ -10,5 +10,4 @@ //! by another crate. The compiler makes the same argument the design does. pub mod name_generator; -pub use worktable_dsl::{Parser, *}; -pub use worktable_dsl::{model, parser}; +pub use worktable_dsl::{Parser, model, parser}; From fdb3f95ecdef5d07064b0ab38172fddcc15e0e81 Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 1 Sep 2026 20:24:15 +0700 Subject: [PATCH 3/6] Read a schema as data, and write one back `worktable_dsl` can parse a declaration, which is half of what a designer needs. The other half is holding one, comparing it, storing it, and writing it back out as text the compiler accepts. `crate::model` cannot do any of that: it is built out of `Ident` and `TokenStream`, which is right for a thing whose job is to become Rust code and wrong for everything else. An `Ident` cannot be serialised or constructed outside a proc-macro context without a `Span::call_site` that lies about where it came from, and a `TokenStream` is not `PartialEq`, so two schemas cannot even be asked whether they differ. That is the one question a migration planner exists to answer. `schema::Schema` is the same declaration with the compiler's concerns removed: `String` for `Ident`, ordered `Vec`s for `HashMap`s, no spans. It derives `PartialEq` so schemas can be diffed, and under an optional `serde` feature it derives `Serialize`, so one can be stored next to the data it describes and read back by a process that never saw the Rust type. The feature is off by default because `worktable_codegen` depends on this crate and is a proc macro: every WorkTable user compiles it for the host before anything else in their build, and they should not pay for a derive macro that serves consumers who are not the compiler. The approach is additive. Nothing in the parser or the generators changed, and the model is untouched apart from three `cfg_attr` derives on plain enums the IR reuses rather than duplicates. Inverting the parser to produce plain data directly would have been a 61-file edit across 13k lines of generators with no test proving the output was unchanged. Two emitters. `to_dsl` renders the declaration body back to text, which is what makes a drawing editable: read, change, write, compile. `to_mermaid` renders UML class notation, chosen because it is text, so it is diffable and needs no rendering dependency, and because it renders anywhere Markdown does. Columns are attributes carrying their markers, queries are operations, and the partition key is a note rather than an attribute because it is stored once per partition and no query can name it. The schema language has no foreign keys, so `infer_relations` guesses links from a single stated naming rule and returns what it guessed; `schemas_to_mermaid` draws those as dependencies rather than associations, because a dashed arrow is the honest notation for a link the declaration does not make. Ordering is a guarantee here, not an accident. `columns_map` is a `HashMap`, whose iteration order Rust randomises, so a consumer walking it draws a different table on every run; `field_positions` records the declaration order and is what the IR sorts by. The query maps have no such field, so those are sorted by name, which is at least stable, and a test says which is which. The round trip is checked against the repository rather than against a fixture. `tests/round_trip.rs` finds all 128 `worktable!` invocations in the tree, sets aside the 12 that are `macro_rules!` templates full of metavariables, and asserts `parse(emit(parse(x))) == parse(x)` for the remaining 116. Those were written by people not thinking about this crate, which makes them a better corpus than anything written here. `codegen` adds the claim this crate cannot make about itself: that emitted text is a declaration the macro accepts. That check has to live on the near side of the proc-macro boundary. Three things the corpus turned up. The emitter writes no comma after `delete`, `in_place` or `config` blocks. `parse_updates` consumes one and those three do not, so a comma there arrives at a dispatch loop as a `,` token and dies as "Unexpected identifier". Omitting it is the only form all of them accept. Expanding one declaration twice does not produce one program. Several generators iterate `columns_map` directly to emit an ordered construct, the `RowFields` and `AvaiableTypes` enums among them, so the variant order differs between two expansions in one process and can differ between two compilations of the same source. `generator_determinism` records it with the evidence and is ignored rather than deleted: the fix changes the generated code of every table and deserves reviewing on its own. It is also why `emitted_declarations` can only assert that an emitted declaration expands, not that it generates identical code. --- codegen/src/worktable/mod.rs | 167 +++++++++++++++ dsl/Cargo.toml | 14 +- dsl/src/lib.rs | 5 + dsl/src/model/index.rs | 1 + dsl/src/model/persistence.rs | 1 + dsl/src/model/primary_key.rs | 1 + dsl/src/schema/emit_dsl.rs | 168 +++++++++++++++ dsl/src/schema/emit_uml.rs | 216 ++++++++++++++++++++ dsl/src/schema/mod.rs | 381 +++++++++++++++++++++++++++++++++++ dsl/tests/round_trip.rs | 180 +++++++++++++++++ dsl/tests/schema.rs | 258 ++++++++++++++++++++++++ 11 files changed, 1391 insertions(+), 1 deletion(-) create mode 100644 dsl/src/schema/emit_dsl.rs create mode 100644 dsl/src/schema/emit_uml.rs create mode 100644 dsl/src/schema/mod.rs create mode 100644 dsl/tests/round_trip.rs create mode 100644 dsl/tests/schema.rs diff --git a/codegen/src/worktable/mod.rs b/codegen/src/worktable/mod.rs index c4ddfac2..ca671904 100644 --- a/codegen/src/worktable/mod.rs +++ b/codegen/src/worktable/mod.rs @@ -704,3 +704,170 @@ 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); + } +} diff --git a/dsl/Cargo.toml b/dsl/Cargo.toml index 9f23067b..1650db82 100644 --- a/dsl/Cargo.toml +++ b/dsl/Cargo.toml @@ -8,14 +8,26 @@ 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 make this a rewrite rather than a lift. +# 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"] diff --git a/dsl/src/lib.rs b/dsl/src/lib.rs index b044c841..2a737cb8 100644 --- a/dsl/src/lib.rs +++ b/dsl/src/lib.rs @@ -37,7 +37,12 @@ pub mod model; pub mod parser; +pub mod schema; #[allow(unused_imports)] pub use model::*; pub use parser::Parser; +pub use schema::{ + ColumnSpec, ConfigSpec, IndexSpec, OperationSpec, PartitionKeySpec, QueriesSpec, Relation, Schema, infer_relations, + schemas_to_mermaid, +}; diff --git a/dsl/src/model/index.rs b/dsl/src/model/index.rs index b133c3b6..53c17be6 100644 --- a/dsl/src/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/dsl/src/model/persistence.rs b/dsl/src/model/persistence.rs index bef59fa5..ade72936 100644 --- a/dsl/src/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/dsl/src/model/primary_key.rs b/dsl/src/model/primary_key.rs index bbcb6441..d6d3bcb1 100644 --- a/dsl/src/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/dsl/src/schema/emit_dsl.rs b/dsl/src/schema/emit_dsl.rs new file mode 100644 index 00000000..8b881c1a --- /dev/null +++ b/dsl/src/schema/emit_dsl.rs @@ -0,0 +1,168 @@ +//! 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. `parse_updates` consumes one if it is + // there, but `parse_deletes` and `parse_in_place` do not, so a comma after + // either of those blocks reaches the `queries` dispatch loop as a `,` + // token and dies as "Unexpected identifier". Omitting it is accepted by + // all three, which makes it the only form that is always valid. + 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..7cf14057 --- /dev/null +++ b/dsl/src/schema/mod.rs @@ -0,0 +1,381 @@ +//! 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 emit_dsl; +mod emit_uml; + +pub use emit_uml::{Relation, infer_relations, schemas_to_mermaid}; + +/// 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", + )); + } + _ => return Err(syn::Error::new(ident.span(), "Unexpected identifier")), + } + } + + 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() + } +} diff --git a/dsl/tests/round_trip.rs b/dsl/tests/round_trip.rs new file mode 100644 index 00000000..e343b143 --- /dev/null +++ b/dsl/tests/round_trip.rs @@ -0,0 +1,180 @@ +//! 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. +//! +//! Both delimiter forms appear in the corpus (`worktable! { .. }` and +//! `worktable!( .. )`), so this accepts either. +//! +//! 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. + +use std::fs; +use std::path::{Path, PathBuf}; +use std::str::FromStr as _; + +use proc_macro2::{Delimiter, TokenStream, TokenTree}; +use worktable_dsl::Schema; + +/// Pull every `worktable! { .. }` body out of a token stream, including the +/// ones nested inside modules, functions and other macros. +/// +/// This walks tokens rather than using `syn`'s item tree because an invocation +/// inside a function body is not an item, and several of the corpus files put +/// one there. +fn collect_invocations(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_invocations(group.stream(), found); + } + index += 1; + } +} + +/// Whether a body is a `macro_rules!` template rather than a declaration. +/// +/// A dozen of the corpus's invocations sit inside `macro_rules!` and read +/// `name: $name, ... using $backend`. Those are not schemas: the metavariables +/// stand for text that only exists once the outer macro expands, and no parser +/// for this grammar can or should accept them. +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, + }) +} +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 declarations = Vec::new(); + let mut templates = 0; + for file in &files { + let Ok(contents) = fs::read_to_string(file) else { + continue; + }; + if !contents.contains("worktable!") { + continue; + } + let Ok(tokens) = TokenStream::from_str(&contents) else { + continue; + }; + let mut found = Vec::new(); + collect_invocations(tokens, &mut found); + for body in found { + if is_macro_template(&body) { + templates += 1; + continue; + } + declarations.push((file.clone(), body)); + } + } + + assert!( + declarations.len() >= 100, + "expected the repository's declarations to be found, got {}", + declarations.len() + ); + + let mut unparsed = Vec::new(); + let mut checked = 0; + for (file, body) in declarations { + let source = body.to_string(); + let Ok(schema) = Schema::from_tokens(body) else { + unparsed.push((file, source)); + continue; + }; + + 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!( + unparsed.is_empty(), + "{} declaration(s) the parser rejected:\n{}", + unparsed.len(), + unparsed + .iter() + .map(|(file, source)| format!(" {}: {source}", file.display())) + .collect::>() + .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..f11f5233 --- /dev/null +++ b/dsl/tests/schema.rs @@ -0,0 +1,258 @@ +//! What the IR and the emitters promise, stated one claim per test. + +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 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,")); +} + +#[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()); +} + +#[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()); +} From 87214e2bed7f566c2789473542d8afb3e6001698 Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 1 Sep 2026 20:38:01 +0700 Subject: [PATCH 4/6] Say what changed between two schemas, and what it costs With a schema as data on both sides, the decision a version mismatch forces can be computed instead of hand-written. `Diff::between` says what changed, `Cost` says what applying it costs, and `transforms_required` says which parts a person still has to write, which are the parts that need intent rather than mechanism. The cost model 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 what a change costs is not how many columns moved but whether a row is still where it was. Any change to the archived layout invalidates every link in the table at once and there is no cheaper answer than writing every row somewhere else. A change to an index invalidates nothing: the rows have not moved and the index can be rebuilt from them. Hence three tiers and a fourth for the changes no diff can settle. That last tier is the point of the exercise. A changed primary key, a changed partition key, a renamed table and a flipped `persist` are not expensive, they are underdetermined, and the useful thing a planner can do is say so rather than guess. The routing key is the clearest case: it is not in the row, so which partition a row belongs to cannot be recomputed from the row, only from where the row already is. The planner invents a value only when there is exactly one it could be. Widening a column to `optional` has one answer. Narrowing it does not, and neither does adding a required column or changing a type, so each of those comes back as a `TransformRequest` naming the column and why. A rename is reported as a drop and an add, because nothing in a declaration distinguishes it from a deletion beside an unrelated addition, and guessing by type equality would be wrong exactly when it mattered. Three things worth knowing that the tests state as claims. A version bump on its own costs nothing, which is what keeps bumping cheap enough to be habitual. Reordering columns is a layout change, because declaration order is the row struct's field order: it is the change most likely to be made by accident and least likely to look like one. And a schema that changed without a version bump is still detected, at the cost of comparing two small structs and reading no rows, which is the middle branch of the load state machine. `plan` lifts the same comparison to a set of tables, matching by name because that is how spaces are matched on disk. A dropped table is `NeedsIntent` rather than free: whether to delete data is a decision, not a consequence of a declaration. --- dsl/src/lib.rs | 1 + dsl/src/schema/diff.rs | 636 +++++++++++++++++++++++++++++++++++++++++ dsl/src/schema/mod.rs | 18 ++ dsl/tests/diff.rs | 387 +++++++++++++++++++++++++ 4 files changed, 1042 insertions(+) create mode 100644 dsl/src/schema/diff.rs create mode 100644 dsl/tests/diff.rs diff --git a/dsl/src/lib.rs b/dsl/src/lib.rs index 2a737cb8..e93f3e75 100644 --- a/dsl/src/lib.rs +++ b/dsl/src/lib.rs @@ -42,6 +42,7 @@ pub mod schema; #[allow(unused_imports)] pub use model::*; pub use parser::Parser; +pub use schema::{Change, Cost, Diff, TableChange, TransformReason, TransformRequest, plan}; pub use schema::{ ColumnSpec, ConfigSpec, IndexSpec, OperationSpec, PartitionKeySpec, QueriesSpec, Relation, Schema, infer_relations, schemas_to_mermaid, 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/mod.rs b/dsl/src/schema/mod.rs index 7cf14057..32641c12 100644 --- a/dsl/src/schema/mod.rs +++ b/dsl/src/schema/mod.rs @@ -48,9 +48,11 @@ use syn::spanned::Spanned as _; use crate::model::{Columns, GeneratorType, IndexBackend, Persistence, Queries}; use crate::parser::Parser; +mod diff; mod emit_dsl; mod emit_uml; +pub use diff::{Change, Cost, Diff, TableChange, TransformReason, TransformRequest, plan}; pub use emit_uml::{Relation, infer_relations, schemas_to_mermaid}; /// One `worktable!` declaration, as data. @@ -379,3 +381,19 @@ impl Schema { 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/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")); +} From 8f0dad041e21e36a850c6f785a0335b3433b6b3d Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 1 Sep 2026 21:00:13 +0700 Subject: [PATCH 5/6] Bake each table's declaration into the code it generates A compiled binary could not say what schema it was built against. The information was there at expansion time and then thrown away, so a migration planner had no "declared" side to compare against what is on disk, and a designer could not draw a diagram of an application whose source it did not have. Every generated table now carries a `_SCHEMA` const, named after `_VERSION` because it answers the question next to it: the version says which schema, and this says what that schema is. The stored form is the DSL text rather than a serialised structure. It needs no format decision, keeps serde out of the dependency graph of every user's build, is legible in a hex dump, and is read back by the same parser that read the original. It is also, being a declaration, exactly what regenerates an old table type, which is the hand-maintained `version_tables: { 1 => v1::UserV1WorkTable }` that makes migrations something people put off. `dsl/tests/round_trip.rs` holds the property this rests on against all 116 declarations in this repository, and the tests here check the emitted const against the declaration it came from and that the macro accepts it back. In-memory tables get it too. A designer reading a crate wants every table, and the const costs a string either way. Two details worth the words. The second parse runs at the end of `expand` rather than the start, so this function's diagnostics stay 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. And the const is `allow(dead_code)`, because a `worktable!` inside a function body puts it inside that body, where nothing refers to it and `-D warnings` would fail a user's build over a const they never asked for. --- codegen/src/common/name_generator.rs | 12 +++ codegen/src/worktable/mod.rs | 125 +++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) 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/worktable/mod.rs b/codegen/src/worktable/mod.rs index ca671904..909fcc6e 100644 --- a/codegen/src/worktable/mod.rs +++ b/codegen/src/worktable/mod.rs @@ -2,8 +2,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; @@ -84,9 +93,42 @@ pub fn expand(input: TokenStream) -> syn::Result { generated.extend(crate::generators::partitions::expand(&name, &key, persistence)); } + generated.extend(gen_schema_const(&worktable_dsl::Schema::from_tokens(declaration)?)); + Ok(generated) } +/// 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; + } +} + /// 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 @@ -871,3 +913,86 @@ mod generator_determinism { 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"); + } +} From ede0f479438a694ec6f82c645351dd60f29bf1d6 Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 1 Sep 2026 21:03:43 +0700 Subject: [PATCH 6/6] Find the declarations in a source tree 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. `declarations_in_source` walks tokens rather than `syn`'s item tree, because an invocation inside a function body is not an item and real code puts them there. An item walk would quietly miss a table, and the caller would never learn a table existed to be missed. Both delimiter forms are accepted: the repository uses `worktable!( .. )` 83 times and `worktable! { .. }` 45 times, and a reader that took only one would be wrong about the language. The return is not a `Vec`. Some invocations are not declarations: a `macro_rules!` body writing `name: $name, ... using $backend` is a template whose metavariables stand for text that exists only after the outer macro expands, and counting those as failures would be wrong. 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. The corpus round-trip test now goes through this, so it is also the evidence that the scanner finds what is there: 116 declarations read, 12 templates set aside, nothing rejected, across the whole repository. --- dsl/src/lib.rs | 6 +- dsl/src/schema/mod.rs | 2 + dsl/src/schema/scan.rs | 114 +++++++++++++++++++++++++++++++++++ dsl/tests/round_trip.rs | 128 +++++++++++----------------------------- dsl/tests/schema.rs | 65 +++++++++++++++++++- 5 files changed, 216 insertions(+), 99 deletions(-) create mode 100644 dsl/src/schema/scan.rs diff --git a/dsl/src/lib.rs b/dsl/src/lib.rs index e93f3e75..bbcfcc88 100644 --- a/dsl/src/lib.rs +++ b/dsl/src/lib.rs @@ -42,8 +42,8 @@ pub mod schema; #[allow(unused_imports)] pub use model::*; pub use parser::Parser; -pub use schema::{Change, Cost, Diff, TableChange, TransformReason, TransformRequest, plan}; pub use schema::{ - ColumnSpec, ConfigSpec, IndexSpec, OperationSpec, PartitionKeySpec, QueriesSpec, Relation, Schema, infer_relations, - schemas_to_mermaid, + Change, ColumnSpec, ConfigSpec, Cost, Declarations, Diff, IndexSpec, OperationSpec, PartitionKeySpec, QueriesSpec, + Relation, Schema, TableChange, TransformReason, TransformRequest, declarations_in_source, declarations_in_tokens, + infer_relations, plan, schemas_to_mermaid, }; diff --git a/dsl/src/schema/mod.rs b/dsl/src/schema/mod.rs index 32641c12..468ae335 100644 --- a/dsl/src/schema/mod.rs +++ b/dsl/src/schema/mod.rs @@ -51,9 +51,11 @@ use crate::parser::Parser; mod diff; mod emit_dsl; mod emit_uml; +mod scan; pub use diff::{Change, Cost, Diff, TableChange, TransformReason, TransformRequest, plan}; 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)] 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/tests/round_trip.rs b/dsl/tests/round_trip.rs index e343b143..6fc04d55 100644 --- a/dsl/tests/round_trip.rs +++ b/dsl/tests/round_trip.rs @@ -6,63 +6,20 @@ //! better corpus than anything written here would be: they use the grammar the //! way it is really used, including the corners. //! -//! Both delimiter forms appear in the corpus (`worktable! { .. }` and -//! `worktable!( .. )`), so this accepts either. -//! //! 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 std::str::FromStr as _; -use proc_macro2::{Delimiter, TokenStream, TokenTree}; -use worktable_dsl::Schema; +use worktable_dsl::{Schema, declarations_in_source}; -/// Pull every `worktable! { .. }` body out of a token stream, including the -/// ones nested inside modules, functions and other macros. -/// -/// This walks tokens rather than using `syn`'s item tree because an invocation -/// inside a function body is not an item, and several of the corpus files put -/// one there. -fn collect_invocations(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_invocations(group.stream(), found); - } - index += 1; - } -} - -/// Whether a body is a `macro_rules!` template rather than a declaration. -/// -/// A dozen of the corpus's invocations sit inside `macro_rules!` and read -/// `name: $name, ... using $backend`. Those are not schemas: the metavariables -/// stand for text that only exists once the outer macro expands, and no parser -/// for this grammar can or should accept them. -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, - }) -} fn rust_files(root: &Path, out: &mut Vec) { let Ok(entries) = fs::read_dir(root) else { return }; for entry in entries.flatten() { @@ -88,8 +45,10 @@ fn every_declaration_in_the_repository_survives_a_round_trip() { files.sort(); assert!(!files.is_empty(), "found no sources to read"); - let mut declarations = Vec::new(); + 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; @@ -97,67 +56,46 @@ fn every_declaration_in_the_repository_survives_a_round_trip() { if !contents.contains("worktable!") { continue; } - let Ok(tokens) = TokenStream::from_str(&contents) else { + let Ok(found) = declarations_in_source(&contents) else { continue; }; - let mut found = Vec::new(); - collect_invocations(tokens, &mut found); - for body in found { - if is_macro_template(&body) { - templates += 1; - continue; - } - declarations.push((file.clone(), body)); - } - } - - assert!( - declarations.len() >= 100, - "expected the repository's declarations to be found, got {}", - declarations.len() - ); - let mut unparsed = Vec::new(); - let mut checked = 0; - for (file, body) in declarations { - let source = body.to_string(); - let Ok(schema) = Schema::from_tokens(body) else { - unparsed.push((file, source)); - continue; - }; + templates += found.templates.len(); + for (source, error) in found.rejected { + rejected.push(format!(" {}: {error}\n {source}", file.display())); + } - 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}", + 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() - ) - }); - assert_eq!( - schema, - reparsed, - "round trip changed `{}` from {}\n{emitted}", - schema.name, - file.display() - ); - checked += 1; + ); + checked += 1; + } } assert!( - unparsed.is_empty(), + rejected.is_empty(), "{} declaration(s) the parser rejected:\n{}", - unparsed.len(), - unparsed - .iter() - .map(|(file, source)| format!(" {}: {source}", file.display())) - .collect::>() - .join("\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}" + "the `macro_rules!` templates stopped being found, so the filter is now hiding \ + something else: {templates}" ); } diff --git a/dsl/tests/schema.rs b/dsl/tests/schema.rs index f11f5233..61c3bab8 100644 --- a/dsl/tests/schema.rs +++ b/dsl/tests/schema.rs @@ -1,6 +1,6 @@ //! What the IR and the emitters promise, stated one claim per test. -use worktable_dsl::{Schema, infer_relations, schemas_to_mermaid}; +use worktable_dsl::{Schema, declarations_in_source, infer_relations, schemas_to_mermaid}; fn parse(source: &str) -> Schema { Schema::parse(source).unwrap_or_else(|error| panic!("{error}\n{source}")) @@ -256,3 +256,66 @@ fn a_schema_survives_a_trip_through_serde() { 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()); + assert!(found.rejected[0].1.to_string().contains("Unexpected identifier")); +}