fix: planner fixes for abstract selection rewriter, node selection rules

v2/pkg/engine/plan/abstract_selection_rewriter.go

@@ -406,7 +406,7 @@ func (r *fieldSelectionRewriter) processInterfaceSelection(fieldRef int, interfa
 		4) for types which have inline-fragment - add not selected shared fields to existing inline fragment
 	*/
 
-	interfaceTypeNames, isInterfaceObject, err := r.getAllowedInterfaceMemberTypeNames(fieldRef, interfaceDefRef, enclosingTypeName)
+	interfaceTypeName, interfaceTypeNames, isInterfaceObject, err := r.getAllowedInterfaceMemberTypeNames(fieldRef, interfaceDefRef, enclosingTypeName)
 	if err != nil {
 		return resultNotRewritten, err
 	}
@@ -418,7 +418,7 @@ func (r *fieldSelectionRewriter) processInterfaceSelection(fieldRef int, interfa
 	}
 	selectionSetInfo.isInterfaceObject = isInterfaceObject
 
-	needRewrite := r.interfaceFieldSelectionNeedsRewrite(selectionSetInfo, interfaceTypeNames, entityNames)
+	needRewrite := r.interfaceFieldSelectionNeedsRewrite(selectionSetInfo, interfaceTypeNames, entityNames, interfaceTypeName)
 	if !needRewrite {
 		return resultNotRewritten, nil
 	}
@@ -444,11 +444,43 @@ func (r *fieldSelectionRewriter) processInterfaceSelection(fieldRef int, interfa
 	}, nil
 }
 
