From 51b51c68fcee2b1340e9880b4c5f1dbc2964ed6f Mon Sep 17 00:00:00 2001 From: meh Date: Tue, 1 Sep 2026 14:04:51 +0700 Subject: [PATCH 1/2] 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 +- .../process_insert_at_big_amount.wt.idx | Bin 65536 -> 65536 bytes .../process_insert_at_big_amount.wt.idx | Bin 49152 -> 49152 bytes .../process_remove_at_node_id.wt.idx | Bin 49152 -> 49152 bytes .../process_split_node.wt.idx | Bin 65536 -> 65536 bytes tests/worktable/nonunique_arctic.rs | 4 +- tests/worktable/partitioned.rs | 6 +- 42 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 17e90cfd..4eb912bb 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/data/space_index_unsized/indexset/process_insert_at_big_amount.wt.idx b/tests/data/space_index_unsized/indexset/process_insert_at_big_amount.wt.idx index 94bd08f0ee809bdbaa4ccd48b8408876b7517b75..bd2fc259d35f44ba6a97b31d0c593baaa49ccbec 100644 GIT binary patch delta 18 ZcmZo@U}A(O1E*lf)+fNi=S=8VF0CP?U&j0`b diff --git a/tests/data/space_index_unsized/process_split_node.wt.idx b/tests/data/space_index_unsized/process_split_node.wt.idx index 77b2488a749b07570a4bd03c6bf72b0bedc71b33..b01ebf726562db9c2149c47f8a8b58fa97b10292 100644 GIT binary patch delta 18 ZcmZo@U} Date: Tue, 1 Sep 2026 20:40:19 +0700 Subject: [PATCH 2/2] fix(codegen): drop the unused glob re-export `pub use worktable_dsl::{Parser, *}` failed clippy under `-D warnings` with "unused import: `*`". `worktable_codegen` is a proc-macro crate, so its `pub use` re-exports are not reachable from outside it -- the glob was only ever visible within this crate, and nothing here needed what it brought in beyond the `model` and `parser` modules the next line already re-exports. Naming `Parser` alone keeps every `crate::common::` path working. --- codegen/src/common/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen/src/common/mod.rs b/codegen/src/common/mod.rs index 298d1f4c..99a83229 100644 --- a/codegen/src/common/mod.rs +++ b/codegen/src/common/mod.rs @@ -10,5 +10,5 @@ //! 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::Parser; pub use worktable_dsl::{model, parser};