Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variable deduplication in query planner #872

Merged
merged 17 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions apollo-router-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ http-body = "0.4.4"
hyper = { version = "0.14.18", features = ["client"] }
hyper-rustls = "0.23.0"
include_dir = "0.7.2"
indexmap = "1.8.1"
itertools = "0.10.3"
lazy_static = "1.4.0"
lru = "0.7.5"
Expand Down
4 changes: 2 additions & 2 deletions apollo-router-core/src/json_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ where
/// A GraphQL path element that is composes of strings or numbers.
/// e.g `/book/3/name`
#[doc(hidden)]
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, Hash)]
#[serde(untagged)]
pub enum PathElement {
/// A path element that given an array will flatmap the content.
Expand Down Expand Up @@ -424,7 +424,7 @@ where
///
/// This can be composed of strings and numbers
#[doc(hidden)]
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, Default)]
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, Default, Hash)]
#[serde(transparent)]
pub struct Path(pub Vec<PathElement>);

Expand Down
8 changes: 4 additions & 4 deletions apollo-router-core/src/plugins/include_subgraph_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ mod test {

let account_mocks = vec![
(
r#"{"query":"query TopProducts__accounts__3($representations:[_Any!]!){_entities(representations:$representations){...on User{name}}}","operationName":"TopProducts__accounts__3","variables":{"representations":[{"__typename":"User","id":"1"},{"__typename":"User","id":"2"},{"__typename":"User","id":"1"}]}}"#,
r#"{"data":{"_entities":[{"name":"Ada Lovelace"},{"name":"Alan Turing"},{"name":"Ada Lovelace"}]}}"#
r#"{"query":"query TopProducts__accounts__3($representations:[_Any!]!){_entities(representations:$representations){...on User{name}}}","operationName":"TopProducts__accounts__3","variables":{"representations":[{"__typename":"User","id":"1"},{"__typename":"User","id":"2"}]}}"#,
r#"{"data":{"_entities":[{"name":"Ada Lovelace"},{"name":"Alan Turing"}]}}"#
)
].into_iter().map(|(query, response)| (serde_json::from_str(query).unwrap(), serde_json::from_str(response).unwrap())).collect();
let account_service = MockSubgraph::new(account_mocks);
Expand All @@ -152,8 +152,8 @@ mod test {
r#"{"data":{"topProducts":[{"__typename":"Product","upc":"1","name":"Table"},{"__typename":"Product","upc":"2","name":"Couch"}]}}"#
),
(
r#"{"query":"query TopProducts__products__2($representations:[_Any!]!){_entities(representations:$representations){...on Product{name}}}","operationName":"TopProducts__products__2","variables":{"representations":[{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"2"}]}}"#,
r#"{"data":{"_entities":[{"name":"Table"},{"name":"Table"},{"name":"Couch"}]}}"#
r#"{"query":"query TopProducts__products__2($representations:[_Any!]!){_entities(representations:$representations){...on Product{name}}}","operationName":"TopProducts__products__2","variables":{"representations":[{"__typename":"Product","upc":"1"},{"__typename":"Product","upc":"2"}]}}"#,
r#"{"data":{"_entities":[{"name":"Table"},{"name":"Couch"}]}}"#
)
].into_iter().map(|(query, response)| (serde_json::from_str(query).unwrap(), serde_json::from_str(response).unwrap())).collect();

Expand Down
40 changes: 28 additions & 12 deletions apollo-router-core/src/query_planner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,9 @@ impl PlanNode {
pub(crate) mod fetch {
use super::selection::{select_object, Selection};
use crate::prelude::graphql::*;
use indexmap::IndexSet;
use serde::Deserialize;
use std::sync::Arc;
use std::{collections::HashMap, sync::Arc};
use tower::ServiceExt;
use tracing::{instrument, Instrument};

Expand Down Expand Up @@ -285,7 +286,7 @@ pub(crate) mod fetch {

struct Variables {
variables: Object,
paths: Vec<Path>,
paths: HashMap<Path, usize>,
}

impl Variables {
Expand All @@ -308,13 +309,20 @@ pub(crate) mod fetch {
.map(|(variable_key, value)| (variable_key.clone(), value.clone()))
}));

let mut paths = Vec::new();
let mut values = Vec::new();
let mut values: IndexSet<Value> = IndexSet::new();
let mut paths: HashMap<Path, usize> = HashMap::new();
data.select_values_and_paths(current_dir, |path, value| {
if let Value::Object(content) = value {
if let Ok(Some(value)) = select_object(content, requires, schema) {
paths.push(path);
values.push(value);
match values.get_index_of(&value) {
Some(index) => {
paths.insert(path, index);
}
None => {
paths.insert(path, values.len());
values.insert(value);
}
}
}
}
});
Expand All @@ -323,7 +331,7 @@ pub(crate) mod fetch {
return None;
}

variables.insert("representations", Value::Array(values));
variables.insert("representations", Value::Array(Vec::from_iter(values)));

Some(Variables { variables, paths })
} else {
Expand All @@ -336,7 +344,7 @@ pub(crate) mod fetch {
.map(|(variable_key, value)| (variable_key.clone(), value.clone()))
})
.collect::<Object>(),
paths: Vec::new(),
paths: HashMap::new(),
})
}
}
Expand Down Expand Up @@ -446,7 +454,7 @@ pub(crate) mod fetch {
fn response_at_path<'a>(
&'a self,
current_dir: &'a Path,
paths: Vec<Path>,
paths: HashMap<Path, usize>,
data: Value,
) -> Result<Value, FetchError> {
if !self.requires.is_empty() {
Expand All @@ -458,9 +466,17 @@ pub(crate) mod fetch {

if let Value::Array(array) = entities {
let mut value = Value::default();

for (entity, path) in array.into_iter().zip(paths.into_iter()) {
value.insert(&path, entity)?;
for (path, entity_idx) in paths {
value.insert(
&path,
array
.get(entity_idx)
.ok_or_else(|| FetchError::ExecutionInvalidContent {
reason: "Received invalid content for key `_entities`!"
.to_string(),
})?
.clone(),
)?;
}
return Ok(value);
} else {
Expand Down