Skip to content

Commit 4fbdf0b

Browse files
authored
Improve error message for OR operator with column filters (#6078)
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 Fixes #6041 Closes #6041
1 parent 97992cb commit 4fbdf0b

File tree

2 files changed

+267
-0
lines changed

2 files changed

+267
-0
lines changed

graph/src/data/query/error.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ pub enum QueryExecutionError {
4141
FilterNotSupportedError(String, String),
4242
UnknownField(Pos, String, String),
4343
EmptyQuery,
44+
InvalidOrFilterStructure(Vec<String>, String),
4445
SubgraphDeploymentIdError(String),
4546
RangeArgumentsError(&'static str, u32, i64),
4647
InvalidFilterError,
@@ -97,6 +98,7 @@ impl QueryExecutionError {
9798
| ChildFilterNestingNotSupportedError(_, _)
9899
| UnknownField(_, _, _)
99100
| EmptyQuery
101+
| InvalidOrFilterStructure(_, _)
100102
| SubgraphDeploymentIdError(_)
101103
| InvalidFilterError
102104
| EntityFieldError(_, _)
@@ -210,6 +212,10 @@ impl fmt::Display for QueryExecutionError {
210212
write!(f, "The `{}` argument must be between 0 and {}, but is {}", arg, max, actual)
211213
}
212214
InvalidFilterError => write!(f, "Filter must by an object"),
215+
InvalidOrFilterStructure(fields, example) => {
216+
write!(f, "Cannot mix column filters with 'or' operator at the same level. Found column filter(s) {} alongside 'or' operator.\n\n{}",
217+
fields.join(", "), example)
218+
}
213219
EntityFieldError(e, a) => {
214220
write!(f, "Entity `{}` has no attribute `{}`", e, a)
215221
}

graphql/src/store/query.rs

Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,34 @@ fn build_filter_from_object<'a>(
240240
object: &Object,
241241
schema: &InputSchema,
242242
) -> Result<Vec<EntityFilter>, QueryExecutionError> {
243+
// Check if we have both column filters and 'or' operator at the same level
244+
if let Some(_) = object.get("or") {
245+
let column_filters: Vec<String> = object
246+
.iter()
247+
.filter_map(|(key, _)| {
248+
if key != "or" && key != "and" && key != "_change_block" {
249+
Some(format!("'{}'", key))
250+
} else {
251+
None
252+
}
253+
})
254+
.collect();
255+
256+
if !column_filters.is_empty() {
257+
let filter_list = column_filters.join(", ");
258+
let example = format!(
259+
"Instead of:\nwhere: {{ {}, or: [...] }}\n\nUse:\nwhere: {{ or: [{{ {}, ... }}, {{ {}, ... }}] }}",
260+
filter_list,
261+
filter_list,
262+
filter_list
263+
);
264+
return Err(QueryExecutionError::InvalidOrFilterStructure(
265+
column_filters,
266+
example,
267+
));
268+
}
269+
}
270+
243271
object
244272
.iter()
245273
.map(|(key, value)| {
@@ -957,4 +985,237 @@ mod tests {
957985
Some(EntityFilter::And(vec![EntityFilter::ChangeBlockGte(10)]))
958986
)
959987
}
988+
989+
#[test]
990+
fn build_query_detects_invalid_or_filter_structure() {
991+
// Test that mixing column filters with 'or' operator produces a helpful error
992+
let query_field = default_field_with(
993+
"where",
994+
r::Value::Object(Object::from_iter(vec![
995+
("name".into(), r::Value::String("John".to_string())),
996+
(
997+
"or".into(),
998+
r::Value::List(vec![r::Value::Object(Object::from_iter(vec![(
999+
"email".into(),
1000+
r::Value::String("[email protected]".to_string()),
1001+
)]))]),
1002+
),
1003+
])),
1004+
);
1005+
1006+
// We only allow one entity type in these tests
1007+
assert_eq!(query_field.selection_set.fields().count(), 1);
1008+
let obj_type = query_field
1009+
.selection_set
1010+
.fields()
1011+
.map(|(obj, _)| &obj.name)
1012+
.next()
1013+
.expect("there is one object type");
1014+
let Some(object) = INPUT_SCHEMA.object_or_interface(obj_type, None) else {
1015+
panic!("object type {} not found", obj_type);
1016+
};
1017+
1018+
let result = build_query(
1019+
&object,
1020+
BLOCK_NUMBER_MAX,
1021+
&query_field,
1022+
std::u32::MAX,
1023+
std::u32::MAX,
1024+
&*INPUT_SCHEMA,
1025+
);
1026+
1027+
assert!(result.is_err());
1028+
let error = result.unwrap_err();
1029+
1030+
// Check that we get the specific error we expect
1031+
match error {
1032+
graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => {
1033+
assert_eq!(fields, vec!["'name'"]);
1034+
assert!(example.contains("Instead of:"));
1035+
assert!(example.contains("where: { 'name', or: [...] }"));
1036+
assert!(example.contains("Use:"));
1037+
assert!(example.contains("where: { or: [{ 'name', ... }, { 'name', ... }] }"));
1038+
}
1039+
_ => panic!("Expected InvalidOrFilterStructure error, got: {}", error),
1040+
}
1041+
}
1042+
1043+
#[test]
1044+
fn build_query_detects_invalid_or_filter_structure_multiple_fields() {
1045+
// Test that multiple column filters with 'or' operator are all reported
1046+
let query_field = default_field_with(
1047+
"where",
1048+
r::Value::Object(Object::from_iter(vec![
1049+
("name".into(), r::Value::String("John".to_string())),
1050+
(
1051+
"email".into(),
1052+
r::Value::String("[email protected]".to_string()),
1053+
),
1054+
(
1055+
"or".into(),
1056+
r::Value::List(vec![r::Value::Object(Object::from_iter(vec![(
1057+
"name".into(),
1058+
r::Value::String("Jane".to_string()),
1059+
)]))]),
1060+
),
1061+
])),
1062+
);
1063+
1064+
// We only allow one entity type in these tests
1065+
assert_eq!(query_field.selection_set.fields().count(), 1);
1066+
let obj_type = query_field
1067+
.selection_set
1068+
.fields()
1069+
.map(|(obj, _)| &obj.name)
1070+
.next()
1071+
.expect("there is one object type");
1072+
let Some(object) = INPUT_SCHEMA.object_or_interface(obj_type, None) else {
1073+
panic!("object type {} not found", obj_type);
1074+
};
1075+
1076+
let result = build_query(
1077+
&object,
1078+
BLOCK_NUMBER_MAX,
1079+
&query_field,
1080+
std::u32::MAX,
1081+
std::u32::MAX,
1082+
&*INPUT_SCHEMA,
1083+
);
1084+
1085+
assert!(result.is_err());
1086+
let error = result.unwrap_err();
1087+
1088+
// Check that we get the specific error we expect
1089+
match error {
1090+
graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => {
1091+
// Should detect both column filters
1092+
assert_eq!(fields.len(), 2);
1093+
assert!(fields.contains(&"'name'".to_string()));
1094+
assert!(fields.contains(&"'email'".to_string()));
1095+
assert!(example.contains("Instead of:"));
1096+
assert!(example.contains("Use:"));
1097+
}
1098+
_ => panic!("Expected InvalidOrFilterStructure error, got: {}", error),
1099+
}
1100+
}
1101+
1102+
#[test]
1103+
fn build_query_allows_valid_or_filter_structure() {
1104+
// Test that valid 'or' filters without column filters at the same level work correctly
1105+
let query_field = default_field_with(
1106+
"where",
1107+
r::Value::Object(Object::from_iter(vec![(
1108+
"or".into(),
1109+
r::Value::List(vec![
1110+
r::Value::Object(Object::from_iter(vec![(
1111+
"name".into(),
1112+
r::Value::String("John".to_string()),
1113+
)])),
1114+
r::Value::Object(Object::from_iter(vec![(
1115+
"email".into(),
1116+
r::Value::String("[email protected]".to_string()),
1117+
)])),
1118+
]),
1119+
)])),
1120+
);
1121+
1122+
// This should not produce an error
1123+
let result = query(&query_field);
1124+
assert!(result.filter.is_some());
1125+
1126+
// Verify that the filter is correctly structured
1127+
match result.filter.unwrap() {
1128+
EntityFilter::And(filters) => {
1129+
assert_eq!(filters.len(), 1);
1130+
match &filters[0] {
1131+
EntityFilter::Or(_) => {
1132+
// This is expected - OR filter should be wrapped in AND
1133+
}
1134+
_ => panic!("Expected OR filter, got: {:?}", filters[0]),
1135+
}
1136+
}
1137+
_ => panic!("Expected AND filter with OR inside"),
1138+
}
1139+
}
1140+
1141+
#[test]
1142+
fn build_query_detects_invalid_or_filter_structure_with_operators() {
1143+
// Test that column filters with operators (like name_gt) are also detected
1144+
let query_field = default_field_with(
1145+
"where",
1146+
r::Value::Object(Object::from_iter(vec![
1147+
("name_gt".into(), r::Value::String("A".to_string())),
1148+
(
1149+
"or".into(),
1150+
r::Value::List(vec![r::Value::Object(Object::from_iter(vec![(
1151+
"email".into(),
1152+
r::Value::String("[email protected]".to_string()),
1153+
)]))]),
1154+
),
1155+
])),
1156+
);
1157+
1158+
// We only allow one entity type in these tests
1159+
assert_eq!(query_field.selection_set.fields().count(), 1);
1160+
let obj_type = query_field
1161+
.selection_set
1162+
.fields()
1163+
.map(|(obj, _)| &obj.name)
1164+
.next()
1165+
.expect("there is one object type");
1166+
let Some(object) = INPUT_SCHEMA.object_or_interface(obj_type, None) else {
1167+
panic!("object type {} not found", obj_type);
1168+
};
1169+
1170+
let result = build_query(
1171+
&object,
1172+
BLOCK_NUMBER_MAX,
1173+
&query_field,
1174+
std::u32::MAX,
1175+
std::u32::MAX,
1176+
&*INPUT_SCHEMA,
1177+
);
1178+
1179+
assert!(result.is_err());
1180+
let error = result.unwrap_err();
1181+
1182+
// Check that we get the specific error we expect
1183+
match error {
1184+
graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example) => {
1185+
assert_eq!(fields, vec!["'name_gt'"]);
1186+
assert!(example.contains("Instead of:"));
1187+
assert!(example.contains("where: { 'name_gt', or: [...] }"));
1188+
assert!(example.contains("Use:"));
1189+
assert!(example.contains("where: { or: [{ 'name_gt', ... }, { 'name_gt', ... }] }"));
1190+
}
1191+
_ => panic!("Expected InvalidOrFilterStructure error, got: {}", error),
1192+
}
1193+
}
1194+
1195+
#[test]
1196+
fn test_error_message_formatting() {
1197+
// Test that the error message is properly formatted
1198+
let fields = vec!["'age_gt'".to_string(), "'name'".to_string()];
1199+
let example = format!(
1200+
"Instead of:\nwhere: {{ {}, or: [...] }}\n\nUse:\nwhere: {{ or: [{{ {}, ... }}, {{ {}, ... }}] }}",
1201+
fields.join(", "),
1202+
fields.join(", "),
1203+
fields.join(", ")
1204+
);
1205+
1206+
let error =
1207+
graph::data::query::QueryExecutionError::InvalidOrFilterStructure(fields, example);
1208+
let error_msg = format!("{}", error);
1209+
1210+
println!("Error message:\n{}", error_msg);
1211+
1212+
// Verify the error message contains the key elements
1213+
assert!(error_msg.contains("Cannot mix column filters with 'or' operator"));
1214+
assert!(error_msg.contains("'age_gt', 'name'"));
1215+
assert!(error_msg.contains("Instead of:"));
1216+
assert!(error_msg.contains("Use:"));
1217+
assert!(error_msg.contains("where: { 'age_gt', 'name', or: [...] }"));
1218+
assert!(error_msg
1219+
.contains("where: { or: [{ 'age_gt', 'name', ... }, { 'age_gt', 'name', ... }] }"));
1220+
}
9601221
}

0 commit comments

Comments
 (0)