diff --git a/AGENTS.md b/AGENTS.md index 58603d2..3861947 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,6 +75,16 @@ Every level reports through one vocabulary in `problem.rs`: a `Problem` (a `code Test fixtures for the spec rules are in `crates/data-dict/tests/fixtures/{valid,invalid,spec}/`. Each fixture has a `# expected: ...` header documenting the intended outcome. Integration tests mirror the levels: `tests/validate_spec.rs` / `validate_meta.rs` / `validate_data.rs`. +Every test that asserts a diagnostic error or warning must also include a snapshot assertion: + +```rust +diagnostic.assert_contains(&["S07", "expected phrase"]); +#[cfg(unix)] +assert_snapshot!(diagnostic); +``` + +After adding new snapshot assertions, generate them with `cargo insta test -p data-dict --test validate_spec`, inspect the `.snap.new` files to confirm they look right, then accept with `cargo insta accept --workspace`. + ### Problem reporting Two principles guide how problems are surfaced: diff --git a/crates/data-dict-parquet/src/metadata.rs b/crates/data-dict-parquet/src/metadata.rs index 6c544d2..dbdba5d 100644 --- a/crates/data-dict-parquet/src/metadata.rs +++ b/crates/data-dict-parquet/src/metadata.rs @@ -144,10 +144,16 @@ fn parquet_type_to_dict_type(field: &Type) -> String { LogicalType::Integer { .. } | LogicalType::Float16 | LogicalType::Decimal { .. } => { return "number".into(); } + LogicalType::List => return parquet_list_to_dict_type(field), _ => {} } } + // Group types with no list logical annotation are structs. + if field.is_group() { + return "struct".into(); + } + match field.get_physical_type() { PhysicalType::BOOLEAN => "boolean".into(), PhysicalType::INT32 | PhysicalType::INT64 => "number".into(), @@ -156,3 +162,24 @@ fn parquet_type_to_dict_type(field: &Type) -> String { PhysicalType::BYTE_ARRAY | PhysicalType::FIXED_LEN_BYTE_ARRAY => "string".into(), } } + +/// Map a Parquet LIST-annotated group field to a `list(element_type)` string. +/// Parquet encodes lists as `list`; +/// we descend through the standard wrapper to find the element field. +fn parquet_list_to_dict_type(field: &Type) -> String { + let elem_type = parquet_list_element(field) + .map(|e| parquet_type_to_dict_type(e)) + .unwrap_or_else(|| "string".into()); + format!("list({elem_type})") +} + +/// Navigate the standard Parquet LIST encoding to the element field: +/// repeated group (LIST) { repeated group list { optional element } } +/// Returns `None` when the structure deviates from the standard layout. +fn parquet_list_element(field: &Type) -> Option<&Type> { + let fields = field.get_fields(); + // The outer LIST group has one repeated child ("list" or similar). + let middle = fields.first()?; + // That middle group has one child — the element. + middle.get_fields().first().map(|f| f.as_ref()) +} diff --git a/crates/data-dict/src/lower.rs b/crates/data-dict/src/lower.rs index 35a3ff6..15bb5bf 100644 --- a/crates/data-dict/src/lower.rs +++ b/crates/data-dict/src/lower.rs @@ -97,6 +97,7 @@ fn lower_column(node: &YamlWithSourceInfo) -> Option { let mut examples: Option = None; let mut units: Option> = None; let mut time_zone: Option> = None; + let mut fields: Option> = None; for entry in entries { let Some(key) = entry.key.yaml.as_str() else { continue; @@ -147,6 +148,17 @@ fn lower_column(node: &YamlWithSourceInfo) -> Option { } } } + "fields" => { + if let Some(items) = entry.value.as_array() { + let mut fs = Vec::new(); + for f in items { + if let Some(col) = lower_column(f) { + fs.push(col); + } + } + fields = Some(fs); + } + } _ => {} } } @@ -159,6 +171,7 @@ fn lower_column(node: &YamlWithSourceInfo) -> Option { examples, units, time_zone, + fields, }) } diff --git a/crates/data-dict/src/model.rs b/crates/data-dict/src/model.rs index 5adcd11..4f8c465 100644 --- a/crates/data-dict/src/model.rs +++ b/crates/data-dict/src/model.rs @@ -73,6 +73,10 @@ pub struct Column { pub examples: Option, pub units: Option>, pub time_zone: Option>, + /// Fields for `struct` and `list(struct)` columns. `None` means the + /// `fields` key was absent; `Some` means it was present (possibly empty, + /// which S07 rejects). + pub fields: Option>, } #[derive(Debug, Clone)] diff --git a/crates/data-dict/src/validate_spec.rs b/crates/data-dict/src/validate_spec.rs index 686cb24..65632c4 100644 --- a/crates/data-dict/src/validate_spec.rs +++ b/crates/data-dict/src/validate_spec.rs @@ -21,7 +21,7 @@ use quarto_yaml_validation::error::ValidationErrorKind; use quarto_yaml_validation::{Schema, SchemaRegistry, ValidationDiagnostic, ValidationError}; use crate::join_expr::{JoinExpr, QCol}; -use crate::model::{Cardinality, Column, DataDict, Scalar, Spanned, Table}; +use crate::model::{Cardinality, Column, Constraint, DataDict, Scalar, Spanned, Table}; use crate::problem::{Problem, ProblemKind, ProblemSet, Suggestion, subspan}; use crate::{SourceContext, lower}; @@ -180,7 +180,7 @@ pub(crate) fn validate_and_lower( /// check runs only when the general one it refines passed: a malformed `name` /// blocks the uniqueness check, and the representation chain narrows from "the /// right key is present" (S07) to "its values have the right type" (S12) to "the -/// range is ordered" (S13). +/// range is ordered" (S13). Struct columns recurse into their fields. fn check_spec(dict: &DataDict, out: &mut ProblemSet) { validate_s02_relationship_table_refs(dict, out); validate_s03_relationship_column_refs(dict, out); @@ -194,21 +194,43 @@ fn check_spec(dict: &DataDict, out: &mut ProblemSet) { if validate_s11_table_name(table, out) { validate_s10_unique_table_name(table, &mut seen_tables, out); } - let mut seen: HashMap = HashMap::new(); - for col in &table.columns { + check_columns(dict, table, &table.columns, false, out); + } +} + +/// Run all column-level checks for a slice of columns. `in_struct` is `true` +/// when the columns are fields inside a `struct`, which changes S01 (skipped) +/// and S20 (always fires for primary_key/foreign_key). +fn check_columns( + dict: &DataDict, + table: &Table, + columns: &[Column], + in_struct: bool, + out: &mut ProblemSet, +) { + let mut seen: HashMap = HashMap::new(); + for col in columns { + if !in_struct { validate_s01_foreign_key(dict, table, col, out); - validate_s08_units(table, col, out); - validate_s14_time_zone(table, col, out); - validate_s15_time_zone_format(table, col, out); - if validate_s11_column_name(table, col, out) { - validate_s10_unique_name(table, col, &mut seen, out); - } + } + validate_s08_units(table, col, out); + validate_s14_time_zone(table, col, out); + validate_s15_time_zone_format(table, col, out); + validate_s20_key_constraints(table, col, in_struct, out); + if validate_s11_column_name(table, col, out) { + validate_s10_unique_name(table, col, &mut seen, out); + } + if validate_s19_type(table, col, out) { if validate_s07_representation(table, col, out) && validate_s12_value_types(table, col, out) { validate_s13_range_order(table, col, out); } } + // Recurse into struct fields (covers both `struct` and `list(struct)`). + if let Some(fields) = &col.fields { + check_columns(dict, table, fields, true, out); + } } } @@ -493,10 +515,127 @@ fn side_has_unique_implied( }) } +// --- Type helpers ----------------------------------------------------- + +/// The fixed scalar and composite type names (excluding list variants). +const KNOWN_TYPES: &[&str] = &[ + "string", + "number", + "number(id)", + "number(ordinal)", + "number(quantity)", + "boolean", + "date", + "datetime", + "enum", + "struct", +]; + +/// If `type_name` is `list(element_type)`, returns the element type string. +fn list_element_type(type_name: &str) -> Option<&str> { + type_name.strip_prefix("list(")?.strip_suffix(")") +} + +/// Whether `type_name` is a recognised type string (fixed scalar, `struct`, or +/// `list(element_type)` where the element type is itself a known fixed type). +fn is_valid_type(type_name: &str) -> bool { + if KNOWN_TYPES.contains(&type_name) { + return true; + } + if let Some(elem) = list_element_type(type_name) { + return KNOWN_TYPES.contains(&elem); + } + false +} + +// --- S19 -------------------------------------------------------------- + +/// Validate that the column's type string, if present, is a known type. +/// Returns `true` when the type is absent (name-only column) or valid, so +/// that callers can gate further checks on the result. +fn validate_s19_type(table: &Table, col: &Column, out: &mut ProblemSet) -> bool { + let Some(col_type) = &col.col_type else { + return true; + }; + if is_valid_type(&col_type.value) { + return true; + } + let message = if let Some(elem) = list_element_type(&col_type.value) { + format!("`{elem}` is not a recognised list element type") + } else { + format!("`{}` is not a recognised type", col_type.value) + }; + out.push_spec_error( + "S19", + "A column's `type` must be a known type.", + message, + [ + table.name.span.clone(), + col.name.span.clone(), + col_type.span.clone(), + ], + ); + false +} + +// --- S20 -------------------------------------------------------------- + +/// Error when `primary_key` or `foreign_key` appears on a `list` or `struct` +/// column, or on any field inside a `struct`. +fn validate_s20_key_constraints( + table: &Table, + col: &Column, + in_struct: bool, + out: &mut ProblemSet, +) { + let type_name = col + .col_type + .as_ref() + .map(|t| t.value.as_str()) + .unwrap_or(""); + let is_list_or_struct = list_element_type(type_name).is_some() || type_name == "struct"; + if !in_struct && !is_list_or_struct { + return; + } + for c in &col.constraints { + if matches!(c.value, Constraint::PrimaryKey | Constraint::ForeignKey) { + let constraint_name = match c.value { + Constraint::PrimaryKey => "primary_key", + Constraint::ForeignKey => "foreign_key", + _ => unreachable!(), + }; + let context = if in_struct { + "struct fields".to_string() + } else { + format!("`{type_name}` columns") + }; + out.push_spec_error( + "S20", + format!("`{constraint_name}` is not valid on {context}."), + format!("has `{constraint_name}`"), + [ + table.name.span.clone(), + col.name.span.clone(), + c.span.clone(), + ], + ); + } + } +} + // --- S07 -------------------------------------------------------------- const RANGE_TYPES: &[&str] = &["number(ordinal)", "number(quantity)", "date", "datetime"]; +/// "A" or "An" before a backtick-quoted type name, based on the first letter +/// of the unquoted name (not the backtick). +fn article(type_name: &str) -> &'static str { + match type_name.chars().next() { + Some(c) if "aeiouAEIOU".contains(c) => "An", + _ => "A", + } +} + /// Returns whether the column carries the representation its type requires and /// no other — i.e. whether checking that representation's values (S12) makes /// sense. @@ -515,11 +654,45 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet let at = |span: &SourceInfo| [table.name.span.clone(), col.name.span.clone(), span.clone()]; let before = out.items.len(); - if type_name == "enum" { + + // For list types, delegate representation rules to the element type and + // check fields only for list(struct). + let (effective_type, is_list) = match list_element_type(type_name) { + Some(elem) => (elem, true), + None => (type_name, false), + }; + + let art = article(type_name); + + // `struct` and `list(struct)` require `fields` and no representation keys. + if effective_type == "struct" { + if col.fields.is_none() { + out.push_spec_error( + "S07", + format!("{art} `{type_name}` column must document its fields with `fields`."), + missing("fields"), + at(&col_type.span), + ); + } + for (span, key) in [ + (col.values.as_ref(), "values"), + (col.range.as_ref().map(|r| &r.span), "range"), + (col.examples.as_ref().map(|e| &e.span), "examples"), + ] { + if let Some(span) = span { + out.push_spec_error( + "S07", + format!("{art} `{type_name}` column must not use `{key}`."), + found(key), + at(span), + ); + } + } + } else if effective_type == "enum" { if col.values.is_none() { out.push_spec_error( "S07", - "An `enum` column must list its categories with `values`.", + format!("{art} `{type_name}` column must list its categories with `values`."), missing("values"), at(&col_type.span), ); @@ -527,7 +700,7 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet if let Some(range) = &col.range { out.push_spec_error( "S07", - "An `enum` column must use `values`, not `range`.", + format!("{art} `{type_name}` column must use `values`, not `range`."), found("range"), at(&range.span), ); @@ -535,16 +708,16 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet if let Some(examples) = &col.examples { out.push_spec_error( "S07", - "An `enum` column must use `values`, not `examples`.", + format!("{art} `{type_name}` column must use `values`, not `examples`."), found("examples"), at(&examples.span), ); } - } else if RANGE_TYPES.contains(&type_name) { + } else if RANGE_TYPES.contains(&effective_type) { if col.range.is_none() { out.push_spec_error( "S07", - format!("A `{type_name}` column must describe its bounds with `range`."), + format!("{art} `{type_name}` column must describe its bounds with `range`."), missing("range"), at(&col_type.span), ); @@ -552,7 +725,7 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet if let Some(values) = &col.values { out.push_spec_error( "S07", - format!("A `{type_name}` column must use `range`, not `values`."), + format!("{art} `{type_name}` column must use `range`, not `values`."), found("values"), at(values), ); @@ -560,12 +733,13 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet if let Some(examples) = &col.examples { out.push_spec_error( "S07", - format!("A `{type_name}` column must use `range`, not `examples`."), + format!("{art} `{type_name}` column must use `range`, not `examples`."), found("examples"), at(&examples.span), ); } - } else if type_name == "boolean" { + } else if effective_type == "boolean" { + // Neither scalar boolean nor list(boolean) takes representation keys. for (span, key) in [ (col.values.as_ref(), "values"), (col.range.as_ref().map(|r| &r.span), "range"), @@ -581,10 +755,11 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet } } } else { + // string, number, number(id), and list(boolean): examples required. if col.examples.is_none() { out.push_spec_error( "S07", - format!("A `{type_name}` column must describe its data with `examples`."), + format!("{art} `{type_name}` column must describe its data with `examples`."), missing("examples"), at(&col_type.span), ); @@ -592,7 +767,7 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet if let Some(values) = &col.values { out.push_spec_error( "S07", - format!("A `{type_name}` column must not use `values`."), + format!("{art} `{type_name}` column must not use `values`."), found("values"), at(values), ); @@ -600,12 +775,30 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet if let Some(range) = &col.range { out.push_spec_error( "S07", - format!("A `{type_name}` column must not use `range`."), + format!("{art} `{type_name}` column must not use `range`."), found("range"), at(&range.span), ); } } + + // `fields` is only valid on struct and list(struct) columns. + if effective_type != "struct" { + if let Some(fields_span) = col + .fields + .as_ref() + .and_then(|f| f.first()) + .map(|f| &f.name.span) + { + out.push_spec_error( + "S07", + format!("{art} `{type_name}` column must not use `fields`."), + found("fields"), + at(fields_span), + ); + } + } + out.items.len() == before } @@ -613,10 +806,10 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet fn validate_s08_units(table: &Table, col: &Column, out: &mut ProblemSet) { let Some(units) = &col.units else { return }; - let is_quantity = col - .col_type - .as_ref() - .is_some_and(|t| t.value == "number(quantity)"); + let is_quantity = col.col_type.as_ref().is_some_and(|t| { + let effective = list_element_type(&t.value).unwrap_or(&t.value); + effective == "number(quantity)" + }); if is_quantity { return; } @@ -795,15 +988,21 @@ fn validate_s11_column_name(table: &Table, col: &Column, out: &mut ProblemSet) - /// The representation list whose values are type-checked for a given column /// type, or `None` for types that carry no typed representation (`enum`, -/// `boolean`, and any unrecognized type). Mirrors S07: each type owns exactly -/// one representation key, and we only check the one it owns so that a -/// misplaced key reports as S07 rather than cascading into S12. -fn typed_representation(col: &Column) -> Option<(&'static str, &[Spanned])> { - match col.col_type.as_ref()?.value.as_str() { +/// `boolean`, `struct`, `list(struct)`, and any unrecognized type). Mirrors +/// S07: each type owns exactly one representation key, and we only check the +/// one it owns so that a misplaced key reports as S07 rather than cascading +/// into S12. For list types the element type determines the key and the +/// expected value kind. +fn typed_representation(col: &Column) -> Option<(&'static str, &str, &[Spanned])> { + let type_name = col.col_type.as_ref()?.value.as_str(); + let effective = list_element_type(type_name).unwrap_or(type_name); + match effective { "number(ordinal)" | "number(quantity)" | "date" | "datetime" => { - Some(("range", &col.range.as_ref()?.items)) + Some(("range", effective, &col.range.as_ref()?.items)) + } + "string" | "number" | "number(id)" => { + Some(("examples", effective, &col.examples.as_ref()?.items)) } - "string" | "number" | "number(id)" => Some(("examples", &col.examples.as_ref()?.items)), _ => None, } } @@ -814,13 +1013,13 @@ fn validate_s12_value_types(table: &Table, col: &Column, out: &mut ProblemSet) - let Some(type_name) = col.col_type.as_ref().map(|t| t.value.as_str()) else { return true; }; - let Some((key, values)) = typed_representation(col) else { + let Some((key, effective_type, values)) = typed_representation(col) else { return true; }; let tz_present = col.time_zone.is_some(); let mut ok = true; for v in values { - if value_matches_type(type_name, &v.value, tz_present) { + if value_matches_type(effective_type, &v.value, tz_present) { continue; } ok = false; @@ -830,7 +1029,7 @@ fn validate_s12_value_types(table: &Table, col: &Column, out: &mut ProblemSet) - "Each `{}` value of a `{}` column must be {}.", key, type_name, - expected_noun(type_name, tz_present), + expected_noun(effective_type, tz_present), ), format!("is {}", v.value.noun()), [ @@ -911,11 +1110,12 @@ fn validate_s13_range_order(table: &Table, col: &Column, out: &mut ProblemSet) { return; }; let Some(range) = &col.range else { return }; - if !RANGE_TYPES.contains(&type_name) || range.items.len() != 2 { + let effective = list_element_type(type_name).unwrap_or(type_name); + if !RANGE_TYPES.contains(&effective) || range.items.len() != 2 { return; } let (lo, hi) = (&range.items[0], &range.items[1]); - if range_descending(type_name, &lo.value, &hi.value, col.time_zone.is_some()) { + if range_descending(effective, &lo.value, &hi.value, col.time_zone.is_some()) { out.push_spec_error( "S13", "A range's minimum must be less than or equal to its maximum.", diff --git a/crates/data-dict/tests/snapshots/validate_spec__s07_fields_on_non_struct.snap b/crates/data-dict/tests/snapshots/validate_spec__s07_fields_on_non_struct.snap new file mode 100644 index 0000000..bb5d740 --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__s07_fields_on_non_struct.snap @@ -0,0 +1,25 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: tags + type: list(string) + examples: [a, b, c] + fields: + - name: x + type: string + examples: [a] +──────────────────────────────────────── +error[S07]: A `list(string)` column must not use `fields`. + --> dict.yaml:10:19 + | +LL | - name: t +LL | columns: +LL | - name: tags +... +LL | - name: x + | ^ has type `list(string)` but uses `fields` diff --git a/crates/data-dict/tests/snapshots/validate_spec__s07_list_missing_representation.snap b/crates/data-dict/tests/snapshots/validate_spec__s07_list_missing_representation.snap new file mode 100644 index 0000000..0f5f6a3 --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__s07_list_missing_representation.snap @@ -0,0 +1,19 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: tags + type: list(string) +──────────────────────────────────────── +error[S07]: A `list(string)` column must describe its data with `examples`. + --> dict.yaml:7:15 + | +LL | - name: t +LL | columns: +LL | - name: tags +LL | type: list(string) + | ^^^^^^^^^^^^ has type `list(string)` but is missing `examples` diff --git a/crates/data-dict/tests/snapshots/validate_spec__s07_struct_without_fields.snap b/crates/data-dict/tests/snapshots/validate_spec__s07_struct_without_fields.snap new file mode 100644 index 0000000..c30ff17 --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__s07_struct_without_fields.snap @@ -0,0 +1,19 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: addr + type: struct +──────────────────────────────────────── +error[S07]: A `struct` column must document its fields with `fields`. + --> dict.yaml:7:15 + | +LL | - name: t +LL | columns: +LL | - name: addr +LL | type: struct + | ^^^^^^ has type `struct` but is missing `fields` diff --git a/crates/data-dict/tests/snapshots/validate_spec__s19_invalid_list_element_type.snap b/crates/data-dict/tests/snapshots/validate_spec__s19_invalid_list_element_type.snap new file mode 100644 index 0000000..0ea7c71 --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__s19_invalid_list_element_type.snap @@ -0,0 +1,20 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: c + type: list(foo) + examples: [a, b, c] +──────────────────────────────────────── +error[S19]: A column's `type` must be a known type. + --> dict.yaml:7:15 + | +LL | - name: t +LL | columns: +LL | - name: c +LL | type: list(foo) + | ^^^^^^^^^ `foo` is not a recognised list element type diff --git a/crates/data-dict/tests/snapshots/validate_spec__s19_invalid_type.snap b/crates/data-dict/tests/snapshots/validate_spec__s19_invalid_type.snap new file mode 100644 index 0000000..dd0058b --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__s19_invalid_type.snap @@ -0,0 +1,20 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: c + type: foobar + examples: [1, 2, 3] +──────────────────────────────────────── +error[S19]: A column's `type` must be a known type. + --> dict.yaml:7:15 + | +LL | - name: t +LL | columns: +LL | - name: c +LL | type: foobar + | ^^^^^^ `foobar` is not a recognised type diff --git a/crates/data-dict/tests/snapshots/validate_spec__s20_foreign_key_on_list.snap b/crates/data-dict/tests/snapshots/validate_spec__s20_foreign_key_on_list.snap new file mode 100644 index 0000000..fe9eae0 --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__s20_foreign_key_on_list.snap @@ -0,0 +1,31 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: tags + type: list(string) + constraints: [foreign_key] + examples: [a, b, c] +──────────────────────────────────────── +error[S01]: Every `foreign_key` column must have a matching relationship to a `primary_key`. + --> dict.yaml:8:23 + | +LL | - name: t +LL | columns: +LL | - name: tags +LL | type: list(string) +LL | constraints: [foreign_key] + | ^^^^^^^^^^^ is `foreign_key` but no relationship points it at a `primary_key` +error[S20]: `foreign_key` is not valid on `list(string)` columns. + --> dict.yaml:8:23 + | +LL | - name: t +LL | columns: +LL | - name: tags +LL | type: list(string) +LL | constraints: [foreign_key] + | ^^^^^^^^^^^ has `foreign_key` diff --git a/crates/data-dict/tests/snapshots/validate_spec__s20_primary_key_on_struct.snap b/crates/data-dict/tests/snapshots/validate_spec__s20_primary_key_on_struct.snap new file mode 100644 index 0000000..6ac86fb --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__s20_primary_key_on_struct.snap @@ -0,0 +1,25 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: addr + type: struct + constraints: [primary_key] + fields: + - name: street + type: string + examples: [123 Main St] +──────────────────────────────────────── +error[S20]: `primary_key` is not valid on `struct` columns. + --> dict.yaml:8:23 + | +LL | - name: t +LL | columns: +LL | - name: addr +LL | type: struct +LL | constraints: [primary_key] + | ^^^^^^^^^^^ has `primary_key` diff --git a/crates/data-dict/tests/snapshots/validate_spec__s20_primary_key_on_struct_field.snap b/crates/data-dict/tests/snapshots/validate_spec__s20_primary_key_on_struct_field.snap new file mode 100644 index 0000000..24ca060 --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__s20_primary_key_on_struct_field.snap @@ -0,0 +1,25 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: addr + type: struct + fields: + - name: id + type: number(id) + constraints: [primary_key] + examples: [1, 2, 3] +──────────────────────────────────────── +error[S20]: `primary_key` is not valid on struct fields. + --> dict.yaml:11:27 + | +LL | - name: t +... +LL | - name: id +LL | type: number(id) +LL | constraints: [primary_key] + | ^^^^^^^^^^^ has `primary_key` diff --git a/crates/data-dict/tests/snapshots/validate_spec__struct_field_s12_wrong_type.snap b/crates/data-dict/tests/snapshots/validate_spec__struct_field_s12_wrong_type.snap new file mode 100644 index 0000000..da46ddb --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_spec__struct_field_s12_wrong_type.snap @@ -0,0 +1,24 @@ +--- +source: crates/data-dict/tests/validate_spec.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + - name: t + columns: + - name: addr + type: struct + fields: + - name: zip + type: number(id) + examples: [not-a-number] +──────────────────────────────────────── +error[S12]: Each `examples` value of a `number(id)` column must be a number. + --> dict.yaml:11:24 + | +LL | - name: t +... +LL | - name: zip +LL | type: number(id) +LL | examples: [not-a-number] + | ^^^^^^^^^^^^ is a string diff --git a/crates/data-dict/tests/validate_spec.rs b/crates/data-dict/tests/validate_spec.rs index 808d66c..b39061a 100644 --- a/crates/data-dict/tests/validate_spec.rs +++ b/crates/data-dict/tests/validate_spec.rs @@ -890,3 +890,272 @@ fn s15_bad_time_zone() { #[cfg(unix)] assert_snapshot!(diagnostic); } + +// --- list and struct types (S07, S19, S20) -------------------------------- + +#[test] +fn struct_with_fields_ok() { + assert_clean_dict(indoc! {" + tables: + - name: deliveries + columns: + - name: id + type: number(id) + constraints: [primary_key] + examples: [1, 2, 3] + - name: address + type: struct + fields: + - name: street + type: string + examples: [123 Main St, 456 Oak Ave] + - name: zip + type: string + examples: [97201, 78701] + "}); +} + +#[test] +fn list_string_with_examples_ok() { + assert_clean_dict(indoc! {" + tables: + - name: posts + columns: + - name: id + type: number(id) + constraints: [primary_key] + examples: [1, 2, 3] + - name: tags + type: list(string) + examples: [nature, outdoor, urban] + "}); +} + +#[test] +fn list_enum_with_values_ok() { + assert_clean_dict(indoc! {" + tables: + - name: products + columns: + - name: id + type: number(id) + constraints: [primary_key] + examples: [1, 2, 3] + - name: categories + type: list(enum) + values: [food, drink, dessert] + "}); +} + +#[test] +fn list_quantity_with_range_ok() { + assert_clean_dict(indoc! {" + tables: + - name: orders + columns: + - name: id + type: number(id) + constraints: [primary_key] + examples: [1, 2, 3] + - name: prices + type: list(number(quantity)) + units: USD + range: [0.99, 999.99] + "}); +} + +#[test] +fn list_struct_with_fields_ok() { + assert_clean_dict(indoc! {" + tables: + - name: orders + columns: + - name: id + type: number(id) + constraints: [primary_key] + examples: [1, 2, 3] + - name: line_items + type: list(struct) + fields: + - name: product_id + type: number(id) + examples: [101, 204, 389] + - name: quantity + type: number(quantity) + units: units + range: [1, 100] + "}); +} + +#[test] +fn list_boolean_no_representation_ok() { + assert_clean_dict(indoc! {" + tables: + - name: t + columns: + - name: id + type: number(id) + constraints: [primary_key] + examples: [1, 2, 3] + - name: flags + type: list(boolean) + "}); +} + +// S19: list(foo) names the bad element type, not the whole list type. +#[test] +fn s19_invalid_list_element_type() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: c + type: list(foo) + examples: [a, b, c] + "}); + diagnostic.assert_contains(&["S19", "foo", "element type"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} + +// S19: an unrecognised type string is rejected. +#[test] +fn s19_invalid_type() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: c + type: foobar + examples: [1, 2, 3] + "}); + diagnostic.assert_contains(&["S19", "foobar"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} + +// S07: a struct column without fields is an error. +#[test] +fn s07_struct_without_fields() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: addr + type: struct + "}); + diagnostic.assert_contains(&["S07", "struct", "fields"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} + +// S07: `fields` on a non-struct column is an error. +#[test] +fn s07_fields_on_non_struct() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: tags + type: list(string) + examples: [a, b, c] + fields: + - name: x + type: string + examples: [a] + "}); + diagnostic.assert_contains(&["S07", "list(string)", "fields"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} + +// S07: a list(string) column without examples is an error. +#[test] +fn s07_list_missing_representation() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: tags + type: list(string) + "}); + diagnostic.assert_contains(&["S07", "list(string)", "examples"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} + +// S20: primary_key on a struct column is an error. +#[test] +fn s20_primary_key_on_struct() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: addr + type: struct + constraints: [primary_key] + fields: + - name: street + type: string + examples: [123 Main St] + "}); + diagnostic.assert_contains(&["S20", "primary_key", "struct"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} + +// S20: foreign_key on a list column is an error. +#[test] +fn s20_foreign_key_on_list() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: tags + type: list(string) + constraints: [foreign_key] + examples: [a, b, c] + "}); + diagnostic.assert_contains(&["S20", "foreign_key", "list(string)"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} + +// S20: primary_key on a struct field is always an error. +#[test] +fn s20_primary_key_on_struct_field() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: addr + type: struct + fields: + - name: id + type: number(id) + constraints: [primary_key] + examples: [1, 2, 3] + "}); + diagnostic.assert_contains(&["S20", "primary_key"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} + +// Struct fields are themselves validated (e.g. S12 catches wrong value types). +#[test] +fn struct_field_s12_wrong_type() { + let diagnostic = failing_dict(indoc! {" + tables: + - name: t + columns: + - name: addr + type: struct + fields: + - name: zip + type: number(id) + examples: [not-a-number] + "}); + diagnostic.assert_contains(&["S12"]); + #[cfg(unix)] + assert_snapshot!(diagnostic); +} diff --git a/schema.yaml b/schema.yaml index 07bbc83..c7edf8e 100644 --- a/schema.yaml +++ b/schema.yaml @@ -85,9 +85,11 @@ object: units: string # Shape (S15) and placement (S14) are checked semantically time_zone: string - type: - enum: ["enum", "number(ordinal)", "number(quantity)", "date", - "datetime", "number", "number(id)", "string", "boolean"] + # Valid types: the fixed scalars, "struct", and "list(element_type)" + # where element_type is any of the above. Semantic validation + # (S19) catches unknown strings; the schema accepts any string + # so that list(...) variants don't require enumeration here. + type: string values: anyOf: - arrayOf: # [M, F, U] @@ -109,6 +111,12 @@ object: enum: [primary_key, foreign_key, required, unique] examples: arrayOf: any + # Struct fields: same shape as columns. Type-specific rules + # (struct requires fields; fields banned on other types) + # are enforced semantically (S07). arrayOf: any lets the + # recursive structure through; S07/S11/etc. validate it. + fields: + arrayOf: any relationships: arrayOf: diff --git a/site/spec.md b/site/spec.md index 7d46a66..f262fcd 100644 --- a/site/spec.md +++ b/site/spec.md @@ -135,6 +135,8 @@ The supported types are: * `date`: calendar dates, written as ISO 8601 strings (`YYYY-MM-DD`, e.g. `2024-01-31`). * `datetime`: date-times, written as ISO 8601 strings. Without a `time_zone` they carry an offset (e.g. `2024-01-31T09:30:00Z`); with a `time_zone` they're written zoneless and interpreted in that zone (see [Time zones](#time-zones)). * `enum`: a column with repeated values from a known set. The allowed values are listed in the `values` property. +* `list(element_type)`: an ordered sequence of zero or more elements of the given type (see [List element types](#list-element-types)). +* `struct`: a structured record with named fields documented in the required `fields` property (see [Struct fields](#struct-fields)). #### Measures @@ -155,9 +157,60 @@ A `number(quantity)` column can also declare its `units`: a free-text string nam range: [0, 5000] ``` +#### List element types + +The element type in `list(element_type)` may be any type: `string`, `number`, `number(id)`, `number(ordinal)`, `number(quantity)`, `boolean`, `date`, `datetime`, `enum`, or `struct`. The same properties that apply to a column of that type apply when it is used as a list element type — `values` for `enum`, `fields` for `struct`, and so on. + +```yaml +- name: tags + type: list(string) + examples: [nature, outdoor, urban, photography, wildlife] + +- name: categories + type: list(enum) + values: [food, drink, dessert] + +- name: line_items + type: list(struct) + fields: + - name: product_id + type: number(id) + examples: [101, 204, 389] + - name: quantity + type: number(quantity) + units: units + range: [1, 100] + - name: price + type: number(quantity) + units: USD + range: [0.99, 999.99] +``` + +#### Struct fields + +A `struct` column may include a `fields` property — an ordered list of field descriptors. Each field descriptor uses the same schema as a column descriptor. A field may itself be `list(...)` or `struct` (with its own `fields`), allowing deep nesting. + +```yaml +- name: address + type: struct + fields: + - name: street + type: string + examples: [123 Main St, 456 Oak Ave, 789 Elm Dr] + - name: city + type: string + examples: [Portland, Austin, Chicago] + - name: zip + type: string + examples: ["97201", "78701", "60601"] + - name: country + type: enum + values: [US, CA, MX] +``` + #### Representative values -Every type has some way of representing the data it contains: an exhaustive set of values, a range, or a handful of examples. Each such column carries exactly one of the following three properties, determined by the column's `type`: +Most typed columns carry exactly one of the following three properties to represent the data they contain. The exceptions are `boolean` (values are always `true`/`false`) and `struct` (whose fields carry their own). * `values`: the allowed values for an `enum` column. Can be a list (`[M, F, U]`) when values are self-explanatory, or a map (`{M: Male, F: Female, U: Unknown}`) when values need labels. The values themselves must be scalars (string, number, or boolean); in the map form the labels must be strings. (`boolean` columns implicitly have `values: [true, false]`, no need to explicitly include it.) * `range`: a two-element list `[min, max]` giving the inclusive minimum and maximum *observed* in the column. Like `examples`, it describes the data rather than constraining it — a value outside the range will generate a warning, not a validation error. Used for the ordered numeric and temporal types: `number(ordinal)`, `number(quantity)`, `date`, and `datetime`. Both elements must match the column's type, and the minimum must not exceed the maximum. @@ -167,6 +220,8 @@ Every type has some way of representing the data it contains: an exhaustive set `boolean` columns are the exception to this rule because they can only contain `true`, `false`, and (if not required) `null`. +For `list(element_type)` columns, the same properties apply but describe the element values, not the lists themselves. The mapping follows the element type: `values` for `list(enum)`, `range` for `list(number(ordinal))`, `list(number(quantity))`, `list(date)`, and `list(datetime)`, `examples` for `list(string)`, `list(number)`, and `list(number(id))`, and no representative values for `list(boolean)` or `list(struct)` (same as their scalar counterparts). Each property means the same thing it would for a scalar column of the element type — for instance, `range` on a `list(number(quantity))` column gives the minimum and maximum element value observed across all lists. + #### Time zones A `datetime` column can declare its `time_zone`, which says how to interpret its values as moments in time. The value is either an [IANA time zone name](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones) or the sentinel `naive`: @@ -192,8 +247,8 @@ NB: when `time_zone` is present, write the column's `range` as plain, zoneless d The `constraints` property is a list of constraint names. The supported constraints are: -* `primary_key`: the set of columns with the `primary_key` constraint uniquely identifies each row. Implies `required` and `unique`. -* `foreign_key`: the column references a primary key in another table (or in the current table, if a self-join). The specific relationship is defined in [`relationships`](#relationships). +* `primary_key`: the set of columns with the `primary_key` constraint uniquely identifies each row. Implies `required` and `unique`. Not valid on `list` or `struct` columns, or on fields within a `struct`. +* `foreign_key`: the column references a primary key in another table (or in the current table, if a self-join). The specific relationship is defined in [`relationships`](#relationships). Not valid on `list` or `struct` columns, or on fields within a `struct`. * `required`: the column does not contain null/missing values. * `unique`: the column's values are distinct (no duplicates). diff --git a/site/validation.md b/site/validation.md index b55b7e7..eb4ff4c 100644 --- a/site/validation.md +++ b/site/validation.md @@ -48,6 +48,8 @@ When validating the spec, each problem with the dictionary is one of: * **Misplaced single-table description** (S16, warning): a dictionary with exactly one table carries `label`, `description`, or `details` on that table; for a single-table dictionary these belong at the top level. * **Malformed version** (S17, error): the top-level `version` does not give exactly one of `number`, `date`, or `hash`; its `number` is not three dot-separated numeric components (`MAJOR.MINOR.PATCH`) with an optional pre-release/build suffix; or its `date` is not a valid ISO 8601 date (`YYYY-MM-DD`). * **Missing `$version`** (S18, error): the document omits the required top-level `$version` key. +* **Invalid type** (S19, error): a column's `type` is not a recognised type string. Valid types are the fixed scalars (`string`, `number`, `number(id)`, `number(ordinal)`, `number(quantity)`, `boolean`, `date`, `datetime`), `enum`, `struct`, and `list(element_type)` where the element type is any of the above. +* **Key constraint on list or struct** (S20, error): a `primary_key` or `foreign_key` constraint appears on a `list` or `struct` column, or on any field inside a `struct`. (An `enum`'s `values` are constrained structurally by the schema rather than by an `S` check: each value must be a scalar, and in the map form each label must be a string. The `version` map's allowed keys and their value types are likewise structural; S17 covers only the semantics the schema can't express.)