Skip to content

Commit

Permalink
fix: fix planning error for leaf nodes (wundergraph#641)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Aenimus @soonick
Aenimus authored and soonick committed Oct 31, 2023
1 parent d354164 commit 069383a
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 8 deletions.
17 changes: 9 additions & 8 deletions v2/pkg/engine/plan/datasource_filter_visitor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,25 +413,26 @@ func selectUniqNodes(nodes NodeSuggestions) []NodeSuggestion {
continue
}

isNodeUniq := nodes.isNodeUniq(i)
if !isNodeUniq {
isNodeUnique := nodes.isNodeUniq(i)
if !isNodeUnique {
continue
}

// uniq nodes are always has priority
// unique nodes always have priority
nodes[i].selectWithReason(ReasonStage1Uniq)

if !nodes[i].onFragment { // on a first stage do not select parent of nodes on fragments
// if node parent of the uniq node is on the same source, prioritize it too
// if node parent of the unique node is on the same source, prioritize it too
parentIdx, ok := nodes.parentNodeOnSameSource(i)
if ok {
// Only select the parent on this stage if the node is a leaf; otherwise, the parent is selected elsewhere
if ok && nodes.isLeaf(i) {
nodes[parentIdx].selectWithReason(ReasonStage1SameSourceParent)
}
}

// if node has leaf childs on the same source, prioritize them too
childs := nodes.childNodesOnSameSource(i)
for _, child := range childs {
// if node has leaf children on the same source, prioritize them too
children := nodes.childNodesOnSameSource(i)
for _, child := range children {
if nodes.isLeaf(child) && nodes.isNodeUniq(child) {
nodes[child].selectWithReason(ReasonStage1SameSourceLeafChild)
}
Expand Down
144 changes: 144 additions & 0 deletions v2/pkg/engine/plan/datasource_filter_visitor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,71 @@ func TestFindBestDataSourceSet(t *testing.T) {
},
},
},
{
Description: "Shareable: no root node parent",
Definition: conflictingPathsDefinition,
Query: `
query {
user {
id
object {
name
}
nested {
uniqueOne
uniqueTwo
nested {
shared
uniqueOne
uniqueTwo
}
}
}
}
`,
DataSources: []DataSourceConfiguration{
conflictingPaths1,
conflictingPaths2,
},
ExpectedVariants: []Variant{
{
dsOrder: []int{0, 1},
suggestions: NodeSuggestions{
{TypeName: "Query", FieldName: "user", DataSourceHash: 11, Path: "query.user", ParentPath: "query", IsRootNode: true, selected: true},
{TypeName: "User", FieldName: "id", DataSourceHash: 11, Path: "query.user.id", ParentPath: "query.user", IsRootNode: true, selected: true},
{TypeName: "User", FieldName: "object", DataSourceHash: 11, Path: "query.user.object", ParentPath: "query.user", IsRootNode: true, selected: true},
{TypeName: "Object", FieldName: "name", DataSourceHash: 11, Path: "query.user.object.name", ParentPath: "query.user.object", IsRootNode: false, selected: true},
{TypeName: "User", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, selected: true},
{TypeName: "User", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, selected: true},
{TypeName: "NestedOne", FieldName: "uniqueOne", DataSourceHash: 11, Path: "query.user.nested.uniqueOne", ParentPath: "query.user.nested", IsRootNode: false, selected: true},
{TypeName: "NestedOne", FieldName: "uniqueTwo", DataSourceHash: 22, Path: "query.user.nested.uniqueTwo", ParentPath: "query.user.nested", IsRootNode: false, selected: true},
{TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, selected: true},
{TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, selected: true},
{TypeName: "NestedTwo", FieldName: "shared", DataSourceHash: 11, Path: "query.user.nested.nested.shared", ParentPath: "query.user.nested.nested", IsRootNode: false, selected: true},
{TypeName: "NestedTwo", FieldName: "uniqueOne", DataSourceHash: 11, Path: "query.user.nested.nested.uniqueOne", ParentPath: "query.user.nested.nested", IsRootNode: false, selected: true},
{TypeName: "NestedTwo", FieldName: "uniqueTwo", DataSourceHash: 22, Path: "query.user.nested.nested.uniqueTwo", ParentPath: "query.user.nested.nested", IsRootNode: false, selected: true},
},
},
{
dsOrder: []int{1, 0},
suggestions: NodeSuggestions{
{TypeName: "Query", FieldName: "user", DataSourceHash: 11, Path: "query.user", ParentPath: "query", IsRootNode: true, selected: true},
{TypeName: "User", FieldName: "id", DataSourceHash: 11, Path: "query.user.id", ParentPath: "query.user", IsRootNode: true, selected: true},
{TypeName: "User", FieldName: "object", DataSourceHash: 11, Path: "query.user.object", ParentPath: "query.user", IsRootNode: true, selected: true},
{TypeName: "Object", FieldName: "name", DataSourceHash: 11, Path: "query.user.object.name", ParentPath: "query.user.object", IsRootNode: false, selected: true},
{TypeName: "User", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, selected: true},
{TypeName: "User", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested", ParentPath: "query.user", IsRootNode: true, selected: true},
{TypeName: "NestedOne", FieldName: "uniqueOne", DataSourceHash: 11, Path: "query.user.nested.uniqueOne", ParentPath: "query.user.nested", IsRootNode: false, selected: true},
{TypeName: "NestedOne", FieldName: "uniqueTwo", DataSourceHash: 22, Path: "query.user.nested.uniqueTwo", ParentPath: "query.user.nested", IsRootNode: false, selected: true},
{TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 22, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, selected: true},
{TypeName: "NestedOne", FieldName: "nested", DataSourceHash: 11, Path: "query.user.nested.nested", ParentPath: "query.user.nested", IsRootNode: false, selected: true},
{TypeName: "NestedTwo", FieldName: "shared", DataSourceHash: 22, Path: "query.user.nested.nested.shared", ParentPath: "query.user.nested.nested", IsRootNode: false, selected: true},
{TypeName: "NestedTwo", FieldName: "uniqueOne", DataSourceHash: 11, Path: "query.user.nested.nested.uniqueOne", ParentPath: "query.user.nested.nested", IsRootNode: false, selected: true},
{TypeName: "NestedTwo", FieldName: "uniqueTwo", DataSourceHash: 22, Path: "query.user.nested.nested.uniqueTwo", ParentPath: "query.user.nested.nested", IsRootNode: false, selected: true},
},
},
},
},
}

run := func(t *testing.T, Definition, Query string, DataSources []DataSourceConfiguration, expected NodeSuggestions) {
Expand Down Expand Up @@ -960,3 +1025,82 @@ var shareableDS3 = dsb().Hash(33).Schema(shareableDS3Schema).
RootNode("User", "id", "details").
ChildNode("Details", "age").
DS()

const conflictingPaths1Schema = `
type Query {
user: User!
}
type User @key(fields: "id") {
id: ID!
nested: NestedOne!
}
type NestedOne {
uniqueOne: String!
nested: NestedTwo!
}
type NestedTwo {
shared: String!
uniqueOne: Int!
}
`

var conflictingPaths1 = dsb().Hash(11).Schema(conflictingPaths1Schema).
RootNode("Query", "user").RootNode("User", "id", "nested", "object").
ChildNode("Object", "name").
ChildNode("NestedOne", "uniqueOne", "nested").
ChildNode("NestedTwo", "shared", "uniqueOne").
DS()

const conflictingPaths2Schema = `
type User @key(fields: "id") {
id: ID!
nested: NestedOne!
}
type NestedOne {
uniqueTwo: String!
nested: NestedTwo!
}
type NestedTwo {
shared: String!
uniqueTwo: Int!
}
`

var conflictingPaths2 = dsb().Hash(22).Schema(conflictingPaths2Schema).
RootNode("User", "id", "nested").
ChildNode("NestedOne", "uniqueTwo", "nested").
ChildNode("NestedTwo", "shared", "uniqueTwo").
DS()

var conflictingPathsDefinition = `
type Query {
user: User!
}
type User {
id: ID!
nested: NestedOne!
object: Object!
}
type Object {
name: String!
}
type NestedOne {
uniqueOne: String!
uniqueTwo: String!
nested: NestedTwo!
}
type NestedTwo {
shared: String!
uniqueOne: Int!
uniqueTwo: Int!
}
`

0 comments on commit 069383a

Please sign in to comment.