diff --git a/vortex-test/compat-gen/DESIGN.md b/vortex-test/compat-gen/DESIGN.md index fe20f6d004c..b0779e1b740 100644 --- a/vortex-test/compat-gen/DESIGN.md +++ b/vortex-test/compat-gen/DESIGN.md @@ -18,14 +18,15 @@ from any version decodes to the wrong values, the test fails. ## How Fixtures Work -A fixture is a small `.vortex` file with known contents. The expected -contents are defined by a Rust function — `build()` — that deterministically -constructs the arrays. The same `build()` code is used at both ends: +A fixture is a small `.vortex` file with known contents. A deterministic +Rust function, `build()`, defines the expected contents. The two ends use +these functions: -- **At publish time:** `build()` produces arrays -> the writer serializes them - to a `.vortex` file -> the file is uploaded to S3. -- **At check time:** `build()` produces the same arrays -> the reader decodes - the old file -> the two are compared value-by-value. +- **At publish time:** `build_written()` produces arrays -> the writer + serializes them to a `.vortex` file -> the file is uploaded to S3. + `build_written()` defaults to `build()`. +- **At check time:** `build()` produces the expected arrays -> the reader + decodes the old file -> the two are compared value-by-value. If the reader is correct, the values match. If a code change breaks decoding, they don't. @@ -40,6 +41,22 @@ contain. The contract is that `build()` for a given fixture name is immutable once defined. It must never change its output. If someone modifies it, the check fails loudly against every old version. +Some fixtures must publish an old physical form. An example is a decimal +array that is stored narrower or wider than the canonical width of its +precision. `build()` must not return such a form, because `check` runs +`build()` with the current library. So these fixtures also override +`build_written()`, which builds the arrays that `generate` and `publish` +write. `check` still compares each published file with the result of +`build()`. The two results have the same dtype and the same logical values. +Only their physical form differs. So the published file keeps the old form, +and `build()` stays the oracle. + +An override works only while the current library can still build the old +form. For the decimal fixtures, that is while `DecimalArray::try_new` accepts +the stored width. When it rejects the width, `build_written()` fails, and the +override must be deleted. The files that earlier releases published keep the +old form. Later releases publish the canonical form under the same name. + ### Fixture evolution Because `build()` is immutable, you cannot add a column to an existing @@ -52,24 +69,44 @@ Every fixture implements this trait in Rust: ```rust pub trait Fixture { - /// Filename, e.g. "primitives.vortex". + /// Filename, e.g. "primitives.vortex" or "tpch_lineitem.regular.vortex". fn name(&self) -> &str; /// Human-readable description. fn description(&self) -> &str; - /// Optional async setup (download external data, etc). - /// Called before build(). Default is a no-op. - fn setup(&self, _tmp_dir: &Path) -> VortexResult<()> { Ok(()) } - - /// Build the expected arrays. Must be deterministic. - /// Returns a Vec to support chunked fixtures (multiple chunks). - fn build(&self, tmp_dir: &Path) -> VortexResult>; + /// Write the file(s) that `generate` and `publish` produce into `dir`. + fn write(&self, dir: &Path, ctx: &mut ExecutionCtx) -> VortexResult>; + + /// Write the file(s) that `check` compares with the published files. + /// Defaults to `write()`. + fn write_expected( + &self, + dir: &Path, + ctx: &mut ExecutionCtx, + ) -> VortexResult> { + self.write(dir, ctx) + } } ``` All fixtures are registered in `all_fixtures()` in `src/fixtures/mod.rs`. +Synthetic fixtures implement `FlatLayoutFixture`, and `FlatLayoutAdapter` +turns each of them into a `Fixture`. Two optional methods separate the file +that a fixture publishes from the file that `check` compares it with: + +- `FlatLayoutFixture::build_written()` builds the arrays that `generate` + writes. It defaults to `build()`. Override it only to publish an old + physical form that `build()` must not return, as + [the oracle question](#the-oracle-question) explains. +- `Fixture::write_expected()` writes the file that `check` compares with a + published file. `FlatLayoutAdapter` writes `build()` in `write_expected()` + and `build_written()` in `write()`. Dataset fixtures keep the default. + +The expected-encodings check runs on the result of `build_written()`, because +that is the form that the published file holds. + ### Current fixture suite | Fixture | File | Description | diff --git a/vortex-test/compat-gen/README.md b/vortex-test/compat-gen/README.md index e91101a0d2e..a6a90c83e68 100644 --- a/vortex-test/compat-gen/README.md +++ b/vortex-test/compat-gen/README.md @@ -70,11 +70,13 @@ Output: ### `check --dir --mode [--exclude ]` -For each `.vortex` file in the directory: +For each known fixture: -1. Run `setup()` + `build()` to reconstruct expected arrays from current code -2. Read the file bytes and decode via `adapter::read_file()` -3. Combine chunks into `ChunkedArray` and compare with `assert_arrays_eq!` +1. Write a fresh file into a temporary directory with `write_expected()`. + This builds the expected arrays from the current code. For a synthetic + fixture, it writes the result of `build()`. +2. Read the stored file and the fresh file with `adapter::read_file()`. +3. Compare the two arrays with `assert_arrays_eq!`. JSON result to stdout: ```json diff --git a/vortex-test/compat-gen/src/check.rs b/vortex-test/compat-gen/src/check.rs index a2834fa733b..357ead33fcd 100644 --- a/vortex-test/compat-gen/src/check.rs +++ b/vortex-test/compat-gen/src/check.rs @@ -42,7 +42,7 @@ struct FailedFixture { /// Check `.vortex` files in `dir` against in-memory fixtures. /// /// For each known fixture, generates fresh files in a temp directory via -/// `fixture.write(tmp_dir)`, then reads both the stored file and fresh file, +/// `fixture.write_expected(tmp_dir)`, then reads both the stored file and fresh file, /// decodes them, and compares the arrays. /// /// Prints JSON result to stdout, human-readable progress to stderr. @@ -67,7 +67,7 @@ pub fn check(dir: &Path, mode: Mode, exclude: &[String]) -> VortexResult<()> { eprintln!("generating fresh fixtures for comparison..."); for fixture in &fixtures { - fixture.write(tmp_dir.path(), &mut ctx)?; + fixture.write_expected(tmp_dir.path(), &mut ctx)?; } // Collect .vortex files in the check directory. diff --git a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/decimal_legacy.rs b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/decimal_legacy.rs new file mode 100644 index 00000000000..15ea9ac12f9 --- /dev/null +++ b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/decimal_legacy.rs @@ -0,0 +1,439 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +//! Compat fixtures that write decimal columns at a stored width other than the canonical width. +//! +//! Older writers stored one decimal dtype at any width that held its values, so old files hold +//! narrow, wide and Arrow-style forms. Each fixture here writes such forms with +//! [`FlatLayoutFixture::build_written`], with the raw value of every slot, including nonzero +//! values in null slots. [`FlatLayoutFixture::build`] returns the same logical values at the +//! canonical width, so `check` compares each old file with a canonical array. +//! +//! The overrides depend on [`DecimalArray::try_new`] accepting a stored width that differs from +//! the canonical width. Once it rejects such a width, `build_written` returns that error, and +//! `generate` and the round-trip test fail. The overrides must then be deleted. The published files +//! keep the old forms. + +use vortex_array::ArrayId; +use vortex_array::ArrayRef; +use vortex_array::ArrayVTable; +use vortex_array::ExecutionCtx; +use vortex_array::IntoArray; +use vortex_array::arrays::Decimal; +use vortex_array::arrays::DecimalArray; +use vortex_array::arrays::StructArray; +use vortex_array::dtype::BigCast; +use vortex_array::dtype::DecimalDType; +use vortex_array::dtype::DecimalType; +use vortex_array::match_each_decimal_value_type; +use vortex_array::validity::Validity; +use vortex_buffer::Buffer; +use vortex_error::VortexResult; +use vortex_error::vortex_err; + +use crate::fixtures::FlatLayoutFixture; + +/// The number of rows in every legacy decimal fixture. +const ROWS: usize = 5; + +/// One column of a legacy decimal fixture. +pub struct LegacyDecimalColumn { + /// The field name of the column. + pub name: &'static str, + /// The logical dtype of the column. + pub decimal_dtype: DecimalDType, + /// The width at which the written form stores the column. + pub stored_type: DecimalType, + /// The raw value of every slot, including null slots. + pub slots: [i128; ROWS], + /// The validity of every slot, or `None` for a non-nullable column. + pub validity: Option<[bool; ROWS]>, +} + +impl LegacyDecimalColumn { + /// Builds the column at its stored width with the raw slot values. + fn written(&self) -> VortexResult { + decimal_array( + self.decimal_dtype, + self.stored_type, + self.slots, + self.validity(), + ) + } + + /// Builds the column at the canonical width. + /// + /// A column that is already stored at the canonical width keeps its raw slot values, so its + /// expected form is its written form. A column that changes width gets zeros in its null + /// slots, because a raw null-slot value need not fit the canonical width. + fn expected(&self) -> VortexResult { + let canonical_type = DecimalType::smallest_decimal_value_type(&self.decimal_dtype); + if canonical_type == self.stored_type { + return self.written(); + } + + let mut slots = self.slots; + if let Some(validity) = self.validity { + for (slot, valid) in slots.iter_mut().zip(validity) { + if !valid { + *slot = 0; + } + } + } + + decimal_array(self.decimal_dtype, canonical_type, slots, self.validity()) + } + + fn validity(&self) -> Validity { + match self.validity { + Some(validity) => Validity::from_iter(validity), + None => Validity::NonNullable, + } + } +} + +/// Builds a decimal array that stores `slots` at `values_type`. +fn decimal_array( + decimal_dtype: DecimalDType, + values_type: DecimalType, + slots: [i128; ROWS], + validity: Validity, +) -> VortexResult { + match_each_decimal_value_type!(values_type, |T| { + let values = slots + .into_iter() + .map(|slot| { + ::from(slot).ok_or_else(|| { + vortex_err!("a slot value must fit in {values_type}, got {slot}") + }) + }) + .collect::>>()?; + Ok(DecimalArray::try_new(values, decimal_dtype, validity)?.into_array()) + }) +} + +/// Builds a non-nullable column. +fn column( + name: &'static str, + decimal_dtype: DecimalDType, + stored_type: DecimalType, + slots: [i128; ROWS], +) -> LegacyDecimalColumn { + LegacyDecimalColumn { + name, + decimal_dtype, + stored_type, + slots, + validity: None, + } +} + +/// Builds a nullable column. +fn nullable_column( + name: &'static str, + decimal_dtype: DecimalDType, + stored_type: DecimalType, + slots: [i128; ROWS], + validity: [bool; ROWS], +) -> LegacyDecimalColumn { + LegacyDecimalColumn { + name, + decimal_dtype, + stored_type, + slots, + validity: Some(validity), + } +} + +/// A compat fixture that stores its decimal columns at the widths of a [`LegacyDecimalColumn`]. +pub struct LegacyDecimalFixture { + name: &'static str, + description: &'static str, + columns: Vec, +} + +impl LegacyDecimalFixture { + #[cfg(test)] + pub fn columns(&self) -> &[LegacyDecimalColumn] { + &self.columns + } + + fn build_struct( + &self, + build_column: impl Fn(&LegacyDecimalColumn) -> VortexResult, + ) -> VortexResult { + let fields = self + .columns + .iter() + .map(|column| Ok((column.name, build_column(column)?))) + .collect::>>()?; + + Ok(StructArray::try_from_iter(fields)?.into_array()) + } +} + +impl FlatLayoutFixture for LegacyDecimalFixture { + fn name(&self) -> &str { + self.name + } + + fn description(&self) -> &str { + self.description + } + + fn expected_encodings(&self) -> Vec { + vec![Decimal.id()] + } + + fn build(&self, _ctx: &mut ExecutionCtx) -> VortexResult { + self.build_struct(LegacyDecimalColumn::expected) + } + + fn build_written(&self, _ctx: &mut ExecutionCtx) -> VortexResult { + self.build_struct(LegacyDecimalColumn::written) + } +} + +/// All legacy decimal fixtures. +pub fn fixtures() -> [LegacyDecimalFixture; 5] { + [ + narrow_storage(), + gap_values(), + high_scale(), + wide_storage(), + null_slots(), + ] +} + +/// Columns stored narrower than the canonical width, with values that fit the stored width at the +/// precision of the stored width. +fn narrow_storage() -> LegacyDecimalFixture { + let max_p9 = 999_999_999; + let max_p18 = 10i128.pow(18) - 1; + let max_p38 = 10i128.pow(38) - 1; + + LegacyDecimalFixture { + name: "decimal_narrow_storage.vortex", + description: "Decimal arrays stored narrower than the canonical width of their precision", + columns: vec![ + column( + "i8_p10_s2", + DecimalDType::new(10, 2), + DecimalType::I8, + [-99, -1, 0, 1, 99], + ), + column( + "i16_p18_s2", + DecimalDType::new(18, 2), + DecimalType::I16, + [-9_999, -1, 0, 1, 9_999], + ), + column( + "i32_p18_s2", + DecimalDType::new(18, 2), + DecimalType::I32, + [-max_p9, -1, 0, 1, max_p9], + ), + column( + "i64_p30_s2", + DecimalDType::new(30, 2), + DecimalType::I64, + [-max_p18, -1, 0, 1, max_p18], + ), + column( + "i128_p40_s0", + DecimalDType::new(40, 0), + DecimalType::I128, + [-max_p38, -1, 0, 1, max_p38], + ), + column( + "i8_p76_s0", + DecimalDType::new(76, 0), + DecimalType::I8, + [-99, -1, 0, 1, 99], + ), + ], + } +} + +/// Columns stored narrower than the canonical width, with values that fit the stored width but +/// need a precision above the precision of the stored width. +fn gap_values() -> LegacyDecimalFixture { + let i32_max: i128 = i32::MAX.into(); + let i64_max: i128 = i64::MAX.into(); + let ten_pow_18 = 10i128.pow(18); + + LegacyDecimalFixture { + name: "decimal_gap_values.vortex", + description: "Decimal arrays stored narrower than canonical, with values that need the \ + precision of the next width", + columns: vec![ + column( + "i8_p10_s2_gap", + DecimalDType::new(10, 2), + DecimalType::I8, + [-128, 100, 101, 126, 127], + ), + column( + "i8_p4_s2_gap", + DecimalDType::new(4, 2), + DecimalType::I8, + [-99, 0, 99, 100, 127], + ), + column( + "i16_p10_s2_gap", + DecimalDType::new(10, 2), + DecimalType::I16, + [10_000, 10_001, 20_000, 32_766, 32_767], + ), + column( + "i32_p18_s2_gap", + DecimalDType::new(18, 2), + DecimalType::I32, + [ + 1_000_000_000, + 1_000_000_001, + 1_500_000_000, + i32_max - 1, + i32_max, + ], + ), + column( + "i64_p38_s0_gap", + DecimalDType::new(38, 0), + DecimalType::I64, + [ + ten_pow_18, + ten_pow_18 + 1, + 5 * ten_pow_18, + i64_max - 1, + i64_max, + ], + ), + ], + } +} + +/// Columns stored narrower than the canonical width, where the scale sets the smallest precision +/// that a narrower child can have. +fn high_scale() -> LegacyDecimalFixture { + LegacyDecimalFixture { + name: "decimal_high_scale.vortex", + description: "Decimal arrays stored narrower than canonical, with a scale near the \ + precision", + columns: vec![ + column( + "i8_p18_s10", + DecimalDType::new(18, 10), + DecimalType::I8, + [-99, -1, 0, 1, 99], + ), + column( + "i8_p18_s5", + DecimalDType::new(18, 5), + DecimalType::I8, + [-99, -1, 0, 1, 99], + ), + column( + "i16_p38_s20", + DecimalDType::new(38, 20), + DecimalType::I16, + [-9_999, -1, 0, 1, 9_999], + ), + column( + "i8_p4_s4", + DecimalDType::new(4, 4), + DecimalType::I8, + [-99, -1, 0, 1, 99], + ), + ], + } +} + +/// Columns stored wider than the canonical width, as an import of an Arrow decimal type can store +/// them. +fn wide_storage() -> LegacyDecimalFixture { + let max_p18 = 10i128.pow(18) - 1; + let max_p38 = 10i128.pow(38) - 1; + + LegacyDecimalFixture { + name: "decimal_wide_storage.vortex", + description: "Decimal arrays stored wider than the canonical width of their precision", + columns: vec![ + column( + "i128_p5_s2", + DecimalDType::new(5, 2), + DecimalType::I128, + [-99_999, -1, 0, 1, 99_999], + ), + column( + "i128_p18_s2", + DecimalDType::new(18, 2), + DecimalType::I128, + [-max_p18, -1, 0, 1, max_p18], + ), + column( + "i64_p4_s2", + DecimalDType::new(4, 2), + DecimalType::I64, + [-9_999, -1, 0, 1, 9_999], + ), + column( + "i32_p2_s1", + DecimalDType::new(2, 1), + DecimalType::I32, + [-99, -1, 0, 1, 99], + ), + column( + "i256_p38_s2", + DecimalDType::new(38, 2), + DecimalType::I256, + [-max_p38, -1, 0, 1, max_p38], + ), + ], + } +} + +/// Nullable columns with nonzero values in their null slots. Each null slot holds a value that is +/// outside the precision, or outside the precision of the narrowest child that the valid values +/// allow. +fn null_slots() -> LegacyDecimalFixture { + let validity = [true, false, true, false, true]; + let i32_max: i128 = i32::MAX.into(); + let outside_p5 = 10i128.pow(30); + + LegacyDecimalFixture { + name: "decimal_null_slots.vortex", + description: "Nullable decimal arrays with nonzero values in null slots", + columns: vec![ + nullable_column( + "canonical_i32_p9_s2", + DecimalDType::new(9, 2), + DecimalType::I32, + [-999_999_999, i32_max, 0, i32_max, 999_999_999], + validity, + ), + nullable_column( + "narrow_i8_p10_s2", + DecimalDType::new(10, 2), + DecimalType::I8, + [-99, 127, 0, -128, 99], + validity, + ), + nullable_column( + "narrow_i8_p4_s2", + DecimalDType::new(4, 2), + DecimalType::I8, + [-99, 127, 0, 127, 99], + validity, + ), + nullable_column( + "wide_i128_p5_s2", + DecimalDType::new(5, 2), + DecimalType::I128, + [-99_999, outside_p5, 0, outside_p5, 99_999], + validity, + ), + ], + } +} diff --git a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/decimal_widths.rs b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/decimal_widths.rs new file mode 100644 index 00000000000..079c3fe8b86 --- /dev/null +++ b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/decimal_widths.rs @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +use vortex_array::ArrayId; +use vortex_array::ArrayRef; +use vortex_array::ArrayVTable; +use vortex_array::ExecutionCtx; +use vortex_array::IntoArray; +use vortex_array::arrays::Decimal; +use vortex_array::arrays::DecimalArray; +use vortex_array::arrays::StructArray; +use vortex_array::dtype::DecimalDType; +use vortex_array::dtype::i256; +use vortex_array::validity::Validity; +use vortex_buffer::buffer; +use vortex_error::VortexExpect; +use vortex_error::VortexResult; + +use crate::fixtures::FlatLayoutFixture; + +/// Decimal columns that are stored at the canonical width of their precision. +/// +/// Each column holds the largest and the smallest value that its precision allows. The columns +/// cover the narrowest and the widest precision of the `i8`, `i16`, `i128` and `i256` widths. The +/// `i32` and `i64` widths are in `decimal.vortex`. +pub struct DecimalCanonicalWidthsFixture; + +impl FlatLayoutFixture for DecimalCanonicalWidthsFixture { + fn name(&self) -> &str { + "decimal_canonical_widths.vortex" + } + + fn description(&self) -> &str { + "Decimal arrays at the canonical width of each precision, with the bounds of each precision" + } + + fn expected_encodings(&self) -> Vec { + vec![Decimal.id()] + } + + fn build(&self, _ctx: &mut ExecutionCtx) -> VortexResult { + let max_p19 = 10i128.pow(19) - 1; + let max_p38 = 10i128.pow(38) - 1; + let max_p39 = i256_max_by_precision(39); + let max_p76 = i256_max_by_precision(76); + + let i8_p1_s0 = DecimalArray::new( + buffer![-9i8, -1, 0, 1, 9], + DecimalDType::new(1, 0), + Validity::NonNullable, + ); + let i8_p2_s1 = DecimalArray::new( + buffer![-99i8, -1, 0, 1, 99], + DecimalDType::new(2, 1), + Validity::NonNullable, + ); + let i16_p3_s1 = DecimalArray::new( + buffer![-999i16, -1, 0, 1, 999], + DecimalDType::new(3, 1), + Validity::NonNullable, + ); + let i16_p4_s2 = DecimalArray::new( + buffer![-9_999i16, -1, 0, 1, 9_999], + DecimalDType::new(4, 2), + Validity::NonNullable, + ); + let i128_p19_s2 = DecimalArray::new( + buffer![-max_p19, -1, 0, 1, max_p19], + DecimalDType::new(19, 2), + Validity::NonNullable, + ); + let i128_p38_s2 = DecimalArray::new( + buffer![-max_p38, -1, 0, 1, max_p38], + DecimalDType::new(38, 2), + Validity::NonNullable, + ); + let i256_p39_s2 = DecimalArray::new( + buffer![-max_p39, -i256::ONE, i256::ZERO, i256::ONE, max_p39], + DecimalDType::new(39, 2), + Validity::NonNullable, + ); + let i256_p76_s0 = DecimalArray::new( + buffer![-max_p76, -i256::ONE, i256::ZERO, i256::ONE, max_p76], + DecimalDType::new(76, 0), + Validity::NonNullable, + ); + let i128_p38_s2_nullable = DecimalArray::from_option_iter( + [Some(-max_p38), None, Some(0), None, Some(max_p38)], + DecimalDType::new(38, 2), + ); + + let arr = StructArray::try_from_iter([ + ("i8_p1_s0", i8_p1_s0.into_array()), + ("i8_p2_s1", i8_p2_s1.into_array()), + ("i16_p3_s1", i16_p3_s1.into_array()), + ("i16_p4_s2", i16_p4_s2.into_array()), + ("i128_p19_s2", i128_p19_s2.into_array()), + ("i128_p38_s2", i128_p38_s2.into_array()), + ("i256_p39_s2", i256_p39_s2.into_array()), + ("i256_p76_s0", i256_p76_s0.into_array()), + ("i128_p38_s2_nullable", i128_p38_s2_nullable.into_array()), + ])?; + + Ok(arr.into_array()) + } +} + +/// Returns `10^precision - 1`, the largest value of a decimal with this precision. +fn i256_max_by_precision(precision: u32) -> i256 { + i256::from_i128(10) + .checked_pow(precision) + .vortex_expect("10^precision fits in i256 for every decimal precision") + - i256::ONE +} diff --git a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/mod.rs b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/mod.rs index 18d0cc124c3..fe1f2e4e532 100644 --- a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/mod.rs +++ b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/arrays/mod.rs @@ -9,6 +9,8 @@ mod bool; mod chunked; mod datetime; mod decimal; +mod decimal_legacy; +mod decimal_widths; mod fixed_size_list; mod list; mod listview; @@ -19,11 +21,14 @@ mod struct_nested; mod varbin; mod varbinview; +#[cfg(test)] +pub(super) use decimal_legacy::fixtures as legacy_decimal_fixtures; + use crate::fixtures::FlatLayoutFixture; /// All per-array-type fixtures. pub fn fixtures() -> Vec> { - vec![ + let mut fixtures: Vec> = vec![ Box::new(primitive::PrimitivesFixture), Box::new(varbin::VarBinFixture), Box::new(varbinview::VarBinViewFixture), @@ -35,7 +40,13 @@ pub fn fixtures() -> Vec> { Box::new(null::NullFixture), Box::new(datetime::DateTimeFixture), Box::new(decimal::DecimalFixture), + Box::new(decimal_widths::DecimalCanonicalWidthsFixture), Box::new(listview::ListViewFixture), Box::new(map::MapFixture), - ] + ]; + fixtures.extend( + decimal_legacy::fixtures() + .map(|fixture| -> Box { Box::new(fixture) }), + ); + fixtures } diff --git a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/decimal_byte_parts_narrow_msp.rs b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/decimal_byte_parts_narrow_msp.rs new file mode 100644 index 00000000000..0478139695c --- /dev/null +++ b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/decimal_byte_parts_narrow_msp.rs @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +use vortex::array::ArrayId; +use vortex::array::ArrayRef; +use vortex::array::ArrayVTable; +use vortex::array::IntoArray; +use vortex::array::arrays::PrimitiveArray; +use vortex::array::arrays::StructArray; +use vortex::array::dtype::DecimalDType; +use vortex::array::validity::Validity; +use vortex::buffer::Buffer; +use vortex::encodings::decimal_byte_parts::DecimalByteParts; +use vortex::error::VortexExpect; +use vortex::error::VortexResult; +use vortex_array::ExecutionCtx; + +use super::N; +use crate::fixtures::FlatLayoutFixture; + +/// DecimalByteParts columns whose MSP width differs from the canonical width of the decimal dtype. +/// +/// The `decimal_byte_parts.vortex` fixture covers only `i64` MSPs. +pub struct DecimalBytePartsNarrowMspFixture; + +impl FlatLayoutFixture for DecimalBytePartsNarrowMspFixture { + fn name(&self) -> &str { + "decimal_byte_parts_narrow_msp.vortex" + } + + fn description(&self) -> &str { + "DecimalByteParts arrays with an MSP narrower or wider than the canonical decimal width" + } + + fn expected_encodings(&self) -> Vec { + vec![DecimalByteParts.id()] + } + + fn build(&self, _ctx: &mut ExecutionCtx) -> VortexResult { + let max_p18 = 10i64.pow(18) - 1; + + let msp_i8_p10_s2 = evenly_spaced(i8::MIN, i8::MAX).into_array(); + let msp_i16_p10_s2_gap = evenly_spaced(10_000i16, i16::MAX).into_array(); + let msp_i32_p18_s2 = evenly_spaced(-999_999_999i32, 999_999_999).into_array(); + let msp_i32_p18_s2_gap = evenly_spaced(1_000_000_000i32, i32::MAX).into_array(); + let msp_i64_p38_s2 = evenly_spaced(-max_p18, max_p18).into_array(); + let msp_i64_p4_s2_wide = evenly_spaced(-9_999i64, 9_999).into_array(); + + // The null slots hold `i32::MAX`, a nonzero value. The remainder 3 keeps the first and the + // last rows valid, so the valid values still reach both bounds. + let is_null = |idx: usize| idx % 7 == 3; + let nullable_values: Buffer = evenly_spaced(-999_999_999i32, 999_999_999) + .into_iter() + .enumerate() + .map(|(idx, value)| if is_null(idx) { i32::MAX } else { value }) + .collect(); + let nullable_validity = Validity::from_iter((0..N).map(|idx| !is_null(idx))); + let msp_i32_p18_s2_nullable = + PrimitiveArray::new(nullable_values, nullable_validity).into_array(); + + let arr = StructArray::try_from_iter([ + ( + "msp_i8_p10_s2", + DecimalByteParts::try_new(msp_i8_p10_s2, DecimalDType::new(10, 2))?, + ), + ( + "msp_i16_p10_s2_gap", + DecimalByteParts::try_new(msp_i16_p10_s2_gap, DecimalDType::new(10, 2))?, + ), + ( + "msp_i32_p18_s2", + DecimalByteParts::try_new(msp_i32_p18_s2, DecimalDType::new(18, 2))?, + ), + ( + "msp_i32_p18_s2_gap", + DecimalByteParts::try_new(msp_i32_p18_s2_gap, DecimalDType::new(18, 2))?, + ), + ( + "msp_i64_p38_s2", + DecimalByteParts::try_new(msp_i64_p38_s2, DecimalDType::new(38, 2))?, + ), + ( + "msp_i32_p18_s2_nullable", + DecimalByteParts::try_new(msp_i32_p18_s2_nullable, DecimalDType::new(18, 2))?, + ), + ( + "msp_i64_p4_s2_wide", + DecimalByteParts::try_new(msp_i64_p4_s2_wide, DecimalDType::new(4, 2))?, + ), + ])?; + + Ok(arr.into_array()) + } +} + +/// Returns `N` values from `min` to `max`, with both bounds included. +/// +/// The steps are as even as integer division allows. When the range holds fewer than `N` values, +/// some values repeat, and every value of the range appears. +fn evenly_spaced(min: T, max: T) -> Buffer +where + T: Copy + Into + TryFrom, +{ + let (min, max) = (min.into(), max.into()); + let last = (N - 1) as i128; + + (0..=last) + .map(|idx| { + T::try_from(min + (max - min) * idx / last) + .ok() + .vortex_expect("an evenly spaced value lies between min and max") + }) + .collect() +} diff --git a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/mod.rs b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/mod.rs index 463fc5380d7..b292c85c367 100644 --- a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/mod.rs +++ b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/encodings/mod.rs @@ -12,6 +12,7 @@ mod bytebool; mod constant; mod datetimeparts; mod decimal_byte_parts; +mod decimal_byte_parts_narrow_msp; #[expect( dead_code, reason = "Unused until `vortex.decimal_byte_parts.v2` joins an edition" @@ -43,6 +44,7 @@ pub fn fixtures() -> Vec> { Box::new(bytebool::ByteBoolFixture), Box::new(datetimeparts::DateTimePartsFixture), Box::new(decimal_byte_parts::DecimalBytePartsFixture), + Box::new(decimal_byte_parts_narrow_msp::DecimalBytePartsNarrowMspFixture), // Re-enable once `vortex.decimal_byte_parts.v2` joins an edition // Box::new(decimal_byte_parts_v2::DecimalBytePartsV2Fixture), // Re-enable this once delta is stable diff --git a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/mod.rs b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/mod.rs index de8c2165854..e3b10450113 100644 --- a/vortex-test/compat-gen/src/fixtures/arrays/synthetic/mod.rs +++ b/vortex-test/compat-gen/src/fixtures/arrays/synthetic/mod.rs @@ -20,24 +20,139 @@ mod tests { use std::sync::LazyLock; use vortex_array::VortexSessionExecute; + use vortex_array::arrays::Decimal; + use vortex_array::arrays::DecimalArray; + use vortex_array::arrays::Struct; + use vortex_array::arrays::StructArray; + use vortex_array::arrays::decimal::DecimalArrayExt; + use vortex_array::arrays::struct_::StructArrayExt; use vortex_array::assert_arrays_eq; + use vortex_array::dtype::BigCast; + use vortex_array::dtype::DecimalType; + use vortex_array::match_each_decimal_value_type; + use vortex_error::VortexExpect; + use vortex_error::VortexResult; use vortex_session::VortexSession; + use super::arrays::legacy_decimal_fixtures; use super::fixtures; use crate::adapter; + use crate::fixtures::FlatLayoutFixture; use crate::fixtures::check_expected_encodings; static SESSION: LazyLock = LazyLock::new(VortexSession::empty); + /// Returns the raw value of every slot of `array`, including null slots. + fn raw_slots(array: &DecimalArray) -> Vec { + match_each_decimal_value_type!(array.values_type(), |T| { + array + .buffer::() + .iter() + .map(|&value| { + ::from(value) + .vortex_expect("every legacy slot value is an i128 in its column spec") + }) + .collect() + }) + } + #[test] - fn roundtrip_fixtures_to_bytes() { + fn roundtrip_fixtures_to_bytes() -> VortexResult<()> { for fixture in fixtures() { let mut ctx = SESSION.create_execution_ctx(); - let array = fixture.build(&mut ctx).unwrap(); - check_expected_encodings(&array, fixture.as_ref()).unwrap(); - let bytes = adapter::write_file_to_bytes(array.clone()).unwrap(); - let roundtripped = adapter::read_file(bytes).unwrap(); - assert_arrays_eq!(array, roundtripped, &mut ctx); + let expected = fixture.build(&mut ctx)?; + let written = fixture.build_written(&mut ctx)?; + assert_arrays_eq!(written, expected, &mut ctx); + + check_expected_encodings(&written, fixture.as_ref())?; + adapter::compute_all_stats(&written, &mut ctx)?; + let bytes = adapter::write_file_to_bytes(written)?; + let roundtripped = adapter::read_file(bytes)?; + assert_arrays_eq!(roundtripped, expected, &mut ctx); } + + Ok(()) + } + + #[test] + fn fixtures_build_canonical_decimal_widths() -> VortexResult<()> { + for fixture in fixtures() { + let mut ctx = SESSION.create_execution_ctx(); + let expected = fixture.build(&mut ctx)?; + for node in expected.depth_first_traversal() { + let Some(decimal) = node.as_opt::() else { + continue; + }; + let canonical_type = + DecimalType::smallest_decimal_value_type(&decimal.decimal_dtype()); + assert_eq!( + decimal.values_type(), + canonical_type, + "a decimal leaf of {} with dtype {}", + fixture.name(), + node.dtype(), + ); + } + } + + Ok(()) + } + + #[test] + fn legacy_decimal_fixtures_keep_stored_widths() -> VortexResult<()> { + for fixture in legacy_decimal_fixtures() { + let mut ctx = SESSION.create_execution_ctx(); + let written = fixture.build_written(&mut ctx)?; + let bytes = adapter::write_file_to_bytes(written.clone())?; + let read_back = adapter::read_file(bytes)?.execute::(&mut ctx)?; + let written = written.as_::(); + + for column in fixture.columns() { + let context = format!("column {} of {}", column.name, fixture.name()); + + // A typo in a column spec can make a column canonical, and the column then tests + // nothing. The name states the intended stored width, and only a column whose + // name starts with `canonical_` is meant to be stored at the canonical width. + let precision = column.decimal_dtype.precision(); + let scale = column.decimal_dtype.scale(); + let spec_name = format!("{}_p{precision}_s{scale}", column.stored_type); + assert!( + column.name.contains(&spec_name), + "{context}: the name must contain {spec_name}", + ); + let canonical_type = + DecimalType::smallest_decimal_value_type(&column.decimal_dtype); + assert_eq!( + column.stored_type == canonical_type, + column.name.starts_with("canonical_"), + "{context}: only a column named `canonical_*` has the canonical width \ + {canonical_type}", + ); + + let written_field = written.unmasked_field_by_name(column.name)?; + assert_eq!( + written_field.as_::().values_type(), + column.stored_type, + "{context}", + ); + + let read_field = read_back + .unmasked_field_by_name(column.name)? + .clone() + .execute::(&mut ctx)?; + assert_eq!( + read_field.values_type(), + column.stored_type, + "{context} after a file round trip", + ); + assert_eq!( + raw_slots(&read_field), + column.slots, + "{context} after a file round trip", + ); + } + } + + Ok(()) } } diff --git a/vortex-test/compat-gen/src/fixtures/mod.rs b/vortex-test/compat-gen/src/fixtures/mod.rs index 3749a1f1483..4a6980c9d57 100644 --- a/vortex-test/compat-gen/src/fixtures/mod.rs +++ b/vortex-test/compat-gen/src/fixtures/mod.rs @@ -32,6 +32,15 @@ pub trait Fixture { /// Generate the fixture file(s) into `dir`, returning manifest entries. fn write(&self, dir: &Path, ctx: &mut ExecutionCtx) -> VortexResult>; + + /// Write the file that `check` compares with a published file. Defaults to [`Self::write`]. + fn write_expected( + &self, + dir: &Path, + ctx: &mut ExecutionCtx, + ) -> VortexResult> { + self.write(dir, ctx) + } } /// A deterministic fixture that produces a single array written via flat layout (no compression). @@ -45,7 +54,16 @@ pub trait FlatLayoutFixture { /// Build the expected arrays. Must be deterministic under all versions of vortex. fn build(&self, ctx: &mut ExecutionCtx) -> VortexResult; - /// Encoding IDs that must appear somewhere in the array tree produced by [`Self::build`]. + /// Build the arrays that `generate` writes. Defaults to [`Self::build`]. + /// + /// Override this only to write an old physical form that [`Self::build`] cannot return. The + /// result has the dtype and the logical values of [`Self::build`]. + fn build_written(&self, ctx: &mut ExecutionCtx) -> VortexResult { + self.build(ctx) + } + + /// Encoding IDs that must appear somewhere in the array tree produced by + /// [`Self::build_written`]. /// /// Only include encodings that the fixture is specifically testing, not incidental ones /// (e.g. a primitives fixture should not list struct even if it wraps values in one). @@ -76,8 +94,22 @@ pub trait DatasetFixture { // --------------------------------------------------------------------------- /// Adapts a [`FlatLayoutFixture`] into a [`Fixture`] by writing through the flat layout strategy. +/// +/// [`Fixture::write`] writes [`FlatLayoutFixture::build_written`], and +/// [`Fixture::write_expected`] writes [`FlatLayoutFixture::build`]. pub struct FlatLayoutAdapter(pub Box); +impl FlatLayoutAdapter { + fn write_array(&self, dir: &Path, array: ArrayRef) -> VortexResult> { + let path = dir.join(self.name()); + adapter::write_file(&path, array)?; + Ok(vec![FixtureEntry { + name: self.name().to_string(), + description: self.description().to_string(), + }]) + } +} + impl Fixture for FlatLayoutAdapter { fn name(&self) -> &str { self.0.name() @@ -88,15 +120,20 @@ impl Fixture for FlatLayoutAdapter { } fn write(&self, dir: &Path, ctx: &mut ExecutionCtx) -> VortexResult> { - let array = self.0.build(ctx)?; + let array = self.0.build_written(ctx)?; check_expected_encodings(&array, self.0.as_ref())?; compute_all_stats(&array, ctx)?; - let path = dir.join(self.name()); - adapter::write_file(&path, array)?; - Ok(vec![FixtureEntry { - name: self.name().to_string(), - description: self.description().to_string(), - }]) + self.write_array(dir, array) + } + + fn write_expected( + &self, + dir: &Path, + ctx: &mut ExecutionCtx, + ) -> VortexResult> { + let array = self.0.build(ctx)?; + compute_all_stats(&array, ctx)?; + self.write_array(dir, array) } }