From 9d2485cb94d489abedb846a55cdb6bfb8f390762 Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Tue, 19 Mar 2024 18:31:43 -0700 Subject: [PATCH] fix: insert nulls in place of missing database document fields --- .../src/mongodb/projection.rs | 20 ++++++------- .../src/mongodb/selection.rs | 28 +++++++++++-------- .../mongodb-agent-common/src/query/foreach.rs | 20 ++++++++++--- crates/mongodb-agent-common/src/query/mod.rs | 6 ++-- .../src/query/relations.rs | 28 +++++++++---------- 5 files changed, 60 insertions(+), 42 deletions(-) diff --git a/crates/mongodb-agent-common/src/mongodb/projection.rs b/crates/mongodb-agent-common/src/mongodb/projection.rs index d9820561..2cf57f41 100644 --- a/crates/mongodb-agent-common/src/mongodb/projection.rs +++ b/crates/mongodb-agent-common/src/mongodb/projection.rs @@ -5,7 +5,7 @@ use serde::Serialize; use dc_api_types::Field; -use crate::mongodb::selection::format_col_path; +use crate::mongodb::selection::serialized_null_checked_column_reference; /// A projection determines which fields to request from the result of a query. /// @@ -58,7 +58,7 @@ fn project_field_as(parent_columns: &[&str], field: &Field) -> ProjectAs { [] => format!("${column}"), _ => format!("${}.{}", parent_columns.join("."), column), }; - let bson_col_path = format_col_path(col_path, column_type); + let bson_col_path = serialized_null_checked_column_reference(col_path, column_type); ProjectAs::Expression(bson_col_path) } Field::NestedObject { column, query } => { @@ -198,18 +198,18 @@ mod tests { assert_eq!( to_document(&projection)?, doc! { - "foo": "$foo", - "foo_again": "$foo", + "foo": { "$ifNull": ["$foo", null] }, + "foo_again": { "$ifNull": ["$foo", null] }, "bar": { - "baz": "$bar.baz", - "baz_again": "$bar.baz" + "baz": { "$ifNull": ["$bar.baz", null] }, + "baz_again": { "$ifNull": ["$bar.baz", null] } }, "bar_again": { - "baz": "$bar.baz" + "baz": { "$ifNull": ["$bar.baz", null] } }, "my_date": { "$dateToString": { - "date": "$my_date" + "date": { "$ifNull": ["$my_date", null] } } } } @@ -260,10 +260,10 @@ mod tests { to_document(&projection)?, doc! { "class_students": { - "name": "$class_students.name", + "name": { "$ifNull": ["$class_students.name", null] }, }, "students": { - "student_name": "$class_students.name", + "student_name": { "$ifNull": ["$class_students.name", null] }, }, } ); diff --git a/crates/mongodb-agent-common/src/mongodb/selection.rs b/crates/mongodb-agent-common/src/mongodb/selection.rs index c3fbedf7..966350d7 100644 --- a/crates/mongodb-agent-common/src/mongodb/selection.rs +++ b/crates/mongodb-agent-common/src/mongodb/selection.rs @@ -58,8 +58,14 @@ fn from_query_request_helper( /// If column_type is date we want to format it as a string. /// TODO: do we want to format any other BSON types in any particular way, /// e.g. formated ObjectId as string? -pub fn format_col_path(col_path: String, column_type: &str) -> Bson { +/// +/// Wraps column reference with an `$isNull` check. That catches cases where a field is missing +/// from a document, and substitutes a concrete null value. Otherwise the field would be omitted +/// from query results which leads to an error in the engine. +pub fn serialized_null_checked_column_reference(col_path: String, column_type: &str) -> Bson { + let col_path = doc! { "$ifNull": [col_path, Bson::Null] }; match column_type { + // Don't worry, $dateToString will returns `null` if `col_path` is null "date" => bson!({"$dateToString": {"date": col_path}}), _ => bson!(col_path), } @@ -80,7 +86,7 @@ fn selection_for_field( [] => format!("${column}"), _ => format!("${}.{}", parent_columns.join("."), column), }; - let bson_col_path = format_col_path(col_path, column_type); + let bson_col_path = serialized_null_checked_column_reference(col_path, column_type); Ok(bson_col_path) } Field::NestedObject { column, query } => { @@ -258,14 +264,14 @@ mod tests { assert_eq!( Into::::into(selection), doc! { - "foo": "$foo", - "foo_again": "$foo", + "foo": { "$ifNull": ["$foo", null] }, + "foo_again": { "$ifNull": ["$foo", null] }, "bar": { "$cond": { "if": "$bar", "then": { - "baz": "$bar.baz", - "baz_again": "$bar.baz" + "baz": { "$ifNull": ["$bar.baz", null] }, + "baz_again": { "$ifNull": ["$bar.baz", null] } }, "else": null } @@ -274,24 +280,24 @@ mod tests { "$cond": { "if": "$bar", "then": { - "baz": "$bar.baz" + "baz": { "$ifNull": ["$bar.baz", null] } }, "else": null } }, "my_date": { "$dateToString": { - "date": "$my_date" + "date": { "$ifNull": ["$my_date", null] } } }, - "array_of_scalars": "$foo", + "array_of_scalars": { "$ifNull": ["$foo", null] }, "array_of_objects": { "$cond": { "if": "$foo", "then": { "$map": { "input": "$foo", - "in": {"baz": "$$this.baz"} + "in": {"baz": { "$ifNull": ["$$this.baz", null] }} } }, "else": null @@ -306,7 +312,7 @@ mod tests { "in": { "$map": { "input": "$$this", - "in": {"baz": "$$this.baz"} + "in": {"baz": { "$ifNull": ["$$this.baz", null] }} } } } diff --git a/crates/mongodb-agent-common/src/query/foreach.rs b/crates/mongodb-agent-common/src/query/foreach.rs index a01c55d4..a70e0ffc 100644 --- a/crates/mongodb-agent-common/src/query/foreach.rs +++ b/crates/mongodb-agent-common/src/query/foreach.rs @@ -176,11 +176,17 @@ mod tests { "$facet": { "__FACET___0": [ { "$match": { "$and": [{ "artistId": {"$eq":1 }}]}}, - { "$replaceWith": { "albumId":"$albumId","title":"$title" }}, + { "$replaceWith": { + "albumId": { "$ifNull": ["$albumId", null] }, + "title": { "$ifNull": ["$title", null] } + } }, ], "__FACET___1": [ { "$match": { "$and": [{ "artistId": {"$eq":2}}]}}, - { "$replaceWith": { "albumId":"$albumId","title":"$title" }}, + { "$replaceWith": { + "albumId": { "$ifNull": ["$albumId", null] }, + "title": { "$ifNull": ["$title", null] } + } }, ] }, }, @@ -284,7 +290,10 @@ mod tests { "__FACET___0": [ { "$match": { "$and": [{ "artistId": {"$eq": 1 }}]}}, { "$facet": { - "__ROWS__": [{ "$replaceWith": { "albumId": "$albumId", "title": "$title" }}], + "__ROWS__": [{ "$replaceWith": { + "albumId": { "$ifNull": ["$albumId", null] }, + "title": { "$ifNull": ["$title", null] } + }}], "count": [{ "$count": "result" }], } }, { "$replaceWith": { @@ -300,7 +309,10 @@ mod tests { "__FACET___1": [ { "$match": { "$and": [{ "artistId": {"$eq": 2 }}]}}, { "$facet": { - "__ROWS__": [{ "$replaceWith": { "albumId": "$albumId", "title": "$title" }}], + "__ROWS__": [{ "$replaceWith": { + "albumId": { "$ifNull": ["$albumId", null] }, + "title": { "$ifNull": ["$title", null] } + }}], "count": [{ "$count": "result" }], } }, { "$replaceWith": { diff --git a/crates/mongodb-agent-common/src/query/mod.rs b/crates/mongodb-agent-common/src/query/mod.rs index ed0abc68..77b506e6 100644 --- a/crates/mongodb-agent-common/src/query/mod.rs +++ b/crates/mongodb-agent-common/src/query/mod.rs @@ -77,7 +77,7 @@ mod tests { let expected_pipeline = json!([ { "$match": { "gpa": { "$lt": 4.0 } } }, - { "$replaceWith": { "student_gpa": "$gpa" } }, + { "$replaceWith": { "student_gpa": { "$ifNull": ["$gpa", null] } } }, ]); let mut collection = MockCollectionTrait::new(); @@ -222,7 +222,7 @@ mod tests { ], "__ROWS__": [{ "$replaceWith": { - "student_gpa": "$gpa", + "student_gpa": { "$ifNull": ["$gpa", null] }, }, }], }, @@ -300,7 +300,7 @@ mod tests { "$replaceWith": { "date": { "$dateToString": { - "date": "$date", + "date": { "$ifNull": ["$date", null] }, }, }, } diff --git a/crates/mongodb-agent-common/src/query/relations.rs b/crates/mongodb-agent-common/src/query/relations.rs index 98a09056..07e0d62f 100644 --- a/crates/mongodb-agent-common/src/query/relations.rs +++ b/crates/mongodb-agent-common/src/query/relations.rs @@ -300,7 +300,7 @@ mod tests { }, { "$replaceWith": { - "student_name": "$name", + "student_name": { "$ifNull": ["$name", null] }, }, } ], @@ -309,7 +309,7 @@ mod tests { }, { "$replaceWith": { - "class_title": "$title", + "class_title": { "$ifNull": ["$title", null] }, "students": { "rows": { "$getField": { "$literal": "students" }, @@ -399,7 +399,7 @@ mod tests { }, { "$replaceWith": { - "class_title": "$title", + "class_title": { "$ifNull": ["$title", null] }, }, } ], @@ -408,7 +408,7 @@ mod tests { }, { "$replaceWith": { - "student_name": "$name", + "student_name": { "$ifNull": ["$name", null] }, "class": { "rows": { "$getField": { "$literal": "class" } } }, @@ -502,7 +502,7 @@ mod tests { }, { "$replaceWith": { - "student_name": "$name", + "student_name": { "$ifNull": ["$name", null] }, }, }, ], @@ -511,7 +511,7 @@ mod tests { }, { "$replaceWith": { - "class_title": "$title", + "class_title": { "$ifNull": ["$title", null] }, "students": { "rows": { "$getField": { "$literal": "students" } }, }, @@ -647,7 +647,7 @@ mod tests { }, { "$replaceWith": { - "assignment_title": "$title", + "assignment_title": { "$ifNull": ["$title", null] }, }, }, ], @@ -659,7 +659,7 @@ mod tests { "assignments": { "rows": { "$getField": { "$literal": "assignments" } }, }, - "student_name": "$name", + "student_name": { "$ifNull": ["$name", null] }, }, }, ], @@ -668,7 +668,7 @@ mod tests { }, { "$replaceWith": { - "class_title": "$title", + "class_title": { "$ifNull": ["$title", null] }, "students": { "rows": { "$getField": { "$literal": "students" } }, }, @@ -905,8 +905,8 @@ mod tests { }, { "$replaceWith": { - "year": "$year", - "title": "$title" + "year": { "$ifNull": ["$year", null] }, + "title": { "$ifNull": ["$title", null] } } } ], @@ -932,7 +932,7 @@ mod tests { } } }, - "name": "$name" + "name": { "$ifNull": ["$name", null] } } }, ]); @@ -1049,7 +1049,7 @@ mod tests { "credits": { "$cond": { "if": "$credits", - "then": { "director": "$credits.director" }, + "then": { "director": { "$ifNull": ["$credits.director", null] } }, "else": null, } }, @@ -1071,7 +1071,7 @@ mod tests { }, { "$replaceWith": { - "name": "$name", + "name": { "$ifNull": ["$name", null] }, "movie": { "rows": { "$getField": {