-func (r *fieldSelectionRewriter) interfaceFieldSelectionNeedsRewrite(selectionSetInfo selectionSetInfo, interfaceTypeNames []string, entityNames []string) (needRewrite bool) {
+func (r *fieldSelectionRewriter) interfaceFieldSelectionNeedsRewrite(selectionSetInfo selectionSetInfo, interfaceTypeNames []string, entityNames []string, interfaceTypeName string) (needRewrite bool) {
 	if r.mustRewrite(selectionSetInfo) {
 		return true
 	}
 
+	if selectionSetInfo.hasFields {
+		// We check that all selected fields are defined on the interface type.
+		// If all implementing types have the field local to the datasource, but interface does not define it - we won't be able to plan such fields.
+		// example:
+		//
+		// current datasource schema:
+		// interface Node {
+		//   id: ID!
+		// }
+		//
+		// type User implements Node {
+		//   id: ID!          <-- local to the current datasource
+		//   name: String!    <-- local to the current datasource, but not defined on the interface
+		// }
+		//
+		// other datasource schema:
+		// interface Node {
+		//   id: ID!
+		//   name: String! <-- defined on the interface and concrete type
+		// }
+		//
+		// type User implements Node {
+		//   id: ID!
+		//   name: String!
+		// }
+		//
+
+		if !r.typeHasAllFieldLocal(interfaceTypeName, selectionSetInfo.fields) {
+			return true
+		}
+	}
+
 	// when we do not have fragments
 	if !selectionSetInfo.hasInlineFragmentsOnInterfaces &&
 		!selectionSetInfo.hasInlineFragmentsOnUnions &&

v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go

@@ -534,30 +534,30 @@ func (r *fieldSelectionRewriter) getAllowedUnionMemberTypeNames(fieldRef int, un
 	return unionTypeNames, nil
 }
 
-func (r *fieldSelectionRewriter) getAllowedInterfaceMemberTypeNames(fieldRef int, interfaceDefRef int, enclosingTypeName ast.ByteSlice) (typeNames []string, isInterfaceObject bool, err error) {
-	interfaceTypeName := r.definition.InterfaceTypeDefinitionNameString(interfaceDefRef)
+func (r *fieldSelectionRewriter) getAllowedInterfaceMemberTypeNames(fieldRef int, interfaceDefRef int, enclosingTypeName ast.ByteSlice) (interfaceTypeName string, typeNames []string, isInterfaceObject bool, err error) {
+	interfaceTypeName = r.definition.InterfaceTypeDefinitionNameString(interfaceDefRef)
 	interfaceTypeNamesFromDefinition, _ := r.definition.InterfaceTypeDefinitionImplementedByObjectWithNames(interfaceDefRef)
 
 	// CurrentObject.field typename from the upstream schema
 	fieldTypeName, ok := r.fieldTypeNameFromUpstreamSchema(fieldRef, enclosingTypeName)
 	if !ok {
-		return nil, false, errors.New("unexpected error: field type name is not found in the upstream schema")
+		return "", nil, false, errors.New("unexpected error: field type name is not found in the upstream schema")
 	}
 
 	// when typename of a field is not equal to the typename of the interface type
 	// then it should implement the interface type in the federated graph schema
 	if interfaceTypeName != fieldTypeName {
 		if slices.Contains(interfaceTypeNamesFromDefinition, fieldTypeName) {
-			return []string{fieldTypeName}, false, nil
+			return fieldTypeName, []string{fieldTypeName}, false, nil
 		}
 
 		// if it doesn't implement an interface type the config is corrupted
-		return nil, false, errors.New("unexpected error: field type do not implement the interface in the federated graph schema")
+		return "", nil, false, errors.New("unexpected error: field type do not implement the interface in the federated graph schema")
 	}
 
 	interfaceNode, hasNode := r.upstreamDefinition.NodeByNameStr(interfaceTypeName)
 	if !hasNode {
-		return nil, false, errors.New("unexpected error: interface type definition not found in the upstream schema")
+		return "", nil, false, errors.New("unexpected error: interface type definition not found in the upstream schema")
 	}
 
 	// when node kind is an interface type definition
@@ -590,18 +590,18 @@ func (r *fieldSelectionRewriter) getAllowedInterfaceMemberTypeNames(fieldRef int
 		// remove possible consecutive duplicates
 		interfaceTypeNames = slices.Compact(interfaceTypeNames)
 
-		return interfaceTypeNames, false, nil
+		return interfaceTypeName, interfaceTypeNames, false, nil
 	}
 
 	// otherwise we should get node kind object type definition
 	// which means we are dealing with the interface object
 	for _, k := range r.dsConfiguration.FederationConfiguration().InterfaceObjects {
 		if k.InterfaceTypeName == interfaceTypeName {
-			return k.ConcreteTypeNames, true, nil
+			return interfaceTypeName, k.ConcreteTypeNames, true, nil
 		}
 	}
 
-	return nil, false, errors.New("unexpected error: node kind is not interface type definition in the upstream schema")
+	return "", nil, false, errors.New("unexpected error: node kind is not interface type definition in the upstream schema")
 }
 
 func (r *fieldSelectionRewriter) typeNameWithRename(typeName ast.ByteSlice) ast.ByteSlice {

v2/pkg/engine/plan/abstract_selection_rewriter_test.go

@@ -1309,6 +1309,7 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
 				RootNode("Query", "iface").
 				RootNode("User", "id", "isUser").
 				RootNode("Admin", "id").
+				ChildNode("Node", "id", "name").
 				KeysMetadata(FederationFieldConfigurations{
 					{
 						TypeName:     "User",
@@ -1357,6 +1358,7 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
 				RootNode("Query", "iface").
 				RootNode("User", "id", "name").
 				RootNode("Admin", "id", "name").
+				ChildNode("Node", "id", "name").
 				KeysMetadata(FederationFieldConfigurations{
 					{
 						TypeName:     "User",
@@ -1443,6 +1445,7 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
 				RootNode("Query", "iface").
 				RootNode("User", "id", "name", "isUser").
 				RootNode("Admin", "id", "name").
+				ChildNode("Node", "id", "name").
 				KeysMetadata(FederationFieldConfigurations{
 					{
 						TypeName:     "User",
@@ -1480,6 +1483,7 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
 			upstreamDefinition: definition,
 			dsBuilder: dsb().
 				RootNode("Query", "iface").
+				ChildNode("Node", "id", "name").
 				ChildNode("User", "id", "name", "isUser").
 				ChildNode("Admin", "id", "name"),
 			operation: `
@@ -4074,6 +4078,75 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
 				}`,
 			shouldRewrite: true,
 		},
+		{
+			name: "field is an interface, which do not have all fields in this datasource",
+			definition: `
+				interface Named {
+					id: ID!
+					name: String!
+				}
+
+				type User implements Named {
+					id: ID!
+					name: String!
+					surname: String!
+				}
+
+				type Admin implements Named {
+					id: ID!
+					name: String!
+					title: String!
+				}
+
+				type Query {
+					user: Named!
+				}`,
+			upstreamDefinition: `
+				interface Named {
+					id: ID!
+				}
+
+				type User implements Named {
+					id: ID!
+					name: String!
+					surname: String!
+				}
+
+				type Admin implements Named {
+					id: ID!
+					name: String!
+					title: String!
+				}
+
+				type Query {
+					user: Named!
+				}`,
+			dsBuilder: dsb().
+				RootNode("Admin", "id", "name", "title").
+				RootNode("User", "id", "name", "surname").
+				ChildNode("Named", "id"),
+			fieldName: "user",
+			operation: `
+				query {
+					__typename
+					user {
+						name
+					}
+				}`,
+			expectedOperation: `
+				query {
+					__typename
+					user {
+						... on Admin {
+							name
+						}
+						... on User {
+							name
+						}
+					}
+				}`,
+			shouldRewrite: true,
+		},
 	}
 
 	for _, testCase := range testCases {

v2/pkg/engine/plan/datasource_filter_visitor.go

@@ -442,40 +442,54 @@ func (f *DataSourceFilter) selectDuplicateNodes(secondPass bool) {
 			continue
 		}
 
-		// Select node based on a check for selected parent of a current node or its duplicates
-		if f.checkNodes(itemIDs, f.checkNodeParent, nil) {
+		// Select a node based on a check for the selected parent of a current node or its duplicates
+		if f.checkNodes(itemIDs, f.checkNodeParent, func(i int) (skip bool) {
+			if f.nodes.items[i].IsLeaf {
+				return false
+			}
+
+			if !f.nodes.items[i].IsRootNode {
+				return false
+			}
+
+			if !f.couldProvideChildFields(i) && !f.nodeCouldProvideKeysToChildNodes(i) {
+				return true
+			}
+
+			return false
+		}) {
 			continue
 		}
 
 		// we are checking if we have selected any child fields on the same datasource
-		// it means that child was unique, so we could select parent of such child
-		if f.checkNodes(itemIDs, f.checkNodeChilds, func(i int) bool {
+		// it means that the child was unique, so we could select the parent of such a child
+		if f.checkNodes(itemIDs, f.checkNodeChilds, func(i int) (skip bool) {
 			// do not evaluate childs for the leaf nodes
 			return f.nodes.items[i].IsLeaf
 		}) {
 			continue
 		}
 
 		// we are checking if we have selected any sibling fields on the same datasource
-		// if sibling is selected on the same datasource, we could select current node
-		if f.checkNodes(itemIDs, f.checkNodeSiblings, func(i int) bool {
-			// we skip non leaf nodes which could not provide any child selections
+		// if a sibling is selected on the same datasource, we could select a current node
+		if f.checkNodes(itemIDs, f.checkNodeSiblings, func(i int) (skip bool) {
+			// we skip non-leaf nodes which could not provide any child selections
 			if !f.nodes.items[i].IsLeaf && !f.couldProvideChildFields(i) {
 				return true
 			}
 
-			// we should not select a __typename field based on a siblings, unless it is on a root query type
+			// we should not select a __typename field based on a sibling, unless it is on a root query type
 			return f.nodes.items[i].isTypeName && !IsMutationOrQueryRootType(f.nodes.items[i].TypeName)
 		}) {
 			continue
 		}
 
-		// if after all checks node was not selected
+		// if after all checks node was not selected,
 		// we need a couple more checks
 
 		// 1. Lookup in duplicates for root nodes with enabled reference resolver
 		// in case current node suggestion is an entity root node, and it contains a key with disabled resolver
-		// we could not select such node, because we could not jump to the subgraph which do not have reference resolver,
+		// we could not select such a node, because we could not jump to the subgraph which do not have a reference resolver,
 		// so we need to find a possible duplicate which has enabled entity resolver
 		// The tricky part here is to check that the parent node could provide keys for the current node
 
@@ -492,17 +506,17 @@ func (f *DataSourceFilter) selectDuplicateNodes(secondPass bool) {
 					return true
 				}
 
-				// when node is a root query node we will not have parent
-				// so we need to check if parent node id is not a root of a tree
+				// when a node is a root query node, we will not have a parent
+				// so we need to check if the parent node id is not a root of a tree
 				if treeNode.GetParentID() != treeRootID {
-					// we need to check if the node with enabled resolver could actually get a key from the parent node
+					// we need to check if the node with the enabled resolver could actually get a key from the parent node
 					if !f.isSelectedParentCouldProvideKeysForCurrentNode(i) {
 						return true
 					}
 				}
 
-				// if node is not a leaf we need to check if it is possible to get any fields (not counting __typename) from this datasource
-				// it may be the case that all fields belongs to different datasource, but this node could serve as the connection point
+				// if node is not a leaf, we need to check if it is possible to get any fields (not counting __typename) from this datasource
+				// it may be the case that all fields belong to different datasource, but this node could serve as the connection point
 				// to the other datasources, so we check if parent could provide a key
 				// and that we could provide a key to the next childs
 				if f.nodes.items[i].IsLeaf {
@@ -640,6 +654,10 @@ func (f *DataSourceFilter) findPossibleParents(i int) (parentIds []int) {
 
 	parentIdx, _ := f.nodes.parentNodeOnSameSource(i)
 	for parentIdx != -1 {
+		if f.nodes.items[parentIdx].IsExternal && !f.nodes.items[i].IsProvided {
+			break
+		}
+
 		nodesIdsToSelect = append(nodesIdsToSelect, parentIdx)
 
 		// for the parent node we are checking if it is a root node and has enabled entity resolver.
@@ -882,7 +900,7 @@ func (f *DataSourceFilter) selectWithExternalCheck(i int, reason string) (nodeIs
 func (f *DataSourceFilter) couldProvideChildFields(i int) bool {
 	nodesIds := f.nodes.childNodesOnSameSource(i)
 
-	hasFields := false
+	hasSelectableFields := false
 	for _, i := range nodesIds {
 		if f.nodes.items[i].isTypeName {
 			// we have to omit __typename field
@@ -894,10 +912,22 @@ func (f *DataSourceFilter) couldProvideChildFields(i int) bool {
 			continue
 		}
 
-		hasFields = true
+		if f.nodes.items[i].IsLeaf {
+			// when the node is a leaf, it could be selected
+			hasSelectableFields = true
+			break
+		}
+
+		// for non-leaf nodes, we need to check again if they could provide any child fields
+		// or keys to the nested child nodes
+		// this ensures bigger chains of non-leaf nodes selected
+		if f.couldProvideChildFields(i) || f.nodeCouldProvideKeysToChildNodes(i) {
+			hasSelectableFields = true
+			break
+		}
 	}
 
-	return hasFields
+	return hasSelectableFields
 }
 
 func (f *DataSourceFilter) childsHasOnlyTypename(i int) (hasOnlyTypename bool) {