From 47ba532728f3f0bc4ea871a117bc7a6a830133e6 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Fri, 31 Jan 2025 17:17:52 +0100 Subject: [PATCH 1/3] original_expr is no longer Option in GeneratedColumn --- crates/modelardb_storage/src/lib.rs | 6 +----- .../src/metadata/model_table_metadata.rs | 10 +++++----- .../src/metadata/table_metadata_manager.rs | 6 +++--- crates/modelardb_storage/src/parser.rs | 2 +- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/crates/modelardb_storage/src/lib.rs b/crates/modelardb_storage/src/lib.rs index c2388b214..63d37ffc4 100644 --- a/crates/modelardb_storage/src/lib.rs +++ b/crates/modelardb_storage/src/lib.rs @@ -454,11 +454,7 @@ fn generated_columns_to_list_array(generated_columns: Vec, /// Original representation of `expr`. It is copied from the SQL statement, so it can be stored /// in the metadata Delta Lake as `expr` does not implement serialization and deserialization. - pub original_expr: Option, + pub original_expr: String, } impl GeneratedColumn { @@ -235,7 +235,7 @@ impl GeneratedColumn { Ok(Self { expr, source_columns: source_columns?, - original_expr: Some(sql_expr.to_owned()), + original_expr: sql_expr.to_owned(), }) } } @@ -371,7 +371,7 @@ mod test { options: wild_card_options.clone(), }, source_columns: vec![], - original_expr: None, + original_expr: "".to_owned(), }); generated_columns[6] = Some(GeneratedColumn { @@ -380,7 +380,7 @@ mod test { options: wild_card_options, }, source_columns: vec![5], - original_expr: None, + original_expr: "".to_owned(), }); let result = ModelTableMetadata::try_new( @@ -486,7 +486,7 @@ mod test { let expected_generated_column = GeneratedColumn { expr: col("field_1") + col("field_2"), source_columns: vec![0, 1], - original_expr: Some(sql_expr.to_owned()), + original_expr: sql_expr.to_owned(), }; let df_schema = schema.to_dfschema().unwrap(); diff --git a/crates/modelardb_storage/src/metadata/table_metadata_manager.rs b/crates/modelardb_storage/src/metadata/table_metadata_manager.rs index 4a92c117e..063789493 100644 --- a/crates/modelardb_storage/src/metadata/table_metadata_manager.rs +++ b/crates/modelardb_storage/src/metadata/table_metadata_manager.rs @@ -382,10 +382,10 @@ impl TableMetadataManager { .enumerate() { if model_table_metadata.is_field(query_schema_index) { - let generated_column_expr = if let Some(generated_column) = + let maybe_generated_column_expr = if let Some(generated_column) = &model_table_metadata.generated_columns[query_schema_index] { - generated_column.original_expr.clone() + Some(generated_column.original_expr.clone()) } else { None }; @@ -412,7 +412,7 @@ impl TableMetadataManager { Arc::new(Int16Array::from(vec![query_schema_index as i16])), Arc::new(Float32Array::from(vec![error_bound_value])), Arc::new(BooleanArray::from(vec![error_bound_is_relative])), - Arc::new(StringArray::from(vec![generated_column_expr])), + Arc::new(StringArray::from(vec![maybe_generated_column_expr])), ], ) .await?; diff --git a/crates/modelardb_storage/src/parser.rs b/crates/modelardb_storage/src/parser.rs index 960e16c3e..8d8eb8d8d 100644 --- a/crates/modelardb_storage/src/parser.rs +++ b/crates/modelardb_storage/src/parser.rs @@ -979,7 +979,7 @@ fn extract_generation_exprs_for_all_columns( // Lake, it is not stored in ModelTableMetadata as it not used for during query // execution. let sql_expr = generation_expr.as_ref().unwrap(); - let original_expr = Some(sql_expr.to_string()); + let original_expr = sql_expr.to_string(); // Ensure that the parsed sqlparser expression can be converted to a logical Apache // Arrow DataFusion expression within the context of schema to check it for errors. From 983f8151aa625125844c929d05c55974e0b140d5 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Fri, 31 Jan 2025 17:31:17 +0100 Subject: [PATCH 2/3] Fixed tests after changing GeneratedColumn --- .../modelardb_storage/src/metadata/table_metadata_manager.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/modelardb_storage/src/metadata/table_metadata_manager.rs b/crates/modelardb_storage/src/metadata/table_metadata_manager.rs index 063789493..3ba09f6bf 100644 --- a/crates/modelardb_storage/src/metadata/table_metadata_manager.rs +++ b/crates/modelardb_storage/src/metadata/table_metadata_manager.rs @@ -1388,13 +1388,13 @@ mod tests { let plus_one_column = Some(GeneratedColumn { expr: col("field_1") + Literal(Int64(Some(1))), source_columns: vec![1], - original_expr: Some("field_1 + 1".to_owned()), + original_expr: "field_1 + 1".to_owned(), }); let addition_column = Some(GeneratedColumn { expr: col("field_1") + col("field_2"), source_columns: vec![1, 2], - original_expr: Some("field_1 + field_2".to_owned()), + original_expr: "field_1 + field_2".to_owned(), }); let expected_generated_columns = From adc096db8a539a96b23d5f716eb0b87f39a50dc3 Mon Sep 17 00:00:00 2001 From: CGodiksen <36046286+CGodiksen@users.noreply.github.com> Date: Fri, 31 Jan 2025 17:42:45 +0100 Subject: [PATCH 3/3] Fixed clippy issue and reformatted --- crates/modelardb_storage/src/lib.rs | 3 ++- .../src/metadata/table_metadata_manager.rs | 11 ++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/modelardb_storage/src/lib.rs b/crates/modelardb_storage/src/lib.rs index 63d37ffc4..b9a99fee8 100644 --- a/crates/modelardb_storage/src/lib.rs +++ b/crates/modelardb_storage/src/lib.rs @@ -454,7 +454,8 @@ fn generated_columns_to_list_array(generated_columns: Vec