accept queries that make array-to-scalar comparisons

crates/integration-tests/src/tests/filtering.rs

@@ -1,6 +1,7 @@
 use insta::assert_yaml_snapshot;
+use ndc_test_helpers::{binop, field, query, query_request, target, variable};
 
-use crate::graphql_query;
+use crate::{connector::Connector, graphql_query, run_connector_query};
 
 #[tokio::test]
 async fn filters_on_extended_json_using_string_comparison() -> anyhow::Result<()> {
@@ -52,3 +53,53 @@ async fn filters_by_comparisons_on_elements_of_array_field() -> anyhow::Result<(
     );
     Ok(())
 }
+
+#[tokio::test]
+async fn filters_by_comparisons_on_elements_of_array_of_scalars() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        graphql_query(
+            r#"
+            query MyQuery {
+              movies(where: { cast: { _eq: "Albert Austin" } }) {
+                title
+                cast
+              }
+            }
+            "#
+        )
+        .run()
+        .await?
+    );
+    Ok(())
+}
+
+#[tokio::test]
+async fn filters_by_comparisons_on_elements_of_array_of_scalars_against_variable(
+) -> anyhow::Result<()> {
+    // Skip this test in MongoDB 5 because the example fails there. We're getting an error:
+    //
+    // > Kind: Command failed: Error code 5491300 (Location5491300): $documents' is not allowed in user requests, labels: {}
+    //
+    // This doesn't affect native queries that don't use the $documents stage.
+    if let Ok(image) = std::env::var("MONGODB_IMAGE") {
+        if image == "mongo:5" {
+            return Ok(());
+        }
+    }
+
+    assert_yaml_snapshot!(
+        run_connector_query(
+            Connector::SampleMflix,
+            query_request()
+                .variables([[("cast_member", "Albert Austin")]])
+                .collection("movies")
+                .query(
+                    query()
+                        .predicate(binop("_eq", target!("cast"), variable!(cast_member)))
+                        .fields([field!("title"), field!("cast")]),
+                )
+        )
+        .await?
+    );
+    Ok(())
+}

crates/integration-tests/src/tests/snapshots/integration_tests__tests__filtering__filters_by_comparisons_on_elements_of_array_of_scalars.snap

@@ -0,0 +1,13 @@
+---
+source: crates/integration-tests/src/tests/filtering.rs
+expression: "graphql_query(r#\"\n            query MyQuery {\n              movies(where: { cast: { _eq: \"Albert Austin\" } }) {\n                title\n                cast\n              }\n            }\n            \"#).run().await?"
+---
+data:
+  movies:
+    - title: The Immigrant
+      cast:
+        - Charles Chaplin
+        - Edna Purviance
+        - Eric Campbell
+        - Albert Austin
+errors: ~

crates/integration-tests/src/tests/snapshots/integration_tests__tests__filtering__filters_by_comparisons_on_elements_of_array_of_scalars_against_variable.snap

@@ -0,0 +1,11 @@
+---
+source: crates/integration-tests/src/tests/filtering.rs
+expression: "run_connector_query(Connector::SampleMflix,\n            query_request().variables([[(\"cast_member\",\n                                            \"Albert Austin\")]]).collection(\"movies\").query(query().predicate(binop(\"_eq\",\n                            target!(\"cast\"),\n                            variable!(cast_member))).fields([field!(\"title\"),\n                            field!(\"cast\")]))).await?"
+---
+- rows:
+    - cast:
+        - Charles Chaplin
+        - Edna Purviance
+        - Eric Campbell
+        - Albert Austin
+      title: The Immigrant

crates/mongodb-agent-common/src/mongo_query_plan/mod.rs

@@ -91,6 +91,9 @@ fn scalar_type_name(t: &Type) -> Option<&'static str> {
     match t {
         Type::Scalar(MongoScalarType::Bson(s)) => Some(s.graphql_name()),
         Type::Scalar(MongoScalarType::ExtendedJSON) => Some(EXTENDED_JSON_TYPE_NAME),
+        Type::ArrayOf(t) if matches!(**t, Type::Scalar(_) | Type::Nullable(_)) => {
+            scalar_type_name(t)
+        }
         Type::Nullable(t) => scalar_type_name(t),
         _ => None,
     }

crates/mongodb-agent-common/src/query/make_selector.rs

@@ -147,11 +147,28 @@ pub fn make_selector(expr: &Expression) -> Result<Document> {
                     ColumnRef::MatchKey(key) => doc! {
                         key: { "$eq": null }
                     },
-                    ColumnRef::Expression(expr) => doc! {
-                        "$expr": {
-                            "$eq": [expr, null]
+                    ColumnRef::Expression(expr) => {
+                        // Special case for array-to-scalar comparisons - this is required because implicit
+                        // existential quantification over arrays for scalar comparisons does not work in
+                        // aggregation expressions.
+                        if column.get_field_type().is_array() {
+                            doc! {
+                                "$expr": {
+                                    "$reduce": {
+                                        "input": expr,
+                                        "initialValue": false,
+                                        "in": { "$eq": ["$$this", null] }
+                                    },
+                                },
+                            }
+                        } else {
+                            doc! {
+                                "$expr": {
+                                    "$eq": [expr, null]
+                                }
+                            }
                         }
-                    },
+                    }
                 };
                 Ok(traverse_relationship_path(
                     column.relationship_path(),
@@ -189,9 +206,26 @@ fn make_binary_comparison_selector(
             let comparison_value = bson_from_scalar_value(value, value_type)?;
             let match_doc = match ColumnRef::from_comparison_target(target_column) {
                 ColumnRef::MatchKey(key) => operator.mongodb_match_query(key, comparison_value),
-                ColumnRef::Expression(expr) => doc! {
-                    "$expr": operator.mongodb_aggregation_expression(expr, comparison_value)
-                },
+                ColumnRef::Expression(expr) => {
+                    // Special case for array-to-scalar comparisons - this is required because implicit
+                    // existential quantification over arrays for scalar comparisons does not work in
+                    // aggregation expressions.
+                    if target_column.get_field_type().is_array() && !value_type.is_array() {
+                        doc! {
+                            "$expr": {
+                                "$reduce": {
+                                    "input": expr,
+                                    "initialValue": false,
+                                    "in": operator.mongodb_aggregation_expression("$$this", comparison_value)
+                                },
+                            },
+                        }
+                    } else {
+                        doc! {
+                            "$expr": operator.mongodb_aggregation_expression(expr, comparison_value)
+                        }
+                    }
+                }
             };
             traverse_relationship_path(target_column.relationship_path(), match_doc)
         }
@@ -200,12 +234,28 @@ fn make_binary_comparison_selector(
             variable_type,
         } => {
             let comparison_value = variable_to_mongo_expression(name, variable_type);
-            let match_doc = doc! {
-                "$expr": operator.mongodb_aggregation_expression(
-                    column_expression(target_column),
-                    comparison_value
-                )
-            };
+            let match_doc =
+                // Special case for array-to-scalar comparisons - this is required because implicit
+                // existential quantification over arrays for scalar comparisons does not work in
+                // aggregation expressions.
+                if target_column.get_field_type().is_array() && !variable_type.is_array() {
+                    doc! {
+                        "$expr": {
+                            "$reduce": {
+                                "input": column_expression(target_column),
+                                "initialValue": false,
+                                "in": operator.mongodb_aggregation_expression("$$this", comparison_value)
+                            },
+                        },
+                    }
+                } else {
+                    doc! {
+                        "$expr": operator.mongodb_aggregation_expression(
+                            column_expression(target_column),
+                            comparison_value
+                        )
+                    }
+                };
             traverse_relationship_path(target_column.relationship_path(), match_doc)
         }
     };

crates/ndc-query-plan/src/plan_for_query_request/helpers.rs

@@ -2,7 +2,7 @@ use std::collections::BTreeMap;
 
 use ndc_models as ndc;
 
-use crate as plan;
+use crate::{self as plan};
 
 use super::query_plan_error::QueryPlanError;
 
@@ -107,3 +107,45 @@ pub fn lookup_relationship<'a>(
         .get(relationship)
         .ok_or_else(|| QueryPlanError::UnspecifiedRelation(relationship.to_owned()))
 }
+
+/// Special case handling for array comparisons! Normally we assume that the right operand of Equal
+/// is the same type as the left operand. BUT MongoDB allows comparing arrays to scalar values in
+/// which case the condition passes if any array element is equal to the given scalar value. So
+/// this function needs to return a scalar type if the user is expecting array-to-scalar
+/// comparison, or an array type if the user is expecting array-to-array comparison. Or if the
+/// column does not have an array type we fall back to the default assumption that the value type
+/// should be the same as the column type.
+///
+/// For now this assumes that if the column has an array type, the value type is a scalar type.
+/// That's the simplest option since we don't support array-to-array comparisons yet.
+///
+/// TODO: When we do support array-to-array comparisons we will need to either:
+///
+/// - input the [ndc::ComparisonValue] into this function, and any query request variables; check
+///   that the given JSON value or variable values are not array values, and if so assume the value
+///   type should be a scalar type
+/// - or get the GraphQL Engine to include a type with [ndc::ComparisonValue] in which case we can
+///   use that as the value type
+///
+/// It is important that queries behave the same when given an inline value or variables. So we
+/// can't just check the value of an [ndc::ComparisonValue::Scalar], and punt on an
+/// [ndc::ComparisonValue::Variable] input. The latter requires accessing query request variables,
+/// and it will take a little more work to thread those through the code to make them available
+/// here.
+pub fn value_type_in_possible_array_equality_comparison<S>(
+    column_type: plan::Type<S>,
+) -> plan::Type<S>
+where
+    S: Clone,
+{
+    match column_type {
+        plan::Type::ArrayOf(t) => *t,
+        plan::Type::Nullable(t) => match *t {
+            v @ plan::Type::ArrayOf(_) => {
+                value_type_in_possible_array_equality_comparison(v.clone())
+            }
+            t => plan::Type::Nullable(Box::new(t)),
+        },
+        _ => column_type,
+    }
+}

crates/ndc-query-plan/src/plan_for_query_request/mod.rs

@@ -15,7 +15,7 @@ mod tests;
 use std::{collections::VecDeque, iter::once};
 
 use crate::{self as plan, type_annotated_field, ObjectType, QueryPlan, Scope};
-use helpers::find_nested_collection_type;
+use helpers::{find_nested_collection_type, value_type_in_possible_array_equality_comparison};
 use indexmap::IndexMap;
 use itertools::Itertools;
 use ndc::{ExistsInCollection, QueryRequest};
@@ -516,7 +516,10 @@ fn plan_for_binary_comparison<T: QueryContext>(
         .context
         .find_comparison_operator(comparison_target.get_field_type(), &operator)?;
     let value_type = match operator_definition {
-        plan::ComparisonOperatorDefinition::Equal => comparison_target.get_field_type().clone(),
+        plan::ComparisonOperatorDefinition::Equal => {
+            let column_type = comparison_target.get_field_type().clone();
+            value_type_in_possible_array_equality_comparison(column_type)
+        }
         plan::ComparisonOperatorDefinition::In => {
             plan::Type::ArrayOf(Box::new(comparison_target.get_field_type().clone()))
         }

crates/ndc-query-plan/src/type_system.rs

@@ -24,6 +24,14 @@ impl<S> Type<S> {
             t => Type::Nullable(Box::new(t)),
         }
     }
+
+    pub fn is_array(&self) -> bool {
+        match self {
+            Type::ArrayOf(_) => true,
+            Type::Nullable(t) => t.is_array(),
+            _ => false,
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]

fixtures/hasura/sample_mflix/metadata/models/Movies.hml

@@ -574,8 +574,12 @@ definition:
           booleanExpressionType: ObjectIdComparisonExp
         - fieldName: awards
           booleanExpressionType: MoviesAwardsComparisonExp
+        - fieldName: cast
+          booleanExpressionType: StringComparisonExp
         - fieldName: fullplot
           booleanExpressionType: StringComparisonExp
+        - fieldName: genres
+          booleanExpressionType: StringComparisonExp
         - fieldName: imdb
           booleanExpressionType: MoviesImdbComparisonExp
         - fieldName: lastupdated