Skip to content

Unified: Join on both name and namespace in derivedStoreReadStep - #22717

Open
hvitved wants to merge 1 commit into
github:mainfrom
hvitved:unified/derived-read-store-step-multi-join
Open

hvitved wants to merge 1 commit into
github:mainfrom
hvitved:unified/derived-read-store-step-multi-join

Conversation

@hvitved

@hvitved hvitved commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

#22707 avoided joining on name alone, mening we joined on namespace. However, it is better to join on both columns. DCA is uneventful.

@hvitved hvitved added the no-change-note-required This PR does not need a change note label Oct 1, 2026
@hvitved
hvitved marked this pull request as ready for review October 1, 2026 09:47
@hvitved
hvitved requested a review from a team as a code owner October 1, 2026 09:47
Copilot AI balanced review requested due to automatic review settings October 1, 2026 09:47

Copilot AI left a comment

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.

Copilot review overview

🟢 Approval recommended

The focused, semantics-preserving refactor correctly implements the intended join optimization.

Review effort: Balanced
Findings: None

What changed in this PR

Optimizes derivedStoreReadStep to join on both namespace and name.

Changes:

  • Adds a non-magic helper predicate around readStep.
  • Preserves binding direction while enabling the two-column join.
File Description
unified/​ql/​lib/​codeql/​unified/​internal/​StaticNameBinding.qll Refactors derived store/read matching for a more efficient join.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Unified

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants