Skip to content

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Jan 6, 2026

Thanks to @nickrolfe for reporting. Note that the rewrite is not strictly semantics preserving, as we replace

result != any(Filter f).getFilterCallable()

with

not result = any(Filter f).getFilterCallable()

which I believe was the original intention.

Before

Evaluated relational algebra for predicate Filters::Filters::FilterCall.getAnAction/0#dispred#9c0da667@85a4cbtp with tuple counts:
           394650       ~2%    {2} r1 = `__#Module::ModuleBase.getAMethod/0#dispred#56626ed3Merge_Module::ModuleBase.getModule/0#dispred#4f2c__#shared` AND NOT `_Filters::Filters::FilterCall.getExceptArgument/0#dispred#515c95c0__#Method::Method.getName/0#dispre__#antijoin_rhs`(FIRST 2)
                               {2}    | AND NOT `project#Filters::Filters::FilterCall.getOnlyArgument/0#dispred#f337e70f`(FIRST 1)
           380366       ~0%    {2}    | SCAN OUTPUT In.1, In.0

            29453       ~0%    {2} r2 = JOIN `_#Module::ModuleBase.getAMethod/0#dispred#56626ed3Merge__#AST::AstNode.getEnclosingModule/0#dispred#__#shared` WITH project#ActionController::ActionControllerActionMethod#6db6f5e0 ON FIRST 1 OUTPUT Lhs.0, Lhs.1

           366017       ~0%    {2} r3 = JOIN `_#Module::ModuleBase.getAMethod/0#dispred#56626ed3Merge_Module::ModuleBase.getModule/0#dispred#4f2ca__#shared` WITH project#ActionController::ActionControllerActionMethod#6db6f5e0 ON FIRST 1 OUTPUT Lhs.0, Lhs.1

           395470       ~0%    {2} r4 = r2 UNION r3
           395470       ~0%    {3}    | JOIN WITH `Method::Method.getName/0#dispred#2acbf239` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0
             2227       ~0%    {2}    | JOIN WITH `Filters::Filters::FilterCall.getOnlyArgument/0#dispred#f337e70f` ON FIRST 2 OUTPUT Lhs.2, Lhs.0

           382593       ~0%    {2} r5 = r1 UNION r4
           133735       ~4%    {2}    | JOIN WITH `project#ActionController::ActionControllerActionMethod.getARoute/0#dispred#9eb85e56` ON FIRST 1 OUTPUT Lhs.1, Lhs.0
        540556870       ~2%    {3}    | JOIN WITH Filters::Filters::Filter#a42c5138 CARTESIAN PRODUCT OUTPUT Rhs.0, Lhs.0, Lhs.1
        525979755     ~127%    {3}    | JOIN WITH `Filters::Filters::FilterImpl.getFilterCallable/0#dispred#451bf7d7` ON FIRST 1 OUTPUT Lhs.1, Lhs.2, Rhs.1
                               {3}    | REWRITE WITH TEST InOut.1 != InOut.2
        525979755  ~407036%    {2}    | SCAN OUTPUT In.0, In.1
                               return r5

After

Evaluated relational algebra for predicate Filters::Filters::FilterCall.getAnAction/0#91dba45c@74dfcepp with tuple counts:
          1363   ~4%    {2} r1 = JOIN `Filters::Filters::FilterCall.getAnActionCand/1#f053150d` WITH `Filters::Filters::FilterCall.getOnlyArgument/0#dispred#f337e70f` ON FIRST 2 OUTPUT Lhs.0, Lhs.2

        140978   ~0%    {3} r2 = `Filters::Filters::FilterCall.getAnActionCand/1#f053150d` AND NOT `Filters::Filters::FilterCall.getExceptArgument/0#dispred#515c95c0#fb`(FIRST 2)
                        {3}    | AND NOT `project#Filters::Filters::FilterCall.getOnlyArgument/0#dispred#f337e70f`(FIRST 1)
        132372   ~3%    {2}    | SCAN OUTPUT In.0, In.2

        133735   ~4%    {2} r3 = r1 UNION r2
                        return r3

