diff --git a/crates/data-dict-parquet/src/dictionary.rs b/crates/data-dict-parquet/src/dictionary.rs new file mode 100644 index 0000000..cbcad8c --- /dev/null +++ b/crates/data-dict-parquet/src/dictionary.rs @@ -0,0 +1,164 @@ +//! Dictionary-page fast path for enum membership (D03). +//! +//! Enums are low-cardinality and so almost always dictionary-encoded, which +//! means a column chunk's distinct values sit in one small dictionary page +//! rather than being spread across every row. Reading those pages lets the +//! common, conforming case be settled without decoding all the data. +//! +//! This proves *conformance* only. It returns `true` when every value is +//! provably in the allowed set, and `false` at the first sign of doubt — a +//! chunk that isn't fully dictionary-encoded, an unsupported physical type, or +//! a dictionary entry outside the set — leaving the caller to fall back to the +//! full value scan (which also reports the offending rows). + +use std::collections::HashSet; + +use parquet::basic::{Encoding, PageType, Type as PhysicalType}; +use parquet::column::page::{Page, PageReader}; +use parquet::file::metadata::ColumnChunkMetaData; +use parquet::file::reader::FileReader; + +use crate::ParquetError; + +/// Whether every non-null value in the `leaf`th column is provably in `allowed`, +/// determined from the row groups' dictionary pages alone. `false` means "not +/// proven" (scan to be sure), never "definitely violates". +/// +/// `allowed` holds the canonical string forms produced by `scan::field_key`; +/// dictionary values are canonicalized the same way here. +pub(crate) fn dictionary_conforms( + reader: &dyn FileReader, + leaf: usize, + allowed: &HashSet, +) -> Result { + let meta = reader.metadata(); + for group in 0..meta.num_row_groups() { + let column = meta.row_group(group).column(leaf); + if column.num_values() == 0 { + continue; + } + if column.dictionary_page_offset().is_none() { + return Ok(false); + } + let row_group = reader.get_row_group(group)?; + let mut pages = row_group.get_column_page_reader(leaf)?; + + // The dictionary page comes first; it holds every value the data pages + // reference. If any entry is outside the set, defer to the scan. + let Some(page @ Page::DictionaryPage { .. }) = pages.get_next_page()? else { + return Ok(false); + }; + if !dictionary_in_set(&page, column.column_type(), allowed) { + return Ok(false); + } + if !data_pages_all_dictionary(column, &mut *pages)? { + return Ok(false); + } + } + Ok(true) +} + +/// Whether every data page draws from the dictionary (so the dictionary page is +/// exhaustive). Uses the footer's page encoding stats when the writer recorded +/// them; otherwise inspects each page's encoding directly — cheap, since these +/// pages hold only dictionary indices and are never decoded here. +fn data_pages_all_dictionary( + column: &ColumnChunkMetaData, + pages: &mut dyn PageReader, +) -> Result { + if let Some(stats) = column.page_encoding_stats() { + let mut data_pages = 0; + for stat in stats { + if matches!(stat.page_type, PageType::DATA_PAGE | PageType::DATA_PAGE_V2) { + if !is_dictionary(stat.encoding) { + return Ok(false); + } + data_pages += stat.count; + } + } + return Ok(data_pages > 0); + } + let mut data_pages = 0; + while let Some(page) = pages.get_next_page()? { + if !is_dictionary(page.encoding()) { + return Ok(false); + } + data_pages += 1; + } + Ok(data_pages > 0) +} + +fn is_dictionary(encoding: Encoding) -> bool { + matches!( + encoding, + Encoding::RLE_DICTIONARY | Encoding::PLAIN_DICTIONARY + ) +} + +/// Whether every value in a PLAIN-encoded dictionary page is in `allowed`. +/// `false` for physical types an enum can't sensibly use, or a malformed buffer. +fn dictionary_in_set(page: &Page, physical: PhysicalType, allowed: &HashSet) -> bool { + let Page::DictionaryPage { + buf, num_values, .. + } = page + else { + return false; + }; + let count = *num_values as usize; + match physical { + PhysicalType::BYTE_ARRAY => byte_arrays_in_set(buf, count, allowed), + PhysicalType::INT32 => fixed_in_set::<4>(buf, count, allowed, |b| { + i32::from_le_bytes(b).to_string() + }), + PhysicalType::INT64 => fixed_in_set::<8>(buf, count, allowed, |b| { + i64::from_le_bytes(b).to_string() + }), + PhysicalType::FLOAT => fixed_in_set::<4>(buf, count, allowed, |b| { + f32::from_le_bytes(b).to_string() + }), + PhysicalType::DOUBLE => fixed_in_set::<8>(buf, count, allowed, |b| { + f64::from_le_bytes(b).to_string() + }), + _ => false, + } +} + +/// Decode `count` PLAIN byte-array values (`[u32 length][bytes]`), requiring each +/// to be UTF-8 (matching `field_key`, which only keys `Field::Str`) and present +/// in `allowed`. +fn byte_arrays_in_set(buf: &[u8], count: usize, allowed: &HashSet) -> bool { + let mut pos = 0; + for _ in 0..count { + let Some(len_bytes) = buf.get(pos..pos + 4) else { + return false; + }; + let len = u32::from_le_bytes(len_bytes.try_into().unwrap()) as usize; + pos += 4; + let Some(value) = buf.get(pos..pos + len) else { + return false; + }; + pos += len; + let Ok(text) = std::str::from_utf8(value) else { + return false; + }; + if !allowed.contains(text) { + return false; + } + } + true +} + +/// Decode `count` fixed-width PLAIN values, canonicalizing each with `key`. +fn fixed_in_set( + buf: &[u8], + count: usize, + allowed: &HashSet, + key: impl Fn([u8; N]) -> String, +) -> bool { + if buf.len() < count * N { + return false; + } + buf.chunks_exact(N) + .take(count) + .all(|chunk| allowed.contains(&key(chunk.try_into().unwrap()))) +} diff --git a/crates/data-dict-parquet/src/lib.rs b/crates/data-dict-parquet/src/lib.rs index fdc9bf9..2bb925d 100644 --- a/crates/data-dict-parquet/src/lib.rs +++ b/crates/data-dict-parquet/src/lib.rs @@ -1,5 +1,6 @@ //! Parquet reader for data-dict.yaml validation. +mod dictionary; mod metadata; mod scan; diff --git a/crates/data-dict-parquet/src/scan.rs b/crates/data-dict-parquet/src/scan.rs index 863d44d..1f0a3f7 100644 --- a/crates/data-dict-parquet/src/scan.rs +++ b/crates/data-dict-parquet/src/scan.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fs::File; use std::path::Path; @@ -13,16 +13,21 @@ use crate::ParquetError; pub struct ColumnNeeds { /// Count nulls and sample the row numbers where they occur. pub nulls: bool, + /// The set of allowed values (D03). When present, non-null values not in + /// the set are counted and sampled. Values are the canonical string form + /// produced by [`field_key`]; the caller must canonicalize its set to match. + pub allowed: Option>, } impl ColumnNeeds { pub fn any(&self) -> bool { - self.nulls + self.nulls || self.allowed.is_some() } pub fn merge(self, other: Self) -> Self { ColumnNeeds { nulls: self.nulls || other.nulls, + allowed: self.allowed.or(other.allowed), } } } @@ -33,6 +38,13 @@ pub struct ColumnStats { pub null_count: usize, /// 1-based row numbers, capped by the caller's limit. pub null_rows: Vec, + /// Non-null values found outside the [`ColumnNeeds::allowed`] set. + pub outside_count: usize, + /// 1-based row numbers of outside values, capped by the caller's limit. + pub outside_rows: Vec, + /// Distinct offending values, capped by the caller's limit, in first-seen + /// order. + pub outside_values: Vec, } /// Gather requested statistics in one projected, streaming pass over the file. @@ -62,9 +74,27 @@ pub fn column_stats( .iter() .map(|(name, _, _)| (name.clone(), ColumnStats::default())) .collect(); + + // Fast path: settle the enum-membership need (D03) from dictionary pages + // where the data conforms, sparing those columns the value scan. A column + // still scanned for its nulls skips the redundant dictionary read. + let proven: HashSet<&str> = requested + .iter() + .filter(|(_, _, need)| need.allowed.is_some() && !need.nulls) + .filter_map(|(name, index, need)| { + let allowed = need.allowed.as_ref()?; + crate::dictionary::dictionary_conforms(&reader, *index, allowed) + .ok() + .filter(|&conforms| conforms) + .map(|_| name.as_str()) + }) + .collect(); + let to_scan: Vec = requested .iter() - .filter(|(_, _, need)| need.nulls) + .filter(|(name, _, need)| { + need.nulls || (need.allowed.is_some() && !proven.contains(name.as_str())) + }) .map(|(_, index, _)| *index) .collect(); @@ -87,10 +117,26 @@ pub fn column_stats( let (Some(stat), Some(need)) = (stats.get_mut(name), needs.get(name)) else { continue; }; - if need.nulls && matches!(field, Field::Null) { - stat.null_count += 1; - if stat.null_rows.len() < limit { - stat.null_rows.push(index + 1); + if matches!(field, Field::Null) { + if need.nulls { + stat.null_count += 1; + if stat.null_rows.len() < limit { + stat.null_rows.push(index + 1); + } + } + continue; + } + if let Some(allowed) = &need.allowed + && !proven.contains(name.as_str()) + && let Some(key) = field_key(field) + && !allowed.contains(&key) + { + stat.outside_count += 1; + if stat.outside_rows.len() < limit { + stat.outside_rows.push(index + 1); + } + if stat.outside_values.len() < limit && !stat.outside_values.contains(&key) { + stat.outside_values.push(key); } } } @@ -98,3 +144,25 @@ pub fn column_stats( Ok(stats) } + +/// The canonical string form of a scalar field value, for set membership (D03). +/// `None` for kinds that can't be an `enum` value (a matching `enum` column +/// would already be an `M01` type mismatch). Integer and float forms follow +/// Rust's `Display`, matching `Scalar::value_key` on the dictionary side. +fn field_key(field: &Field) -> Option { + Some(match field { + Field::Bool(v) => v.to_string(), + Field::Byte(v) => v.to_string(), + Field::Short(v) => v.to_string(), + Field::Int(v) => v.to_string(), + Field::Long(v) => v.to_string(), + Field::UByte(v) => v.to_string(), + Field::UShort(v) => v.to_string(), + Field::UInt(v) => v.to_string(), + Field::ULong(v) => v.to_string(), + Field::Float(v) => v.to_string(), + Field::Double(v) => v.to_string(), + Field::Str(v) => v.clone(), + _ => return None, + }) +} diff --git a/crates/data-dict/src/lower.rs b/crates/data-dict/src/lower.rs index 516ffdd..1cef728 100644 --- a/crates/data-dict/src/lower.rs +++ b/crates/data-dict/src/lower.rs @@ -87,7 +87,7 @@ fn lower_column(node: &YamlWithSourceInfo) -> Option { let mut name: Option> = None; let mut constraints: Vec> = Vec::new(); let mut col_type: Option> = None; - let mut values: Option = None; + let mut values: Option = None; let mut range: Option = None; let mut examples: Option = None; let mut units: Option> = None; @@ -108,7 +108,12 @@ fn lower_column(node: &YamlWithSourceInfo) -> Option { col_type = Some(Spanned::new(s.to_string(), entry.value_span.clone())); } } - "values" => values = Some(entry.value_span.clone()), + "values" => { + values = Some(Representation { + span: entry.value_span.clone(), + items: lower_enum_values(&entry.value), + }); + } "range" => { range = Some(Representation { span: entry.value_span.clone(), @@ -157,6 +162,20 @@ fn lower_column(node: &YamlWithSourceInfo) -> Option { }) } +/// Lower an enum's `values` node into its allowed scalars with spans. +fn lower_enum_values(node: &YamlWithSourceInfo) -> Vec> { + if let Some(entries) = node.as_hash() { + // Map form: the keys are the values, the labels are dropped. + entries + .iter() + .map(|entry| Spanned::new(lower_scalar(&entry.key), entry.key.source_info.clone())) + .collect() + } else { + // List form (or a lone scalar, which the schema rejects upstream). + lower_scalars(node) + } +} + /// Lower a `range` or `examples` node into its scalar elements with spans. /// Non-array nodes yield an empty vector (the schema rejects them upstream). fn lower_scalars(node: &YamlWithSourceInfo) -> Vec> { diff --git a/crates/data-dict/src/model.rs b/crates/data-dict/src/model.rs index 62a8558..073e286 100644 --- a/crates/data-dict/src/model.rs +++ b/crates/data-dict/src/model.rs @@ -59,7 +59,9 @@ pub struct Column { pub name: Spanned, pub constraints: Vec>, pub col_type: Option>, - pub values: Option, + /// The allowed values of an `enum` column: the list items, or the keys of + /// the map form (whose labels are dropped — only the values are constrained). + pub values: Option, pub range: Option, pub examples: Option, pub units: Option>, @@ -93,6 +95,18 @@ impl Scalar { Scalar::Compound => "a list or map", } } + + /// A canonical string form for value-equality comparison against data (D03). + /// `None` for kinds that can't appear as a data value (`null`, compound). + /// Must agree with the data side's canonicalization in `data-dict-parquet`. + pub fn value_key(&self) -> Option { + match self { + Scalar::Number(n) => Some(n.to_string()), + Scalar::String(s) => Some(s.clone()), + Scalar::Bool(b) => Some(b.to_string()), + Scalar::Null | Scalar::Compound => None, + } + } } impl Column { @@ -111,6 +125,10 @@ impl Column { pub fn is_required_implied(&self) -> bool { self.has(Constraint::Required) || self.has(Constraint::PrimaryKey) } + + pub fn is_enum(&self) -> bool { + self.col_type.as_ref().is_some_and(|t| t.value == "enum") + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/data-dict/src/problem.rs b/crates/data-dict/src/problem.rs index e9435de..eb9b120 100644 --- a/crates/data-dict/src/problem.rs +++ b/crates/data-dict/src/problem.rs @@ -138,6 +138,14 @@ pub enum ProblemKind { /// `D01` — a `required` (or `primary_key`) column contains nulls. `rows` /// lists the first few offending row numbers (1-based); `count` is the total. NullsInRequired { count: usize, rows: Vec }, + /// `D03` — an `enum` column contains values outside its declared `values`. + /// `count` is the total; `rows` lists the first few offending row numbers + /// (1-based) and `values` the first few distinct offending values. + ValuesOutsideEnum { + count: usize, + rows: Vec, + values: Vec, + }, } impl ProblemKind { @@ -152,6 +160,7 @@ impl ProblemKind { ProblemKind::MissingSource => "M04", ProblemKind::UnreadableSource => "M05", ProblemKind::NullsInRequired { .. } => "D01", + ProblemKind::ValuesOutsideEnum { .. } => "D03", _ => return None, }) } @@ -166,7 +175,9 @@ impl ProblemKind { | ProblemKind::ExtraInData { .. } | ProblemKind::MissingSource | ProblemKind::UnreadableSource => Level::Meta, - ProblemKind::NullsInRequired { .. } => Level::Data, + ProblemKind::NullsInRequired { .. } | ProblemKind::ValuesOutsideEnum { .. } => { + Level::Data + } _ => return None, }) } @@ -585,6 +596,15 @@ mod tests { .level(), Some(Level::Data) ); + assert_eq!( + ProblemKind::ValuesOutsideEnum { + count: 1, + rows: vec![2], + values: vec!["x".into()], + } + .code(), + Some("D03") + ); assert_eq!(ProblemKind::Io.code(), None); assert_eq!(ProblemKind::Io.level(), None); } diff --git a/crates/data-dict/src/validate_data.rs b/crates/data-dict/src/validate_data.rs index 8f42519..0b1f876 100644 --- a/crates/data-dict/src/validate_data.rs +++ b/crates/data-dict/src/validate_data.rs @@ -3,7 +3,7 @@ //! [`validate_data`] is the entry point; `value_issues` is the value-checking //! core it runs after the metadata checks ([`crate::validate_meta`]). -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::Path; use data_dict_parquet::{ColumnMeta, ColumnNeeds, ColumnStats}; @@ -115,7 +115,7 @@ trait ColumnCheck { /// Every value-level check, run against each present column. Add a check here /// and the plan/scan/check pipeline picks it up automatically. -const VALUE_CHECKS: &[&dyn ColumnCheck] = &[&RequiredNotNull]; +const VALUE_CHECKS: &[&dyn ColumnCheck] = &[&RequiredNotNull, &EnumMembership]; /// D01 — a `required` (or `primary_key`) column must contain no nulls. struct RequiredNotNull; @@ -128,6 +128,7 @@ impl ColumnCheck for RequiredNotNull { fn needs(&self, col: &Column) -> ColumnNeeds { ColumnNeeds { nulls: col.is_required_implied(), + ..ColumnNeeds::default() } } @@ -146,6 +147,73 @@ impl ColumnCheck for RequiredNotNull { } } +/// D03 — an `enum` column's values must all be among its declared `values`. +struct EnumMembership; + +impl ColumnCheck for EnumMembership { + fn check_meta(&self, _table: &Table, col: &Column, _meta: &ColumnMeta) -> CheckResult { + crate::validate_meta::validate_d03_enum_membership(col) + } + + fn needs(&self, col: &Column) -> ColumnNeeds { + ColumnNeeds { + allowed: enum_allowed(col), + ..ColumnNeeds::default() + } + } + + fn check_data(&self, table: &Table, col: &Column, stats: &ColumnStats) -> Option { + // The set was only requested for enum columns, so any outside value is a + // violation. + if stats.outside_count == 0 { + return None; + } + Some(values_outside_enum(table, col, stats)) + } +} + +/// The canonical string forms of an `enum` column's allowed values, or `None` +/// when the column declares no `values` (so it opts out of the check). +fn enum_allowed(col: &Column) -> Option> { + let values = col.values.as_ref()?; + Some( + values + .items + .iter() + .filter_map(|item| item.value.value_key()) + .collect(), + ) +} + +fn values_outside_enum(table: &Table, col: &Column, stats: &ColumnStats) -> Problem { + let count = stats.outside_count; + let rows = crate::problem::format_rows(&stats.outside_rows, count); + let plural = if count == 1 { "" } else { "s" }; + let sample = stats + .outside_values + .iter() + .map(|value| format!("`{value}`")) + .collect::>() + .join(", "); + let values_span = col + .values + .as_ref() + .map_or_else(|| col.name.span.clone(), |values| values.span.clone()); + Problem { + code: Some("D03"), + severity: Severity::Error, + message: format!("has {count} value{plural} outside the allowed set ({sample}; {rows})"), + column: None, + expected: Some("An enum column's values must all be among its declared `values`.".into()), + context: vec![table.name.span.clone(), col.name.span.clone(), values_span], + kind: ProblemKind::ValuesOutsideEnum { + count, + rows: stats.outside_rows.clone(), + values: stats.outside_values.clone(), + }, + } +} + fn nulls_in_required_data(table: &Table, col: &Column, count: usize, rows: Vec) -> Problem { let detail = crate::problem::format_rows(&rows, count); let plural = if count == 1 { "" } else { "s" }; diff --git a/crates/data-dict/src/validate_meta.rs b/crates/data-dict/src/validate_meta.rs index 38c2ac0..102533c 100644 --- a/crates/data-dict/src/validate_meta.rs +++ b/crates/data-dict/src/validate_meta.rs @@ -58,6 +58,18 @@ pub(crate) fn validate_d01_required_not_null( } } +/// Attempt D03 from footer metadata. Set membership can't be settled from the +/// footer — min/max bound the extremes but say nothing about the values between +/// them — so an `enum` column carrying a `values` set is always inconclusive and +/// deferred to the scan; any other column passes here. +pub(crate) fn validate_d03_enum_membership(col: &Column) -> CheckResult { + if col.is_enum() && col.values.is_some() { + CheckResult::Inconclusive + } else { + CheckResult::Pass + } +} + fn nulls_in_required_meta(table: &Table, col: &Column, count: usize) -> Problem { let plural = if count == 1 { "" } else { "s" }; let constraint_span = col diff --git a/crates/data-dict/src/validate_spec.rs b/crates/data-dict/src/validate_spec.rs index c0f32bf..9a1e0a5 100644 --- a/crates/data-dict/src/validate_spec.rs +++ b/crates/data-dict/src/validate_spec.rs @@ -485,7 +485,7 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet "S07", format!("A `{type_name}` column must use `range`, not `values`."), found("values"), - at(values), + at(&values.span), ); } if let Some(examples) = &col.examples { @@ -498,7 +498,7 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet } } else if type_name == "boolean" { for (span, key) in [ - (col.values.as_ref(), "values"), + (col.values.as_ref().map(|v| &v.span), "values"), (col.range.as_ref().map(|r| &r.span), "range"), (col.examples.as_ref().map(|e| &e.span), "examples"), ] { @@ -525,7 +525,7 @@ fn validate_s07_representation(table: &Table, col: &Column, out: &mut ProblemSet "S07", format!("A `{type_name}` column must not use `values`."), found("values"), - at(values), + at(&values.span), ); } if let Some(range) = &col.range { diff --git a/crates/data-dict/tests/snapshots/validate_data__values_outside_enum_reported.snap b/crates/data-dict/tests/snapshots/validate_data__values_outside_enum_reported.snap new file mode 100644 index 0000000..733ecc7 --- /dev/null +++ b/crates/data-dict/tests/snapshots/validate_data__values_outside_enum_reported.snap @@ -0,0 +1,25 @@ +--- +source: crates/data-dict/tests/validate_data.rs +--- +$version: "0.1.0" +$learn_more: http://data-dict.tidyverse.org/ +tables: + t: + source: + parquet: data.parquet + columns: + - name: status + type: enum + values: [active, banned] +──────────────────────────────────────── +Error: [D03] An enum column's values must all be among its declared `values`. + ╭─[ dict.yaml:10:17 ] + │ + 4 │ t: + │ + 8 │ - name: status + 9 │ type: enum + 10 │ values: [active, banned] + │ ───────┬─────── + │ ╰───────── has 1 value outside the allowed set (`sleepy`; row: 4) +────╯ diff --git a/crates/data-dict/tests/validate_data.rs b/crates/data-dict/tests/validate_data.rs index cde468e..8d3bd85 100644 --- a/crates/data-dict/tests/validate_data.rs +++ b/crates/data-dict/tests/validate_data.rs @@ -14,7 +14,7 @@ use std::sync::Arc; use data_dict::{Problem, ProblemKind, ProblemSet, Status, validate_data, validate_meta}; use indoc::{formatdoc, indoc}; -use parquet::data_type::DoubleType; +use parquet::data_type::{ByteArray, ByteArrayType, DoubleType, Int32Type}; use parquet::file::properties::{EnabledStatistics, WriterProperties}; use parquet::file::writer::{SerializedColumnWriter, SerializedFileWriter}; use parquet::schema::parser::parse_message_type; @@ -224,6 +224,193 @@ fn nulls_in_optional_column_ok() { assert_eq!(result.status(), Status::Ok); } +/// Write the given strings as a required UTF-8 byte-array column. +fn write_strings<'a>(values: &'a [&'a str]) -> impl FnOnce(&mut SerializedColumnWriter) + 'a { + move |col| { + let bytes = values + .iter() + .map(|s| ByteArray::from(*s)) + .collect::>(); + col.typed::() + .write_batch(&bytes, None, None) + .unwrap(); + } +} + +#[test] +fn values_outside_enum_reported() { + let yaml = build_column( + "REQUIRED BYTE_ARRAY status (UTF8)", + write_strings(&["active", "banned", "active", "sleepy"]), + indoc! {" + - name: status + type: enum + values: [active, banned] + "}, + ); + let result = validate_data(&yaml, None); + + assert_eq!(result.status(), Status::Error); + assert!( + matches!( + result.items.as_slice(), + [Problem { + code: Some("D03"), + kind: ProblemKind::ValuesOutsideEnum { count: 1, rows, values }, + .. + }] if rows == &[4] && values == &["sleepy"] + ), + "got {:?}", + result.items + ); + #[cfg(unix)] + assert_snapshot!(common::diagnostic(&yaml, &result.render().join("\n"))); +} + +#[test] +fn enum_values_within_set_ok() { + let result = check_column( + "REQUIRED BYTE_ARRAY status (UTF8)", + write_strings(&["active", "banned", "active"]), + indoc! {" + - name: status + type: enum + values: [active, banned] + "}, + ); + + assert_eq!(result.status(), Status::Ok, "got {:?}", result.items); +} + +#[test] +fn enum_map_form_values_are_the_keys() { + // The map form's keys are the allowed values; the labels are ignored. + let result = check_column( + "REQUIRED BYTE_ARRAY status (UTF8)", + write_strings(&["A", "Active"]), + indoc! {" + - name: status + type: enum + values: + A: Active + B: Banned + "}, + ); + + assert_eq!(result.status(), Status::Error); + assert!( + matches!( + result.items.as_slice(), + [Problem { + kind: ProblemKind::ValuesOutsideEnum { count: 1, rows, values }, + .. + }] if rows == &[2] && values == &["Active"] + ), + "got {:?}", + result.items + ); +} + +#[test] +fn nulls_in_optional_enum_are_not_outside_values() { + // A null is the concern of D01 (and only when required); it is never an + // "outside the set" value. + let result = check_column( + "OPTIONAL BYTE_ARRAY status (UTF8)", + |col| { + let bytes = [ByteArray::from("active"), ByteArray::from("banned")]; + col.typed::() + .write_batch(&bytes, Some(&[1, 0, 1]), None) + .unwrap(); + }, + indoc! {" + - name: status + type: enum + values: [active, banned] + "}, + ); + + assert_eq!(result.status(), Status::Ok, "got {:?}", result.items); +} + +#[test] +fn numeric_enum_values_are_checked() { + let result = check_column( + "REQUIRED INT32 grade", + |col| { + col.typed::() + .write_batch(&[1, 2, 3], None, None) + .unwrap(); + }, + indoc! {" + - name: grade + type: enum + values: [1, 2] + "}, + ); + + assert_eq!(result.status(), Status::Error); + assert!( + matches!( + result.items.as_slice(), + [Problem { + kind: ProblemKind::ValuesOutsideEnum { count: 1, rows, values }, + .. + }] if rows == &[3] && values == &["3"] + ), + "got {:?}", + result.items + ); +} + +/// With dictionary encoding disabled, the D03 dictionary fast-path can't prove +/// conformance and must fall back to the value scan — which still finds the +/// violation and its exact row. +#[test] +fn enum_without_dictionary_encoding_falls_back_to_scan() { + let no_dict = || { + WriterProperties::builder() + .set_dictionary_enabled(false) + .build() + }; + + let clean = build_column_with_properties( + "REQUIRED BYTE_ARRAY status (UTF8)", + write_strings(&["active", "banned", "active"]), + indoc! {" + - name: status + type: enum + values: [active, banned] + "}, + no_dict(), + ); + assert_eq!(validate_data(&clean, None).status(), Status::Ok); + + let bad = build_column_with_properties( + "REQUIRED BYTE_ARRAY status (UTF8)", + write_strings(&["active", "banned", "sleepy"]), + indoc! {" + - name: status + type: enum + values: [active, banned] + "}, + no_dict(), + ); + let result = validate_data(&bad, None); + assert!( + matches!( + result.items.as_slice(), + [Problem { + code: Some("D03"), + kind: ProblemKind::ValuesOutsideEnum { count: 1, rows, values }, + .. + }] if rows == &[3] && values == &["sleepy"] + ), + "got {:?}", + result.items + ); +} + #[test] fn primary_key_implies_required_for_nulls() { // `primary_key` implies `required`, so the null is reported even without an diff --git a/site/validation.md b/site/validation.md index 8630086..15e4b95 100644 --- a/site/validation.md +++ b/site/validation.md @@ -65,3 +65,4 @@ When validating the data's metadata against the dictionary, each column mismatch When validating the data's values against the dictionary, each column mismatch is one of: * **Nulls in a required column** (D01, error): a `required` or `primary_key` column contains nulls. +* **Value outside enum** (D03, error): an `enum` column contains a (non-null) value that is not one of its declared `values`.