graphql: Do not pass variables around during execution

Query preprocessing eliminated all variable references

Author: lutter

graph/src/data/graphql/mod.rs

@@ -31,4 +31,3 @@ pub use object_or_interface::ObjectOrInterface;
 pub mod object_macro;
 pub use crate::object;
 pub use object_macro::{object_value, IntoValue};
-pub mod visitor;

graphql/src/execution/execution.rs

@@ -136,7 +136,6 @@ impl Default for WeightedResult {
 
 struct HashableQuery<'a> {
     query_schema_id: &'a DeploymentHash,
-    query_variables: &'a HashMap<String, r::Value>,
     query_fragments: &'a HashMap<String, q::FragmentDefinition>,
     selection_set: &'a q::SelectionSet,
     block_ptr: &'a BlockPtr,
@@ -163,13 +162,6 @@ impl StableHash for HashableQuery<'_> {
         self.query_schema_id
             .stable_hash(sequence_number.next_child(), state);
 
-        // Not stable! Uses to_string()
-        self.query_variables
-            .iter()
-            .map(|(k, v)| (k, v.to_string()))
-            .collect::<HashMap<_, _>>()
-            .stable_hash(sequence_number.next_child(), state);
-
         // Not stable! Uses to_string()
         self.query_fragments
             .iter()
@@ -197,7 +189,6 @@ fn cache_key(
     // Otherwise, incorrect results may be returned.
     let query = HashableQuery {
         query_schema_id: ctx.query.schema.id(),
-        query_variables: &ctx.query.variables,
         query_fragments: &ctx.query.fragments,
         selection_set,
         block_ptr,
@@ -608,8 +599,8 @@ pub fn collect_fields_inner<'a>(
         let selections = selection_set
             .items
             .iter()
-            .filter(|selection| !qast::skip_selection(selection, &ctx.query.variables))
-            .filter(|selection| qast::include_selection(selection, &ctx.query.variables));
+            .filter(|selection| !qast::skip_selection(selection))
+            .filter(|selection| qast::include_selection(selection));
 
         for selection in selections {
             match selection {
@@ -1058,7 +1049,7 @@ pub fn coerce_argument_values<'a>(
         .flatten()
     {
         let value = qast::get_argument_value(&field.arguments, &argument_def.name).cloned();
-        match coercion::coerce_input_value(value, &argument_def, &resolver, &query.variables) {
+        match coercion::coerce_input_value(value, &argument_def, &resolver) {
             Ok(Some(value)) => {
                 if argument_def.name == "text".to_string() {
                     coerced_values.insert(

graphql/src/execution/query.rs

@@ -16,7 +16,6 @@ use graph::prelude::{
 };
 
 use crate::introspection::introspection_schema;
-use crate::query::ext::ValueExt;
 use crate::query::{ast as qast, ext::BlockConstraint};
 use crate::schema::ast as sast;
 use crate::{
@@ -79,9 +78,7 @@ impl<'a> std::fmt::Display for SelectedFields<'a> {
 pub struct Query {
     /// The schema against which to execute the query
     pub schema: Arc<ApiSchema>,
-    /// The variables for the query, coerced into proper values
-    pub variables: HashMap<String, r::Value>,
-    /// The root selection set of the query
+    /// The root selection set of the query. All variable references have already been resolved
     pub selection_set: Arc<q::SelectionSet>,
     /// The ShapeHash of the original query
     pub shape_hash: u64,
@@ -179,7 +176,6 @@ impl Query {
 
         let query = Self {
             schema: raw_query.schema,
-            variables: raw_query.variables,
             fragments: raw_query.fragments,
             selection_set: Arc::new(raw_query.selection_set),
             shape_hash: query.shape_hash,
@@ -272,7 +268,6 @@ impl Query {
 
         Arc::new(Self {
             schema: Arc::new(introspection_schema),
-            variables: self.variables.clone(),
             fragments: self.fragments.clone(),
             selection_set: self.selection_set.clone(),
             shape_hash: self.shape_hash,
@@ -417,7 +412,7 @@ fn coerce_variable(
 
     let resolver = |name: &str| schema.document().get_named_type(name);
 
-    coerce_value(value, &variable_def.var_type, &resolver, &HashMap::new()).map_err(|value| {
+    coerce_value(value, &variable_def.var_type, &resolver).map_err(|value| {
         vec![QueryExecutionError::InvalidArgumentError(
             variable_def.position,
             variable_def.name.to_owned(),

graphql/src/query/ast.rs

@@ -1,5 +1,4 @@
-use graph::prelude::{q::*, r};
-use std::collections::HashMap;
+use graph::prelude::q::*;
 use std::ops::Deref;
 
 use graph::prelude::QueryExecutionError;
@@ -64,18 +63,14 @@ pub fn get_argument_value<'a>(arguments: &'a [(String, Value)], name: &str) -> O
 }
 
 /// Returns true if a selection should be skipped (as per the `@skip` directive).
-pub fn skip_selection(selection: &Selection, variables: &HashMap<String, r::Value>) -> bool {
+pub fn skip_selection(selection: &Selection) -> bool {
     match get_directive(selection, "skip".to_string()) {
         Some(directive) => match get_argument_value(&directive.arguments, "if") {
             Some(val) => match val {
                 // Skip if @skip(if: true)
                 Value::Boolean(skip_if) => *skip_if,
 
-                // Also skip if @skip(if: $variable) where $variable is true
-                Value::Variable(name) => variables.get(name).map_or(false, |var| match var {
-                    r::Value::Boolean(v) => v.to_owned(),
-                    _ => false,
-                }),
+                Value::Variable(_) => unreachable!("variables have already been resolved"),
 
                 _ => false,
             },
@@ -86,18 +81,14 @@ pub fn skip_selection(selection: &Selection, variables: &HashMap<String, r::Valu
 }
 
 /// Returns true if a selection should be included (as per the `@include` directive).
-pub fn include_selection(selection: &Selection, variables: &HashMap<String, r::Value>) -> bool {
+pub fn include_selection(selection: &Selection) -> bool {
     match get_directive(selection, "include".to_string()) {
         Some(directive) => match get_argument_value(&directive.arguments, "if") {
             Some(val) => match val {
                 // Include if @include(if: true)
                 Value::Boolean(include) => *include,
 
-                // Also include if @include(if: $variable) where $variable is true
-                Value::Variable(name) => variables.get(name).map_or(false, |var| match var {
-                    r::Value::Boolean(v) => v.to_owned(),
-                    _ => false,
-                }),
+                Value::Variable(_) => unreachable!("variables have already been resolved"),
 
                 _ => false,
             },

graphql/src/store/prefetch.rs

@@ -895,8 +895,8 @@ fn collect_fields_inner<'a>(
     let selections = selection_set
         .items
         .iter()
-        .filter(|selection| !qast::skip_selection(selection, &ctx.query.variables))
-        .filter(|selection| qast::include_selection(selection, &ctx.query.variables));
+        .filter(|selection| !qast::skip_selection(selection))
+        .filter(|selection| qast::include_selection(selection));
 
     for selection in selections {
         match selection {