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

fix: batched queries with same args in diff order #3398

Merged
merged 1 commit into from
Nov 15, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,25 @@ mod compound_batch {
Ok(())
}

#[connector_test]
async fn two_equal_queries(runner: Runner) -> TestResult<()> {
create_test_data(&runner).await?;

let queries = vec![
r#"query {findUniqueArtist(where:{firstName_lastName:{firstName:"Musti",lastName:"Naukio"} }) {firstName lastName}}"#.to_string(),
r#"query {findUniqueArtist(where:{firstName_lastName:{lastName:"Naukio",firstName:"Musti"} }) {firstName lastName}}"#.to_string(),
];

let batch_results = runner.batch(queries, false, None).await?;

insta::assert_snapshot!(
batch_results.to_string(),
@r###"{"batchResult":[{"data":{"findUniqueArtist":{"firstName":"Musti","lastName":"Naukio"}}},{"data":{"findUniqueArtist":{"firstName":"Musti","lastName":"Naukio"}}}]}"###
);

Ok(())
}

fn should_batch_schema() -> String {
let schema = indoc! {
r#"model Artist {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,20 @@ mod singular_batch {
create_test_data(&runner).await?;

let queries = vec![
r#"query { findUniqueArtist(where: { ArtistId: 1}) { Name }}"#.to_string(),
r#"query { findUniqueArtist(where: { ArtistId: 1}) { Name }}"#.to_string(),
r#"query { findUniqueArtist(where: { ArtistId: 1 }) { Name }}"#.to_string(),
r#"query { findUniqueArtist(where: { ArtistId: 1 }) { Name }}"#.to_string(),
];

let batch_results = runner.batch(queries, false, None).await?;
insta::assert_snapshot!(
batch_results.to_string(),
@r###"{"batchResult":[{"data":{"findUniqueArtist":{"Name":"ArtistWithoutAlbums"}}},{"data":{"findUniqueArtist":{"Name":"ArtistWithoutAlbums"}}}]}"###
);

// With non unique filters
let queries = vec![
r#"query { findUniqueArtist(where: { ArtistId: 1, Name: "ArtistWithoutAlbums" }) { Name }}"#.to_string(),
r#"query { findUniqueArtist(where: { Name: "ArtistWithoutAlbums", ArtistId: 1 }) { Name }}"#.to_string(),
];

let batch_results = runner.batch(queries, false, None).await?;
Expand Down
10 changes: 6 additions & 4 deletions query-engine/core/src/query_document/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use indexmap::IndexMap;
use prisma_models::ModelRef;
use schema::QuerySchemaRef;
use schema_builder::constants::*;
use std::collections::HashMap;

pub type QueryParserResult<T> = std::result::Result<T, QueryParserError>;

Expand Down Expand Up @@ -156,7 +157,7 @@ impl BatchDocumentTransaction {

#[derive(Debug, Clone)]
pub struct CompactedDocument {
pub arguments: Vec<Vec<(String, QueryValue)>>,
pub arguments: Vec<HashMap<String, QueryValue>>,
pub nested_selection: Vec<String>,
pub operation: Operation,
pub keys: Vec<String>,
Expand Down Expand Up @@ -242,14 +243,15 @@ impl CompactedDocument {
// Saving the stub of the query name for later use.
let name = selections[0].name().replacen("findUnique", "", 1);

// Convert the selections into a vector of arguments. This defines the
// Convert the selections into a map of arguments. This defines the
// response order and how we fetch the right data from the response set.
let arguments: Vec<Vec<(String, QueryValue)>> = selections
let arguments: Vec<HashMap<String, QueryValue>> = selections
.into_iter()
.map(|mut sel| {
let where_obj = sel.pop_argument().unwrap().1.into_object().unwrap();
let filter_map: HashMap<String, QueryValue> = extract_filter(where_obj, model).into_iter().collect();

extract_filter(where_obj, model)
filter_map
})
.collect();

Expand Down
13 changes: 6 additions & 7 deletions query-engine/core/src/response_ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::QueryValue;
use indexmap::IndexMap;
use prisma_models::PrismaValue;
use serde::ser::{Serialize, SerializeMap, SerializeSeq, Serializer};
use std::{fmt, sync::Arc};
use std::{collections::HashMap, fmt, sync::Arc};

pub use ir_serializer::*;
pub use response::*;
Expand Down Expand Up @@ -44,16 +44,15 @@ impl List {
self.len() == 0
}

pub fn index_by(self, keys: &[String]) -> Vec<(Vec<QueryValue>, Map)> {
let mut map = Vec::with_capacity(self.len());
pub fn index_by(self, keys: &[String]) -> Vec<(HashMap<String, QueryValue>, Map)> {
let mut map: Vec<(HashMap<String, QueryValue>, Map)> = Vec::with_capacity(self.len());

for item in self.into_iter() {
let inner = item.into_map().unwrap();

let key: Vec<QueryValue> = keys
let key: HashMap<String, QueryValue> = keys
.iter()
.map(|key| inner.get(key).unwrap().clone().into_value().unwrap())
.map(QueryValue::from)
.map(|key| (key.clone(), inner.get(key).unwrap().clone().into_value().unwrap()))
.map(|(key, val)| (key, QueryValue::from(val)))
.collect();

map.push((key, inner));
Expand Down
31 changes: 26 additions & 5 deletions query-engine/request-handlers/src/graphql/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use futures::FutureExt;
use indexmap::IndexMap;
use query_core::{
schema::QuerySchemaRef, BatchDocument, BatchDocumentTransaction, CompactedDocument, Item, Operation, QueryDocument,
QueryExecutor, QueryValue, ResponseData, TxId,
QueryExecutor, ResponseData, TxId,
};
use std::{fmt, panic::AssertUnwindSafe};

Expand Down Expand Up @@ -123,8 +123,29 @@ impl<'a> GraphQlHandler<'a> {
Ok(Ok(response_data)) => {
let mut gql_response: GQLResponse = response_data.into();

// We find the response data and make a hash from the given unique keys.
let data = gql_response
// At this point, many findUnique queries were converted to a single findMany query and that query was run.
// This means we have a list of results and we need to map each result back to their original findUnique query.
// `data` is the piece of logic that allows us to do that mapping.
// It takes the findMany response and converts it to a map of arguments to result.
// Let's take an example. Given the following batched queries:
// [
// findUnique(where: { id: 1, name: "Bob" }) { id name age },
// findUnique(where: { id: 2, name: "Alice" }) { id name age }
// ]
// 1. This gets converted to: findMany(where: { OR: [{ id: 1, name: "Bob" }, { id: 2, name: "Alice" }] }) { id name age }
// 2. Say we get the following result back: [{ id: 1, name: "Bob", age: 18 }, { id: 2, name: "Alice", age: 27 }]
// 3. We know the inputted arguments are ["id", "name"]
// 4. So we go over the result and build the following list:
// [
// ({ id: 1, name: "Bob" }, { id: 1, name: "Bob", age: 18 }),
// ({ id: 2, name: "Alice" }, { id: 2, name: "Alice", age: 27 })
// ]
// 5. Now, given the original findUnique queries and that list, we can easily find back which arguments maps to which result
// [
// findUnique(where: { id: 1, name: "Bob" }) { id name age } -> { id: 1, name: "Bob", age: 18 }
// findUnique(where: { id: 2, name: "Alice" }) { id name age } -> { id: 2, name: "Alice", age: 27 }
// ]
let args_to_results = gql_response
Comment on lines +126 to +148
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very much appreciated and explains a bit the reason behind prisma/prisma#16267

.take_data(plural_name)
.unwrap()
.into_list()
Expand All @@ -134,13 +155,13 @@ impl<'a> GraphQlHandler<'a> {
let results: Vec<GQLResponse> = arguments
.into_iter()
.map(|args| {
let vals: Vec<QueryValue> = args.into_iter().map(|(_, v)| v).collect();
let mut responses = GQLResponse::with_capacity(1);

// This is step 5 of the comment above.
// Copying here is mandatory due to some of the queries
// might be repeated with the same arguments in the original
// batch. We need to give the same answer for both of them.
match data.iter().find(|(k, _)| k == &vals) {
match args_to_results.iter().find(|(a, _)| *a == args) {
Some((_, result)) => {
// Filter out all the keys not selected in the
// original query.
Expand Down