Before
```
Evaluated relational algebra for predicate Filters::Filters::FilterCall.getAnAction/0#dispred#9c0da667@85a4cbtp with tuple counts:
           394650       ~2%    {2} r1 = `__#Module::ModuleBase.getAMethod/0#dispred#56626ed3Merge_Module::ModuleBase.getModule/0#dispred#4f2c__#shared` AND NOT `_Filters::Filters::FilterCall.getExceptArgument/0#dispred#515c95c0__#Method::Method.getName/0#dispre__#antijoin_rhs`(FIRST 2)
                               {2}    | AND NOT `project#Filters::Filters::FilterCall.getOnlyArgument/0#dispred#f337e70f`(FIRST 1)
           380366       ~0%    {2}    | SCAN OUTPUT In.1, In.0

            29453       ~0%    {2} r2 = JOIN `_#Module::ModuleBase.getAMethod/0#dispred#56626ed3Merge__#AST::AstNode.getEnclosingModule/0#dispred#__#shared` WITH project#ActionController::ActionControllerActionMethod#6db6f5e0 ON FIRST 1 OUTPUT Lhs.0, Lhs.1

           366017       ~0%    {2} r3 = JOIN `_#Module::ModuleBase.getAMethod/0#dispred#56626ed3Merge_Module::ModuleBase.getModule/0#dispred#4f2ca__#shared` WITH project#ActionController::ActionControllerActionMethod#6db6f5e0 ON FIRST 1 OUTPUT Lhs.0, Lhs.1

           395470       ~0%    {2} r4 = r2 UNION r3
           395470       ~0%    {3}    | JOIN WITH `Method::Method.getName/0#dispred#2acbf239` ON FIRST 1 OUTPUT Lhs.1, Rhs.1, Lhs.0
             2227       ~0%    {2}    | JOIN WITH `Filters::Filters::FilterCall.getOnlyArgument/0#dispred#f337e70f` ON FIRST 2 OUTPUT Lhs.2, Lhs.0

           382593       ~0%    {2} r5 = r1 UNION r4
           133735       ~4%    {2}    | JOIN WITH `project#ActionController::ActionControllerActionMethod.getARoute/0#dispred#9eb85e56` ON FIRST 1 OUTPUT Lhs.1, Lhs.0
        540556870       ~2%    {3}    | JOIN WITH Filters::Filters::Filter#a42c5138 CARTESIAN PRODUCT OUTPUT Rhs.0, Lhs.0, Lhs.1
        525979755     ~127%    {3}    | JOIN WITH `Filters::Filters::FilterImpl.getFilterCallable/0#dispred#451bf7d7` ON FIRST 1 OUTPUT Lhs.1, Lhs.2, Rhs.1
                               {3}    | REWRITE WITH TEST InOut.1 != InOut.2
        525979755  ~407036%    {2}    | SCAN OUTPUT In.0, In.1
                               return r5
```

After
```
Evaluated relational algebra for predicate Filters::Filters::FilterCall.getAnAction/0#91dba45c@74dfcepp with tuple counts:
          1363   ~4%    {2} r1 = JOIN `Filters::Filters::FilterCall.getAnActionCand/1#f053150d` WITH `Filters::Filters::FilterCall.getOnlyArgument/0#dispred#f337e70f` ON FIRST 2 OUTPUT Lhs.0, Lhs.2

        140978   ~0%    {3} r2 = `Filters::Filters::FilterCall.getAnActionCand/1#f053150d` AND NOT `Filters::Filters::FilterCall.getExceptArgument/0#dispred#515c95c0#fb`(FIRST 2)
                        {3}    | AND NOT `project#Filters::Filters::FilterCall.getOnlyArgument/0#dispred#f337e70f`(FIRST 1)
        132372   ~3%    {2}    | SCAN OUTPUT In.0, In.2

        133735   ~4%    {2} r3 = r1 UNION r2
                        return r3
```
@github-actions github-actions bot added the Ruby label Jan 6, 2026
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Jan 6, 2026
@hvitved hvitved requested a review from nickrolfe January 6, 2026 12:36
@hvitved hvitved marked this pull request as ready for review January 6, 2026 12:36
@hvitved hvitved requested a review from a team as a code owner January 6, 2026 12:36
Copilot AI review requested due to automatic review settings January 6, 2026 12:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a performance issue in the ActionController filters query by refactoring the getAnAction() method to improve query evaluation efficiency. The change reduces tuple counts from ~526 million to ~141 thousand by introducing a helper method and applying pragma[nomagic] annotations to control query optimization.

Key changes:

  • Introduces a new helper method getAnActionCand() to separate candidate action retrieval from filtering logic
  • Adds pragma[nomagic] annotations to prevent premature query optimization
  • Refactors the logic to use not result = instead of result != for consistency with QL best practices

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@nickrolfe nickrolfe left a comment

Choose a reason for hiding this comment

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

Nice — LGTM.

@hvitved hvitved merged commit eca451e into github:main Jan 6, 2026
35 checks passed
@hvitved hvitved deleted the ruby/fix-bad-join branch January 6, 2026 12:44
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 Ruby

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants