graphql: Treat missing variables as null values

Instead of immediately reporting an error, treat missing variables as nulls
and let the rest of the execution logic deal with nulls

Author: lutter

graphql/src/execution/query.rs

@@ -741,17 +741,17 @@ struct Transform {
 }
 
 impl Transform {
-    fn variable(
-        &self,
-        name: &str,
-        default: Option<r::Value>,
-        pos: &Pos,
-    ) -> Result<r::Value, QueryExecutionError> {
+    /// Look up the value of the variable `name`. If the variable is not
+    /// defined, return `r::Value::Null`
+    // graphql-bug-compat: use `default` as the value for an undefined
+    // variable. Once queries are fully validated, all variables will be
+    // defined
+    fn variable(&self, name: &str, default: Option<r::Value>) -> r::Value {
         self.variables
             .get(name)
             .map(|value| value.clone())
             .or(default)
-            .ok_or_else(|| QueryExecutionError::MissingVariableError(pos.clone(), name.to_string()))
+            .unwrap_or(r::Value::Null)
     }
 
     /// Interpolate variable references in the arguments `args`
@@ -760,47 +760,42 @@ impl Transform {
         args: Vec<(String, q::Value)>,
         defaults: &dyn DefaultLookup,
         pos: &Pos,
-    ) -> Result<Vec<(String, r::Value)>, QueryExecutionError> {
+    ) -> Vec<(String, r::Value)> {
         args.into_iter()
             .map(|(name, val)| {
                 let default = defaults.find(&name);
-                self.interpolate_value(val, default, pos)
-                    .map(|val| (name, val))
+                let val = self.interpolate_value(val, default, pos);
+                (name, val)
             })
             .collect()
     }
 
     /// Turn `value` into an `r::Value` by resolving variable references
-    fn interpolate_value(
-        &self,
-        value: q::Value,
-        default: Option<r::Value>,
-        pos: &Pos,
-    ) -> Result<r::Value, QueryExecutionError> {
+    fn interpolate_value(&self, value: q::Value, default: Option<r::Value>, pos: &Pos) -> r::Value {
         match value {
-            q::Value::Variable(var) => self.variable(&var, default, pos),
-            q::Value::Int(ref num) => Ok(r::Value::Int(
-                num.as_i64().expect("q::Value::Int contains an i64"),
-            )),
-            q::Value::Float(f) => Ok(r::Value::Float(f)),
-            q::Value::String(s) => Ok(r::Value::String(s)),
-            q::Value::Boolean(b) => Ok(r::Value::Boolean(b)),
-            q::Value::Null => Ok(r::Value::Null),
-            q::Value::Enum(s) => Ok(r::Value::Enum(s)),
+            q::Value::Variable(var) => self.variable(&var, default),
+            q::Value::Int(ref num) => {
+                r::Value::Int(num.as_i64().expect("q::Value::Int contains an i64"))
+            }
+            q::Value::Float(f) => r::Value::Float(f),
+            q::Value::String(s) => r::Value::String(s),
+            q::Value::Boolean(b) => r::Value::Boolean(b),
+            q::Value::Null => r::Value::Null,
+            q::Value::Enum(s) => r::Value::Enum(s),
             q::Value::List(vals) => {
-                let vals: Vec<_> = vals
+                let vals = vals
                     .into_iter()
                     .map(|val| self.interpolate_value(val, None, pos))
-                    .collect::<Result<Vec<_>, _>>()?;
-                Ok(r::Value::List(vals))
+                    .collect();
+                r::Value::List(vals)
             }
             q::Value::Object(map) => {
                 let mut rmap = BTreeMap::new();
                 for (key, value) in map.into_iter() {
-                    let value = self.interpolate_value(value, None, pos)?;
+                    let value = self.interpolate_value(value, None, pos);
                     rmap.insert(key, value);
                 }
-                Ok(r::Value::object(rmap))
+                r::Value::object(rmap)
             }
         }
     }
@@ -813,7 +808,7 @@ impl Transform {
         dirs: Vec<q::Directive>,
         defns: &Vec<s::Directive>,
     ) -> Result<(Vec<a::Directive>, bool), QueryExecutionError> {
-        let dirs = dirs
+        let dirs: Vec<_> = dirs
             .into_iter()
             .map(|dir| {
                 let q::Directive {
@@ -827,14 +822,14 @@ impl Transform {
                         .find(|defn| defn.name == name)
                         .map(|defn| &defn.arguments),
                 );
-                self.interpolate_arguments(arguments, &defaults, &position)
-                    .map(|arguments| a::Directive {
-                        name,
-                        position,
-                        arguments,
-                    })
+                let arguments = self.interpolate_arguments(arguments, &defaults, &position);
+                a::Directive {
+                    name,
+                    position,
+                    arguments,
+                }
             })
-            .collect::<Result<Vec<_>, _>>()?;
+            .collect();
         let skip = dirs.iter().any(|dir| dir.skip());
         Ok((dirs, skip))
     }
@@ -944,7 +939,7 @@ impl Transform {
         }
 
         let mut arguments =
-            self.interpolate_arguments(arguments, &FieldArgumentDefaults(&field_type), &position)?;
+            self.interpolate_arguments(arguments, &FieldArgumentDefaults(&field_type), &position);
         self.coerce_argument_values(&mut arguments, parent_type, &name)?;
 
         let is_leaf_type = self.schema.document().is_leaf_type(&field_type.field_type);

graphql/tests/query.rs

@@ -1489,7 +1489,7 @@ fn leaf_selection_mismatch() {
 
 // see: graphql-bug-compat
 #[test]
-fn missing_var_uses_default() {
+fn missing_variable() {
     run_test_sequentially(|store| async move {
         let deployment = setup(store.as_ref());
         let result = execute_query_document(
@@ -1512,6 +1512,17 @@ fn missing_var_uses_default() {
         };
         let data = extract_data!(result).unwrap();
         assert_eq!(exp, data);
+
+        let result = execute_query_document(
+            &deployment.hash,
+            // '$where' is not defined but nullable, ignore the argument
+            graphql_parser::parse_query("query { musicians(where: $where) { id } }")
+                .expect("invalid test query")
+                .into_static(),
+        )
+        .await;
+        let data = extract_data!(result).unwrap();
+        assert_eq!(exp, data);
     })
 }