Skip to content

Unified: Fix bad join - #22971

Merged
hvitved merged 1 commit into
github:mainfrom
hvitved:unified/fix-join
Nov 2, 2026
Merged

hvitved merged 1 commit into
github:mainfrom
hvitved:unified/fix-join

Conversation

@hvitved

@hvitved hvitved commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Before

Evaluated relational algebra for predicate StaticNameBinding::unqualifiedMemberAccess/4#0730f1f3@db9ae7oh with tuple counts:
           275976   ~4%    {2} r1 = SCAN `StaticNameBinding::unqualifiedMemberAccessDepth/1#8b18133c` OUTPUT In.1, In.0
        583240193   ~1%    {2}    | JOIN WITH `FacadeAst::Unified::AstNode.getDepth/0#e89ff1d9#bf_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1
           276222  ~10%    {4}    | JOIN WITH `StaticNameBinding::unqualifiedMemberAccessCand/4#dd3e0587_0312#join_rhs` ON FIRST 2 OUTPUT Lhs.0, Rhs.2, Rhs.3, Lhs.1
                           return r1

After

Evaluated relational algebra for predicate StaticNameBinding::unqualifiedMemberAccess/4#0730f1f3@6b447707 with tuple counts:
        276222   ~0%    {5} r1 = JOIN `StaticNameBinding::unqualifiedMemberAccessCand/4#dd3e0587` WITH `StaticNameBinding::unqualifiedMemberAccessDepth/1#8b18133c` ON FIRST 1 OUTPUT Lhs.3, Rhs.1, Lhs.1, Lhs.2, Lhs.0
        276222  ~10%    {4}    | JOIN WITH `FacadeAst::Unified::AstNode.getDepth/0#e89ff1d9` ON FIRST 2 OUTPUT Lhs.4, Lhs.2, Lhs.3, Lhs.0
                        return r1

DCA show a small speedup.

Before
```
Evaluated relational algebra for predicate StaticNameBinding::unqualifiedMemberAccess/4#0730f1f3@db9ae7oh with tuple counts:
           275976   ~4%    {2} r1 = SCAN `StaticNameBinding::unqualifiedMemberAccessDepth/1#8b18133c` OUTPUT In.1, In.0
        583240193   ~1%    {2}    | JOIN WITH `FacadeAst::Unified::AstNode.getDepth/0#e89ff1d9#bf_10#join_rhs` ON FIRST 1 OUTPUT Lhs.1, Rhs.1
           276222  ~10%    {4}    | JOIN WITH `StaticNameBinding::unqualifiedMemberAccessCand/4#dd3e0587_0312#join_rhs` ON FIRST 2 OUTPUT Lhs.0, Rhs.2, Rhs.3, Lhs.1
                           return r1
```

After
```
Evaluated relational algebra for predicate StaticNameBinding::unqualifiedMemberAccess/4#0730f1f3@6b447707 with tuple counts:
        276222   ~0%    {5} r1 = JOIN `StaticNameBinding::unqualifiedMemberAccessCand/4#dd3e0587` WITH `StaticNameBinding::unqualifiedMemberAccessDepth/1#8b18133c` ON FIRST 1 OUTPUT Lhs.3, Rhs.1, Lhs.1, Lhs.2, Lhs.0
        276222  ~10%    {4}    | JOIN WITH `FacadeAst::Unified::AstNode.getDepth/0#e89ff1d9` ON FIRST 2 OUTPUT Lhs.4, Lhs.2, Lhs.3, Lhs.0
                        return r1
```
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Nov 2, 2026
@hvitved
hvitved marked this pull request as ready for review November 2, 2026 11:59
@hvitved
hvitved requested a review from a team as a code owner November 2, 2026 11:59
Copilot AI auto review requested due to automatic review settings November 2, 2026 11:59

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.

🟢 Approval recommended

No active review issues are reported for the join-ordering annotations in static name binding.

No open issues 🎉

What changed in this PR

This PR improves join ordering in Unified’s static name binding, reducing intermediate results when resolving unqualified member accesses.

Changes:

  • Adds pragma[only_bind_into] to the access and accessingClass candidate arguments.
  • Guides evaluation toward the smaller candidate join before checking class depth, without changing the predicate’s logical conditions.
File Description
unified/​ql/​lib/​codeql/​unified/​internal/​StaticNameBinding.qll Adds binding annotations to improve join ordering in unqualifiedMemberAccess.

Review effort: Balanced (auto)

@hvitved
hvitved requested a review from aschackmull November 2, 2026 12:55
@hvitved
hvitved merged commit f9af6ab into github:main Nov 2, 2026
17 checks passed
@hvitved
hvitved deleted the unified/fix-join branch November 2, 2026 13:14
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.

3 participants