Revert "graph, graphql: Optimize away unneeded child queries"

This reverts commit 2d6c531.

Author: lutter

graph/src/components/store/mod.rs

@@ -30,7 +30,7 @@ use crate::blockchain::Block;
 use crate::components::store::write::EntityModification;
 use crate::data::store::scalar::Bytes;
 use crate::data::store::*;
-use crate::data::value::{Object, Word};
+use crate::data::value::Word;
 use crate::data_source::CausalityRegion;
 use crate::schema::InputSchema;
 use crate::util::intern;
@@ -483,84 +483,6 @@ pub enum EntityLink {
     Parent(EntityType, ParentLink),
 }
 
-impl EntityLink {
-    /// Return a list of objects that have only the `id`, parent id, and
-    /// typename set using the child ids from `self` when `self` is
-    /// `Parent`. If `self` is `Direct`, return `None`
-    ///
-    /// The list that is returned is sorted and truncated to `first` many
-    /// entries.
-    ///
-    /// This makes it possible to avoid running a query when all that is
-    /// needed is the `id` of the children
-    pub fn to_basic_objects(self, parents: &Vec<String>, first: usize) -> Option<Vec<Object>> {
-        use crate::data::value::Value as V;
-
-        fn basic_object(entity_type: &EntityType, parent: &str, child: String) -> Object {
-            let mut obj = Vec::new();
-            obj.push((ID.clone(), V::String(child)));
-            obj.push((Word::from("__typename"), V::String(entity_type.to_string())));
-            obj.push((PARENT_ID.clone(), V::String(parent.to_string())));
-            Object::from_iter(obj)
-        }
-
-        fn basic_objects(
-            entity_type: &EntityType,
-            parent: &str,
-            children: Vec<String>,
-        ) -> Vec<Object> {
-            children
-                .into_iter()
-                .map(|child| basic_object(entity_type, parent, child))
-                .collect()
-        }
-
-        fn obj_key<'a>(obj: &'a Object) -> Option<(&'a str, &'a str)> {
-            match (obj.get(&*PARENT_ID), obj.get(ID.as_str())) {
-                (Some(V::String(p)), Some(V::String(id))) => Some((p, id)),
-                _ => None,
-            }
-        }
-
-        fn obj_cmp(a: &Object, b: &Object) -> std::cmp::Ordering {
-            obj_key(a).cmp(&obj_key(b))
-        }
-
-        match self {
-            EntityLink::Direct(_, _) => return None,
-            EntityLink::Parent(entity_type, link) => {
-                let mut objects = Vec::new();
-                match link {
-                    ParentLink::List(ids) => {
-                        for (parent, children) in parents.iter().zip(ids) {
-                            objects.extend(basic_objects(&entity_type, parent, children));
-                        }
-                    }
-                    ParentLink::Scalar(ids) => {
-                        for (parent, child) in parents.iter().zip(ids) {
-                            if let Some(child) = child {
-                                objects.push(basic_object(&entity_type, parent, child));
-                            }
-                        }
-                    }
-                }
-                // Sort the objects by parent id and child id just as
-                // running a query would
-                objects.sort_by(obj_cmp);
-                objects.truncate(first);
-                Some(objects)
-            }
-        }
-    }
-
-    pub fn has_child_ids(&self) -> bool {
-        match self {
-            EntityLink::Direct(_, _) => false,
-            EntityLink::Parent(_, _) => true,
-        }
-    }
-}
-
 /// Window results of an `EntityQuery` query along the parent's id:
 /// the `order_by`, `order_direction`, and `range` of the query apply to
 /// entities that belong to the same parent. Only entities that belong to

graph/src/data/query/error.rs

@@ -75,8 +75,6 @@ pub enum QueryExecutionError {
     InvalidSubgraphManifest,
     ResultTooBig(usize, usize),
     DeploymentNotFound(String),
-    IdMissing,
-    IdNotString,
 }
 
 impl QueryExecutionError {
@@ -134,9 +132,7 @@ impl QueryExecutionError {
             | InvalidSubgraphManifest
             | ValidationError(_, _)
             | ResultTooBig(_, _)
-            | DeploymentNotFound(_)
-            | IdMissing
-            | IdNotString => false,
+            | DeploymentNotFound(_) => false,
         }
     }
 }
