fix(dual-query-planner): use semantic diff when comparing Defer nodes (#6110)

Author: duckki

apollo-router/src/query_planner/dual_query_planner.rs

@@ -300,15 +300,20 @@ fn operation_matches(
     this: &SubgraphOperation,
     other: &SubgraphOperation,
 ) -> Result<(), MatchFailure> {
-    let this_ast = match ast::Document::parse(this.as_serialized(), "this_operation.graphql") {
+    document_str_matches(this.as_serialized(), other.as_serialized())
+}
+
+// Compare operation document strings such as query or just selection set.
+fn document_str_matches(this: &str, other: &str) -> Result<(), MatchFailure> {
+    let this_ast = match ast::Document::parse(this, "this_operation.graphql") {
         Ok(document) => document,
         Err(_) => {
             return Err(MatchFailure::new(
                 "Failed to parse this operation".to_string(),
             ));
         }
     };
-    let other_ast = match ast::Document::parse(other.as_serialized(), "other_operation.graphql") {
+    let other_ast = match ast::Document::parse(other, "other_operation.graphql") {
         Ok(document) => document,
         Err(_) => {
             return Err(MatchFailure::new(
@@ -319,6 +324,20 @@ fn operation_matches(
     same_ast_document(&this_ast, &other_ast)
 }
 
+fn opt_document_string_matches(
+    this: &Option<String>,
+    other: &Option<String>,
+) -> Result<(), MatchFailure> {
+    match (this, other) {
+        (None, None) => Ok(()),
+        (Some(this_sel), Some(other_sel)) => document_str_matches(this_sel, other_sel),
+        _ => Err(MatchFailure::new(format!(
+            "mismatched at opt_document_string_matches\nleft: {:?}\nright: {:?}",
+            this, other
+        ))),
+    }
+}
+
 // The rest is calling the comparison functions above instead of `PartialEq`,
 // but otherwise behave just like `PartialEq`:
 
@@ -494,8 +513,8 @@ fn plan_node_matches(this: &PlanNode, other: &PlanNode) -> Result<(), MatchFailu
                 deferred: other_deferred,
             },
         ) => {
-            check_match!(defer_primary_node_matches(primary, other_primary));
-            check_match!(vec_matches(deferred, other_deferred, deferred_node_matches));
+            defer_primary_node_matches(primary, other_primary)?;
+            vec_matches_result(deferred, other_deferred, deferred_node_matches)?;
         }
         (
             PlanNode::Subscription { primary, rest },
@@ -536,24 +555,30 @@ fn plan_node_matches(this: &PlanNode, other: &PlanNode) -> Result<(), MatchFailu
     Ok(())
 }
 
-fn defer_primary_node_matches(this: &Primary, other: &Primary) -> bool {
+fn defer_primary_node_matches(this: &Primary, other: &Primary) -> Result<(), MatchFailure> {
     let Primary { subselection, node } = this;
-    *subselection == other.subselection && opt_plan_node_matches(node, &other.node).is_ok()
+    opt_document_string_matches(subselection, &other.subselection)
+        .map_err(|err| err.add_description("under defer primary subselection"))?;
+    opt_plan_node_matches(node, &other.node)
+        .map_err(|err| err.add_description("under defer primary plan node"))
 }
 
-fn deferred_node_matches(this: &DeferredNode, other: &DeferredNode) -> bool {
+fn deferred_node_matches(this: &DeferredNode, other: &DeferredNode) -> Result<(), MatchFailure> {
     let DeferredNode {
         depends,
         label,
         query_path,
         subselection,
         node,
     } = this;
-    *depends == other.depends
-        && *label == other.label
-        && *query_path == other.query_path
-        && *subselection == other.subselection
-        && opt_plan_node_matches(node, &other.node).is_ok()
+
+    check_match_eq!(*depends, other.depends);
+    check_match_eq!(*label, other.label);
+    check_match_eq!(*query_path, other.query_path);
+    opt_document_string_matches(subselection, &other.subselection)
+        .map_err(|err| err.add_description("under deferred subselection"))?;
+    opt_plan_node_matches(node, &other.node)
+        .map_err(|err| err.add_description("under deferred node"))
 }
 
 fn flatten_node_matches(this: &FlattenNode, other: &FlattenNode) -> Result<(), MatchFailure> {