From a118172eb884f963fe805498ba4e2e2a1beeb2b9 Mon Sep 17 00:00:00 2001 From: Sebastian Lorenz Date: Tue, 15 Jul 2025 17:45:09 +0200 Subject: [PATCH 1/4] graphql: Improve error message for OR operator with column filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the generic "Filter must by an object" error with a specific, helpful error message when users try to mix column filters with the 'or' operator at the same level. The new error message: - Clearly explains the problem - Shows which specific column filters are conflicting - Provides concrete examples of the problematic structure - Shows how to fix it by restructuring the query This addresses GitHub issues #6040 and #6041 by making the error message more descriptive and actionable for developers. Changes: - Add InvalidOrFilterStructure error variant to QueryExecutionError - Add validation in build_filter_from_object to detect mixed filters - Include comprehensive test coverage for various scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- graph/src/data/query/error.rs | 6 + graphql/src/store/query.rs | 252 ++++++++++++++++++++++++++++++++++ 2 files changed, 258 insertions(+) diff --git a/graph/src/data/query/error.rs b/graph/src/data/query/error.rs index d02b1c9c4bd..cfd900ac596 100644 --- a/graph/src/data/query/error.rs +++ b/graph/src/data/query/error.rs @@ -41,6 +41,7 @@ pub enum QueryExecutionError { FilterNotSupportedError(String, String), UnknownField(Pos, String, String), EmptyQuery, + InvalidOrFilterStructure(Vec, String), SubgraphDeploymentIdError(String), RangeArgumentsError(&'static str, u32, i64), InvalidFilterError, @@ -97,6 +98,7 @@ impl QueryExecutionError { | ChildFilterNestingNotSupportedError(_, _) | UnknownField(_, _, _) | EmptyQuery + | InvalidOrFilterStructure(_, _) | SubgraphDeploymentIdError(_) | InvalidFilterError | EntityFieldError(_, _) @@ -210,6 +212,10 @@ impl fmt::Display for QueryExecutionError { write!(f, "The `{}` argument must be between 0 and {}, but is {}", arg, max, actual) } InvalidFilterError => write!(f, "Filter must by an object"), + InvalidOrFilterStructure(fields, example) => { + write!(f, "Cannot mix column filters with 'or' operator at the same level. Found column filter(s) {} alongside 'or' operator.\n\n{}", + fields.join(", "), example) + } EntityFieldError(e, a) => { write!(f, "Entity `{}` has no attribute `{}`", e, a) } diff --git a/graphql/src/store/query.rs b/graphql/src/store/query.rs index 9c16361a2cf..7ff20c414a4 100644 --- a/graphql/src/store/query.rs +++ b/graphql/src/store/query.rs @@ -240,6 +240,34 @@ fn build_filter_from_object<'a>( object: &Object, schema: &InputSchema, ) -> Result, QueryExecutionError> { + // Check if we have both column filters and 'or' operator at the same level + let has_or = object.get("or").is_some(); + if has_or { + let column_filters: Vec = object + .iter() + .filter_map(|(key, _)| { + if key != "or" && key != "and" && key != "_change_block" { + Some(format!("'{}'", key)) + } else { + None + } + }) + .collect(); + + if !column_filters.is_empty() { + let example = format!( + "Instead of:\nwhere: {{ {}, or: [...] }}\n\nUse:\nwhere: {{ or: [{{ {}, ... }}, {{ {}, ... }}] }}", + column_filters.join(", "), + column_filters.join(", "), + column_filters.join(", ") + ); + return Err(QueryExecutionError::InvalidOrFilterStructure( + column_filters, + example, + )); + } + } + object .iter() .map(|(key, value)| { @@ -957,4 +985,228 @@ mod tests { Some(EntityFilter::And(vec![EntityFilter::ChangeBlockGte(10)])) ) } + + #[test] + fn build_query_detects_invalid_or_filter_structure() { + // Test that mixing column filters with 'or' operator produces a helpful error + let query_field = default_field_with( + "where", + r::Value::Object(Object::from_iter(vec![ + ("name".into(), r::Value::String("John".to_string())), + ("or".into(), r::Value::List(vec![ + r::Value::Object(Object::from_iter(vec![( + "email".into(), + r::Value::String("john@example.com".to_string()), + )])), + ])), + ])), + ); + + // We only allow one entity type in these tests + assert_eq!(query_field.selection_set.fields().count(), 1); + let obj_type = query_field + .selection_set + .fields() + .map(|(obj, _)| &obj.name) + .next() + .expect("there is one object type"); + let Some(object) = INPUT_SCHEMA.object_or_interface(obj_type, None) else { + panic!("object type {} not found", obj_type); + }; + + let result = build_query( + &object, + BLOCK_NUMBER_MAX, + &query_field, + std::u32::MAX, + std::u32::MAX, + &*INPUT_SCHEMA, + ); + + assert!(result.is_err()); + let error = result.unwrap_err(); + + // Check that we get the specific error we expect + match error { + graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => { + assert_eq!(fields, vec!["'name'"]); + assert!(example.contains("Instead of:")); + assert!(example.contains("where: { 'name', or: [...] }")); + assert!(example.contains("Use:")); + assert!(example.contains("where: { or: [{ 'name', ... }, { 'name', ... }] }")); + } + _ => panic!("Expected InvalidOrFilterStructure error, got: {}", error), + } + } + + #[test] + fn build_query_detects_invalid_or_filter_structure_multiple_fields() { + // Test that multiple column filters with 'or' operator are all reported + let query_field = default_field_with( + "where", + r::Value::Object(Object::from_iter(vec![ + ("name".into(), r::Value::String("John".to_string())), + ("email".into(), r::Value::String("john@example.com".to_string())), + ("or".into(), r::Value::List(vec![ + r::Value::Object(Object::from_iter(vec![( + "name".into(), + r::Value::String("Jane".to_string()), + )])), + ])), + ])), + ); + + // We only allow one entity type in these tests + assert_eq!(query_field.selection_set.fields().count(), 1); + let obj_type = query_field + .selection_set + .fields() + .map(|(obj, _)| &obj.name) + .next() + .expect("there is one object type"); + let Some(object) = INPUT_SCHEMA.object_or_interface(obj_type, None) else { + panic!("object type {} not found", obj_type); + }; + + let result = build_query( + &object, + BLOCK_NUMBER_MAX, + &query_field, + std::u32::MAX, + std::u32::MAX, + &*INPUT_SCHEMA, + ); + + assert!(result.is_err()); + let error = result.unwrap_err(); + + // Check that we get the specific error we expect + match error { + graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => { + // Should detect both column filters + assert_eq!(fields.len(), 2); + assert!(fields.contains(&"'name'".to_string())); + assert!(fields.contains(&"'email'".to_string())); + assert!(example.contains("Instead of:")); + assert!(example.contains("Use:")); + } + _ => panic!("Expected InvalidOrFilterStructure error, got: {}", error), + } + } + + #[test] + fn build_query_allows_valid_or_filter_structure() { + // Test that valid 'or' filters without column filters at the same level work correctly + let query_field = default_field_with( + "where", + r::Value::Object(Object::from_iter(vec![ + ("or".into(), r::Value::List(vec![ + r::Value::Object(Object::from_iter(vec![( + "name".into(), + r::Value::String("John".to_string()), + )])), + r::Value::Object(Object::from_iter(vec![( + "email".into(), + r::Value::String("john@example.com".to_string()), + )])), + ])), + ])), + ); + + // This should not produce an error + let result = query(&query_field); + assert!(result.filter.is_some()); + + // Verify that the filter is correctly structured + match result.filter.unwrap() { + EntityFilter::And(filters) => { + assert_eq!(filters.len(), 1); + match &filters[0] { + EntityFilter::Or(_) => { + // This is expected - OR filter should be wrapped in AND + } + _ => panic!("Expected OR filter, got: {:?}", filters[0]), + } + } + _ => panic!("Expected AND filter with OR inside"), + } + } + + #[test] + fn build_query_detects_invalid_or_filter_structure_with_operators() { + // Test that column filters with operators (like name_gt) are also detected + let query_field = default_field_with( + "where", + r::Value::Object(Object::from_iter(vec![ + ("name_gt".into(), r::Value::String("A".to_string())), + ("or".into(), r::Value::List(vec![ + r::Value::Object(Object::from_iter(vec![( + "email".into(), + r::Value::String("test@example.com".to_string()), + )])), + ])), + ])), + ); + + // We only allow one entity type in these tests + assert_eq!(query_field.selection_set.fields().count(), 1); + let obj_type = query_field + .selection_set + .fields() + .map(|(obj, _)| &obj.name) + .next() + .expect("there is one object type"); + let Some(object) = INPUT_SCHEMA.object_or_interface(obj_type, None) else { + panic!("object type {} not found", obj_type); + }; + + let result = build_query( + &object, + BLOCK_NUMBER_MAX, + &query_field, + std::u32::MAX, + std::u32::MAX, + &*INPUT_SCHEMA, + ); + + assert!(result.is_err()); + let error = result.unwrap_err(); + + // Check that we get the specific error we expect + match error { + graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => { + assert_eq!(fields, vec!["'name_gt'"]); + assert!(example.contains("Instead of:")); + assert!(example.contains("where: { 'name_gt', or: [...] }")); + assert!(example.contains("Use:")); + assert!(example.contains("where: { or: [{ 'name_gt', ... }, { 'name_gt', ... }] }")); + } + _ => panic!("Expected InvalidOrFilterStructure error, got: {}", error), + } + } + + #[test] + fn test_error_message_formatting() { + // Test that the error message is properly formatted + let fields = vec!["'age_gt'".to_string(), "'name'".to_string()]; + let example = format!( + "Instead of:\nwhere: {{ {}, or: [...] }}\n\nUse:\nwhere: {{ or: [{{ {}, ... }}, {{ {}, ... }}] }}", + fields.join(", "), + fields.join(", "), + fields.join(", ") + ); + + let error = graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example); + let error_msg = format!("{}", error); + + println!("Error message:\n{}", error_msg); + + // Verify the error message contains the key elements + assert!(error_msg.contains("Cannot mix column filters with 'or' operator")); + assert!(error_msg.contains("'age_gt', 'name'")); + assert!(error_msg.contains("Instead of:")); + assert!(error_msg.contains("Use:")); + assert!(error_msg.contains("where: { 'age_gt', 'name', or: [...] }")); + assert!(error_msg.contains("where: { or: [{ 'age_gt', 'name', ... }, { 'age_gt', 'name', ... }] }")); + } } From 65323e96cc04311eb914ebdb655de10e95abbab1 Mon Sep 17 00:00:00 2001 From: Sebastian Lorenz Date: Tue, 15 Jul 2025 18:03:01 +0200 Subject: [PATCH 2/4] Fix rustfmt formatting in query.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- graphql/src/store/query.rs | 63 ++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/graphql/src/store/query.rs b/graphql/src/store/query.rs index 7ff20c414a4..b87ac6f25d6 100644 --- a/graphql/src/store/query.rs +++ b/graphql/src/store/query.rs @@ -253,7 +253,7 @@ fn build_filter_from_object<'a>( } }) .collect(); - + if !column_filters.is_empty() { let example = format!( "Instead of:\nwhere: {{ {}, or: [...] }}\n\nUse:\nwhere: {{ or: [{{ {}, ... }}, {{ {}, ... }}] }}", @@ -993,12 +993,13 @@ mod tests { "where", r::Value::Object(Object::from_iter(vec![ ("name".into(), r::Value::String("John".to_string())), - ("or".into(), r::Value::List(vec![ - r::Value::Object(Object::from_iter(vec![( + ( + "or".into(), + r::Value::List(vec![r::Value::Object(Object::from_iter(vec![( "email".into(), r::Value::String("john@example.com".to_string()), - )])), - ])), + )]))]), + ), ])), ); @@ -1025,7 +1026,7 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - + // Check that we get the specific error we expect match error { graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => { @@ -1046,13 +1047,17 @@ mod tests { "where", r::Value::Object(Object::from_iter(vec![ ("name".into(), r::Value::String("John".to_string())), - ("email".into(), r::Value::String("john@example.com".to_string())), - ("or".into(), r::Value::List(vec![ - r::Value::Object(Object::from_iter(vec![( + ( + "email".into(), + r::Value::String("john@example.com".to_string()), + ), + ( + "or".into(), + r::Value::List(vec![r::Value::Object(Object::from_iter(vec![( "name".into(), r::Value::String("Jane".to_string()), - )])), - ])), + )]))]), + ), ])), ); @@ -1079,7 +1084,7 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - + // Check that we get the specific error we expect match error { graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => { @@ -1099,8 +1104,9 @@ mod tests { // Test that valid 'or' filters without column filters at the same level work correctly let query_field = default_field_with( "where", - r::Value::Object(Object::from_iter(vec![ - ("or".into(), r::Value::List(vec![ + r::Value::Object(Object::from_iter(vec![( + "or".into(), + r::Value::List(vec![ r::Value::Object(Object::from_iter(vec![( "name".into(), r::Value::String("John".to_string()), @@ -1109,14 +1115,14 @@ mod tests { "email".into(), r::Value::String("john@example.com".to_string()), )])), - ])), - ])), + ]), + )])), ); // This should not produce an error let result = query(&query_field); assert!(result.filter.is_some()); - + // Verify that the filter is correctly structured match result.filter.unwrap() { EntityFilter::And(filters) => { @@ -1139,12 +1145,13 @@ mod tests { "where", r::Value::Object(Object::from_iter(vec![ ("name_gt".into(), r::Value::String("A".to_string())), - ("or".into(), r::Value::List(vec![ - r::Value::Object(Object::from_iter(vec![( + ( + "or".into(), + r::Value::List(vec![r::Value::Object(Object::from_iter(vec![( "email".into(), r::Value::String("test@example.com".to_string()), - )])), - ])), + )]))]), + ), ])), ); @@ -1171,7 +1178,7 @@ mod tests { assert!(result.is_err()); let error = result.unwrap_err(); - + // Check that we get the specific error we expect match error { graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => { @@ -1195,18 +1202,20 @@ mod tests { fields.join(", "), fields.join(", ") ); - - let error = graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example); + + let error = + graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example); let error_msg = format!("{}", error); - + println!("Error message:\n{}", error_msg); - + // Verify the error message contains the key elements assert!(error_msg.contains("Cannot mix column filters with 'or' operator")); assert!(error_msg.contains("'age_gt', 'name'")); assert!(error_msg.contains("Instead of:")); assert!(error_msg.contains("Use:")); assert!(error_msg.contains("where: { 'age_gt', 'name', or: [...] }")); - assert!(error_msg.contains("where: { or: [{ 'age_gt', 'name', ... }, { 'age_gt', 'name', ... }] }")); + assert!(error_msg + .contains("where: { or: [{ 'age_gt', 'name', ... }, { 'age_gt', 'name', ... }] }")); } } From db315e20f8eac508a5bc0c27d393e34765825e92 Mon Sep 17 00:00:00 2001 From: Sebastian Lorenz Date: Tue, 15 Jul 2025 19:54:10 +0200 Subject: [PATCH 3/4] Refactor OR filter check to use if-let pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use more idiomatic Rust pattern 'if let Some(_) = object.get("or")' instead of 'let has_or = object.get("or").is_some(); if has_or'. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- graphql/src/store/query.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/graphql/src/store/query.rs b/graphql/src/store/query.rs index b87ac6f25d6..05c52f6c61c 100644 --- a/graphql/src/store/query.rs +++ b/graphql/src/store/query.rs @@ -241,8 +241,7 @@ fn build_filter_from_object<'a>( schema: &InputSchema, ) -> Result, QueryExecutionError> { // Check if we have both column filters and 'or' operator at the same level - let has_or = object.get("or").is_some(); - if has_or { + if let Some(_) = object.get("or") { let column_filters: Vec = object .iter() .filter_map(|(key, _)| { From 98a974d2a19436c5d2c4dc8d20f3c834614bc0c3 Mon Sep 17 00:00:00 2001 From: Sebastian Lorenz Date: Wed, 16 Jul 2025 12:19:07 +0200 Subject: [PATCH 4/4] Optimize: Store filter list in variable to avoid repeated joins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As suggested by @incrypto32, store column_filters.join(", ") in a variable instead of calling it three times. This improves performance and makes the code cleaner. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- graphql/src/store/query.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/graphql/src/store/query.rs b/graphql/src/store/query.rs index 05c52f6c61c..f98814f9f18 100644 --- a/graphql/src/store/query.rs +++ b/graphql/src/store/query.rs @@ -254,11 +254,12 @@ fn build_filter_from_object<'a>( .collect(); if !column_filters.is_empty() { + let filter_list = column_filters.join(", "); let example = format!( "Instead of:\nwhere: {{ {}, or: [...] }}\n\nUse:\nwhere: {{ or: [{{ {}, ... }}, {{ {}, ... }}] }}", - column_filters.join(", "), - column_filters.join(", "), - column_filters.join(", ") + filter_list, + filter_list, + filter_list ); return Err(QueryExecutionError::InvalidOrFilterStructure( column_filters,