Skip to content

fix: fix parent node jump lookup #1252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
devsergiy merged 2 commits into from
Jul 28, 2025
Merged

fix: fix parent node jump lookup #1252

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6401a90
fix: fix parent node jump lookup
devsergiy Jul 28, 2025
d0f19c3
Merge branch 'master' into sergiy/eng-7686-fix-parent-node-jump-lookup
devsergiy Jul 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions v2/pkg/engine/plan/datasource_filter_visitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,16 @@ func (f *DataSourceFilter) selectUniqNodeParentsUpToRootNode(i int) {
nodesIdsToSelect = append(nodesIdsToSelect, parentIdx)

if f.nodes.items[parentIdx].IsExternal && !f.nodes.items[i].IsProvided {
// such parent can't be selected
break
// such a parent can't be selected,
// so we skip this parent but continue looking for a potential root node higher
current = parentIdx
continue
}

// TODO: there could be a potential situation when we have selected root node with enabled entity resolver,
// but we can't jump to it because no parent could provide a key for it
// Need to consider how to move this logic into the selection process of duplicated nodes maybe?

Comment on lines +264 to +267

@coderabbitai coderabbitai Bot Jul 28, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Address the TODO regarding unreachable root nodes.

The TODO comment highlights a critical edge case where a root node with an enabled entity resolver might be selected but remain unreachable because no parent can provide the required key. This could lead to invalid query plans.

Please verify if this edge case can occur in practice and consider implementing the suggested integration with the duplicate node selection process. You can run the following script to search for related test cases or similar logic patterns:

🏁 Script executed:

#!/bin/bash
# Search for test cases related to entity resolvers and key provision
echo "Searching for entity resolver tests..."
rg -A 10 -B 5 "entity.*resolver|DisabledEntityResolver" --type go

echo -e "\nSearching for key provision logic..."
rg -A 5 -B 5 "provide.*key|requiresKey" --type go

echo -e "\nSearching for duplicate node selection logic..."
rg -A 10 -B 5 "selectDuplicateNodes|duplicated.*node" --type go

Length of output: 67504

Implement handling for unreachable root nodes with enabled entity resolver

The TODO at v2/pkg/engine/plan/datasource_filter_visitor.go (around lines 264–267)

// TODO: there could be a potential situation when we have selected root node with enabled entity resolver,
// but we can't jump to it because no parent could provide a key for it
// Need to consider how to move this logic into the selection process of duplicated nodes maybe?

remains unaddressed and no existing tests cover a scenario where:

  • a node is marked IsRootNode && !DisabledEntityResolver
  • but its ancestor chain cannot provide the required key path (isSelectedParentCouldProvideKeysForCurrentNode fails all the way to the root)
  • causing the planner to select an unreachable entry point

To resolve:

• In selectDuplicateNodes (and/or the initial root-selection stage), filter out any IsRootNode && !DisabledEntityResolver candidates when there is no valid parent-to-child key path.
• Add a new unit test in datasource_filter_visitor that constructs a query plan where a root entity is resolvable but disconnected (no key-providing parent) and asserts it is correctly skipped or replaced by an alternate duplicate with a valid key path.

Fix at:

  • v2/pkg/engine/plan/datasource_filter_visitor.go, method(s) around root-node discovery and in selectDuplicateNodes
  • Add corresponding test in v2/pkg/engine/plan/datasource_filter_visitor_test.go
🤖 Prompt for AI Agents 
In v2/pkg/engine/plan/datasource_filter_visitor.go around lines 264 to 267, the
TODO about handling root nodes with enabled entity resolver but no valid parent
key path is unaddressed. Fix this by updating the selectDuplicateNodes function
and/or the root-selection logic to filter out any nodes marked IsRootNode with
DisabledEntityResolver false when no parent can provide the required key path.
Additionally, add a unit test in
v2/pkg/engine/plan/datasource_filter_visitor_test.go that creates a query plan
with such a disconnected root entity and verifies it is skipped or replaced by a
valid duplicate node with a proper key path.

if f.nodes.items[parentIdx].IsRootNode && !f.nodes.items[parentIdx].DisabledEntityResolver {
rootNodeFound = true
break
Expand Down
Loading