@@ -283,9 +279,7 @@ impl fmt::Display for QueryExecutionError {
             SubgraphManifestResolveError(e) => write!(f, "failed to resolve subgraph manifest: {}", e),
             InvalidSubgraphManifest => write!(f, "invalid subgraph manifest file"),
             ResultTooBig(actual, limit) => write!(f, "the result size of {} is larger than the allowed limit of {}", actual, limit),
-            DeploymentNotFound(id_or_name) => write!(f, "deployment `{}` does not exist", id_or_name),
-            IdMissing => write!(f, "Entity is missing an `id` attribute"),
-            IdNotString => write!(f, "Entity is missing an `id` attribute")
+            DeploymentNotFound(id_or_name) => write!(f, "deployment `{}` does not exist", id_or_name)
         }
     }
 }

graphql/src/store/prefetch.rs

@@ -1,11 +1,12 @@
 //! Run a GraphQL query and fetch all the entitied needed to build the
 //! final result
 
+use anyhow::{anyhow, Error};
 use graph::constraint_violation;
 use graph::data::query::Trace;
-use graph::data::store::{ID, PARENT_ID};
+use graph::data::store::PARENT_ID;
 use graph::data::value::{Object, Word};
-use graph::prelude::{r, CacheWeight, CheapClone, EntityQuery, EntityRange};
+use graph::prelude::{r, CacheWeight, CheapClone};
 use graph::slog::warn;
 use graph::util::cache_weight;
 use std::collections::BTreeMap;
@@ -170,11 +171,11 @@ impl ValueExt for r::Value {
 }
 
 impl Node {
-    fn id(&self) -> Result<String, QueryExecutionError> {
+    fn id(&self) -> Result<String, Error> {
         match self.get("id") {
-            None => Err(QueryExecutionError::IdMissing),
+            None => Err(anyhow!("Entity is missing an `id` attribute")),
             Some(r::Value::String(s)) => Ok(s.clone()),
-            _ => Err(QueryExecutionError::IdNotString),
+            _ => Err(anyhow!("Entity has non-string `id` attribute")),
         }
     }
 
@@ -657,30 +658,6 @@ fn execute_field(
     .map_err(|e| vec![e])
 }
 
-/// Check whether `field` only selects the `id` of its children and whether
-/// it is safe to skip running `query` if we have all child ids in memory
-/// already.
-fn selects_id_only(field: &a::Field, query: &EntityQuery) -> bool {
-    if query.filter.is_some() || query.range.skip != 0 {
-        return false;
-    }
-    match &query.order {
-        EntityOrder::Ascending(attr, _) => {
-            if attr != ID.as_str() {
-                return false;
-            }
-        }
-        _ => {
-            return false;
-        }
-    }
-    field
-        .selection_set
-        .single_field()
-        .map(|field| field.name.as_str() == ID.as_str())
-        .unwrap_or(false)
-}
-
 /// Query child entities for `parents` from the store. The `join` indicates
 /// in which child field to look for the parent's id/join field. When
 /// `is_single` is `true`, there is at most one child per parent.
@@ -727,23 +704,6 @@ fn fetch(
         if windows.is_empty() {
             return Ok((vec![], Trace::None));
         }
-        // See if we can short-circuit query execution and just reuse what
-        // we already have in memory. We could do this probably even with
-        // multiple windows, but this covers the most common case.
-        if windows.len() == 1 && windows[0].link.has_child_ids() && selects_id_only(field, &query) {
-            let mut windows = windows;
-            // unwrap: we checked that len is 1
-            let window = windows.pop().unwrap();
-            let parent_ids = parents
-                .iter()
-                .map(|parent| parent.id())
-                .collect::<Result<_, _>>()
-                .map_err(QueryExecutionError::from)?;
-            // unwrap: we checked in the if condition that the window has child ids
-            let first = query.range.first.unwrap_or(EntityRange::FIRST) as usize;
-            let objs = window.link.to_basic_objects(&parent_ids, first).unwrap();
-            return Ok((objs.into_iter().map(Node::from).collect(), Trace::None));
-        }
         query.collection = EntityCollection::Window(windows);
     }
     resolver