From f2d4d4fec483721aefe8e99d55a1d96a53154d79 Mon Sep 17 00:00:00 2001 From: Trevor Hilton Date: Thu, 31 Oct 2024 20:58:44 -0400 Subject: [PATCH] refactor: re-introduce catalog snapshot tests update serde code The catalog types no longer serialize the id maps, since their info is intrinsic to the list of schema objects themselves. --- influxdb3_catalog/src/catalog.rs | 25 +- influxdb3_catalog/src/serialize.rs | 91 ++++-- ...catalog__tests__catalog_serialization.snap | 260 ++++++++++++++++++ ..._catalog__tests__serialize_last_cache.snap | 117 ++++++++ ...catalog__tests__serialize_series_keys.snap | 106 +++++++ ...after-last-cache-create-and-new-field.snap | 6 - ...g-immediately-after-last-cache-create.snap | 6 - ...g-immediately-after-last-cache-delete.snap | 6 - 8 files changed, 567 insertions(+), 50 deletions(-) create mode 100644 influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__catalog_serialization.snap create mode 100644 influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_last_cache.snap create mode 100644 influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_series_keys.snap diff --git a/influxdb3_catalog/src/catalog.rs b/influxdb3_catalog/src/catalog.rs index a4df821b12a..830a421fb34 100644 --- a/influxdb3_catalog/src/catalog.rs +++ b/influxdb3_catalog/src/catalog.rs @@ -428,14 +428,12 @@ impl InnerCatalog { } } -#[serde_with::serde_as] -#[derive(Debug, Serialize, Deserialize, Eq, PartialEq, Clone)] +#[derive(Debug, Eq, PartialEq, Clone)] pub struct DatabaseSchema { pub id: DbId, pub name: Arc, /// The database is a map of tables pub tables: SerdeVecMap>, - #[serde_as(as = "TableMapAsArray")] pub table_map: BiHashMap>, } @@ -1091,6 +1089,13 @@ mod tests { .databases .insert(database.id, Arc::new(database)); + insta::with_settings!({ + sort_maps => true, + description => "catalog serialization to help catch breaking changes" + }, { + insta::assert_json_snapshot!(catalog); + }); + // Serialize/deserialize to ensure roundtrip to/from JSON let serialized = serde_json::to_string(&catalog).unwrap(); let deserialized_inner: InnerCatalog = serde_json::from_str(&serialized).unwrap(); @@ -1315,6 +1320,13 @@ mod tests { .databases .insert(database.id, Arc::new(database)); + insta::with_settings!({ + sort_maps => true, + description => "catalog serialization to help catch breaking changes" + }, { + insta::assert_json_snapshot!(catalog); + }); + let serialized = serde_json::to_string(&catalog).unwrap(); let deserialized_inner: InnerCatalog = serde_json::from_str(&serialized).unwrap(); let deserialized = Catalog::from_inner(deserialized_inner); @@ -1372,6 +1384,13 @@ mod tests { .databases .insert(database.id, Arc::new(database)); + insta::with_settings!({ + sort_maps => true, + description => "catalog serialization to help catch breaking changes" + }, { + insta::assert_json_snapshot!(catalog); + }); + let serialized = serde_json::to_string(&catalog).unwrap(); let deserialized_inner: InnerCatalog = serde_json::from_str(&serialized).unwrap(); let deserialized = Catalog::from_inner(deserialized_inner); diff --git a/influxdb3_catalog/src/serialize.rs b/influxdb3_catalog/src/serialize.rs index 60f046e66a8..8eefadaeff5 100644 --- a/influxdb3_catalog/src/serialize.rs +++ b/influxdb3_catalog/src/serialize.rs @@ -1,8 +1,10 @@ use crate::catalog::ColumnDefinition; +use crate::catalog::DatabaseSchema; use crate::catalog::TableDefinition; use arrow::datatypes::DataType as ArrowDataType; use bimap::BiHashMap; use influxdb3_id::ColumnId; +use influxdb3_id::DbId; use influxdb3_id::SerdeVecMap; use influxdb3_id::TableId; use influxdb3_wal::{LastCacheDefinition, LastCacheValueColumnsDef}; @@ -12,6 +14,66 @@ use schema::TIME_DATA_TIMEZONE; use serde::{Deserialize, Serialize}; use std::sync::Arc; +impl Serialize for DatabaseSchema { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let snapshot = DatabaseSnapshot::from(self); + snapshot.serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for DatabaseSchema { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + DatabaseSnapshot::deserialize(deserializer).map(Into::into) + } +} + +#[derive(Debug, Serialize, Deserialize)] +struct DatabaseSnapshot { + id: DbId, + name: Arc, + tables: SerdeVecMap, +} + +impl From<&DatabaseSchema> for DatabaseSnapshot { + fn from(db: &DatabaseSchema) -> Self { + Self { + id: db.id, + name: Arc::clone(&db.name), + tables: db + .tables + .iter() + .map(|(table_id, table_def)| (*table_id, table_def.as_ref().into())) + .collect(), + } + } +} + +impl From for DatabaseSchema { + fn from(snap: DatabaseSnapshot) -> Self { + let mut table_map = BiHashMap::with_capacity(snap.tables.len()); + let tables = snap + .tables + .into_iter() + .map(|(id, table)| { + table_map.insert(id, Arc::clone(&table.table_name)); + (id, Arc::new(table.into())) + }) + .collect(); + Self { + id: snap.id, + name: snap.name, + tables, + table_map, + } + } +} + impl Serialize for TableDefinition { fn serialize(&self, serializer: S) -> Result where @@ -48,35 +110,6 @@ struct TableSnapshot { last_caches: Vec, } -serde_with::serde_conv!( - ColumnMapAsArray, - BiHashMap>, - |map: &BiHashMap>| { - let mut vec = map.iter().fold(Vec::new(), |mut acc, (id, name)| { - acc.push(ColumnMap { - column_id: *id, - name: Arc::clone(&name) - }); - acc - }); - - vec.sort_by_key(|col| col.column_id); - vec - }, - |vec: Vec| -> Result<_, std::convert::Infallible> { - Ok(vec.into_iter().fold(BiHashMap::new(), |mut acc, column| { - acc.insert(column.column_id, column.name); - acc - })) - } -); - -#[derive(Debug, Serialize, Deserialize)] -struct ColumnMap { - column_id: ColumnId, - name: Arc, -} - /// Representation of Arrow's `DataType` for table snapshots. /// /// Uses `#[non_exhaustive]` with the assumption that variants will be added as we support diff --git a/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__catalog_serialization.snap b/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__catalog_serialization.snap new file mode 100644 index 00000000000..e5b78da2b5a --- /dev/null +++ b/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__catalog_serialization.snap @@ -0,0 +1,260 @@ +--- +source: influxdb3_catalog/src/catalog.rs +description: catalog serialization to help catch breaking changes +expression: catalog +--- +{ + "databases": [ + [ + 0, + { + "id": 0, + "name": "test_db", + "tables": [ + [ + 1, + { + "table_id": 1, + "table_name": "test_table_1", + "cols": [ + [ + 5, + { + "name": "bool_field", + "id": 5, + "type": "bool", + "influx_type": "field", + "nullable": true + } + ], + [ + 8, + { + "name": "f64_field", + "id": 8, + "type": "f64", + "influx_type": "field", + "nullable": true + } + ], + [ + 6, + { + "name": "i64_field", + "id": 6, + "type": "i64", + "influx_type": "field", + "nullable": true + } + ], + [ + 4, + { + "name": "string_field", + "id": 4, + "type": "str", + "influx_type": "field", + "nullable": true + } + ], + [ + 0, + { + "name": "tag_1", + "id": 0, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 1, + { + "name": "tag_2", + "id": 1, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 2, + { + "name": "tag_3", + "id": 2, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 3, + { + "name": "time", + "id": 3, + "type": { + "time": [ + "ns", + null + ] + }, + "influx_type": "time", + "nullable": false + } + ], + [ + 7, + { + "name": "u64_field", + "id": 7, + "type": "u64", + "influx_type": "field", + "nullable": true + } + ] + ] + } + ], + [ + 2, + { + "table_id": 2, + "table_name": "test_table_2", + "cols": [ + [ + 14, + { + "name": "bool_field", + "id": 14, + "type": "bool", + "influx_type": "field", + "nullable": true + } + ], + [ + 17, + { + "name": "f64_field", + "id": 17, + "type": "f64", + "influx_type": "field", + "nullable": true + } + ], + [ + 15, + { + "name": "i64_field", + "id": 15, + "type": "i64", + "influx_type": "field", + "nullable": true + } + ], + [ + 13, + { + "name": "string_field", + "id": 13, + "type": "str", + "influx_type": "field", + "nullable": true + } + ], + [ + 9, + { + "name": "tag_1", + "id": 9, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 10, + { + "name": "tag_2", + "id": 10, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 11, + { + "name": "tag_3", + "id": 11, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 12, + { + "name": "time", + "id": 12, + "type": { + "time": [ + "ns", + null + ] + }, + "influx_type": "time", + "nullable": false + } + ], + [ + 16, + { + "name": "u64_field", + "id": 16, + "type": "u64", + "influx_type": "field", + "nullable": true + } + ] + ] + } + ] + ] + } + ] + ], + "sequence": 0, + "host_id": "sample-host-id", + "instance_id": "instance-id", + "db_map": [] +} diff --git a/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_last_cache.snap b/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_last_cache.snap new file mode 100644 index 00000000000..cd109deeeb1 --- /dev/null +++ b/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_last_cache.snap @@ -0,0 +1,117 @@ +--- +source: influxdb3_catalog/src/catalog.rs +description: catalog serialization to help catch breaking changes +expression: catalog +--- +{ + "databases": [ + [ + 0, + { + "id": 0, + "name": "test_db", + "tables": [ + [ + 0, + { + "table_id": 0, + "table_name": "test", + "cols": [ + [ + 4, + { + "name": "field", + "id": 4, + "type": "str", + "influx_type": "field", + "nullable": true + } + ], + [ + 0, + { + "name": "tag_1", + "id": 0, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 1, + { + "name": "tag_2", + "id": 1, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 2, + { + "name": "tag_3", + "id": 2, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": true + } + ], + [ + 3, + { + "name": "time", + "id": 3, + "type": { + "time": [ + "ns", + null + ] + }, + "influx_type": "time", + "nullable": false + } + ] + ], + "last_caches": [ + { + "table_id": 0, + "table": "test", + "name": "test_table_last_cache", + "keys": [ + 1, + 2 + ], + "vals": [ + 4 + ], + "n": 1, + "ttl": 600 + } + ] + } + ] + ] + } + ] + ], + "sequence": 0, + "host_id": "sample-host-id", + "instance_id": "instance-id", + "db_map": [] +} diff --git a/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_series_keys.snap b/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_series_keys.snap new file mode 100644 index 00000000000..5190be04ed4 --- /dev/null +++ b/influxdb3_catalog/src/snapshots/influxdb3_catalog__catalog__tests__serialize_series_keys.snap @@ -0,0 +1,106 @@ +--- +source: influxdb3_catalog/src/catalog.rs +description: catalog serialization to help catch breaking changes +expression: catalog +--- +{ + "databases": [ + [ + 0, + { + "id": 0, + "name": "test_db", + "tables": [ + [ + 1, + { + "table_id": 1, + "table_name": "test_table_1", + "key": [ + 0, + 1, + 2 + ], + "cols": [ + [ + 4, + { + "name": "field", + "id": 4, + "type": "str", + "influx_type": "field", + "nullable": true + } + ], + [ + 0, + { + "name": "tag_1", + "id": 0, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": false + } + ], + [ + 1, + { + "name": "tag_2", + "id": 1, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": false + } + ], + [ + 2, + { + "name": "tag_3", + "id": 2, + "type": { + "dict": [ + "i32", + "str" + ] + }, + "influx_type": "tag", + "nullable": false + } + ], + [ + 3, + { + "name": "time", + "id": 3, + "type": { + "time": [ + "ns", + null + ] + }, + "influx_type": "time", + "nullable": false + } + ] + ] + } + ] + ] + } + ] + ], + "sequence": 0, + "host_id": "sample-host-id", + "instance_id": "instance-id", + "db_map": [] +} diff --git a/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-after-last-cache-create-and-new-field.snap b/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-after-last-cache-create-and-new-field.snap index 9943c95638f..28e92be42fa 100644 --- a/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-after-last-cache-create-and-new-field.snap +++ b/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-after-last-cache-create-and-new-field.snap @@ -9,12 +9,6 @@ expression: catalog_json { "id": 0, "name": "db", - "table_map": [ - { - "name": "table", - "table_id": 0 - } - ], "tables": [ [ 0, diff --git a/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-immediately-after-last-cache-create.snap b/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-immediately-after-last-cache-create.snap index 2ca1d9430a5..acbe22787bf 100644 --- a/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-immediately-after-last-cache-create.snap +++ b/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-immediately-after-last-cache-create.snap @@ -9,12 +9,6 @@ expression: catalog_json { "id": 0, "name": "db", - "table_map": [ - { - "name": "table", - "table_id": 0 - } - ], "tables": [ [ 0, diff --git a/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-immediately-after-last-cache-delete.snap b/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-immediately-after-last-cache-delete.snap index ef47311ae82..57da6135928 100644 --- a/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-immediately-after-last-cache-delete.snap +++ b/influxdb3_write/src/write_buffer/snapshots/influxdb3_write__write_buffer__tests__catalog-immediately-after-last-cache-delete.snap @@ -9,12 +9,6 @@ expression: catalog_json { "id": 0, "name": "db", - "table_map": [ - { - "name": "table", - "table_id": 0 - } - ], "tables": [ [ 0,