Skip to content
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.

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

Add explain support for methods like last, pluck and count #50482

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

p8
Copy link
Member

@p8 p8 commented Dec 29, 2023

Motivation / Background

explain can be called on a relation to explain how the database would execute a query.
Currently explain doesn't work for queries using last, pluck or count, as these return the actual result instead of a relation. This makes it difficult to optimize these queries.

By letting explain return a proxy instead, we can support these methods.

Detail

User.all.explain.count
# => "EXPLAIN SELECT COUNT(*) FROM `users`"

User.all.explain.maximum(:id)
# => "EXPLAIN SELECT MAX(`users`.`id`) FROM `users`"

This breaks the existing behaviour in that it requires calling inspect after explain.

User.all.explain.inspect
# => "EXPLAIN SELECT `users`.* FROM `users`"

However, as explain is mostly used from the commandline, this won't be a problem as inspect is called automatically in IRB.

Additional information

This is a variation of #50424 which used arguments instead of a proxy and method calls.

Another variation could be passing a block instead. This would not break the current behaviour, but it's a bit more verbose:

User.all.explain do |relation|
   relation.count
end

With a symbol-to-proc it becomes a bit terser:

User.all.explain(&:count)

Unlike the proxy solution, this notation could also be applied to to_sql:

User.all.to_sql(&:count)

Or we could use method name variations, but that would break with the arguments that can be passed to explain, like explain(:analyze, :buffers):

User.all.explain_count
User.all.explain_maximum(:id)

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@p8 p8 force-pushed the activerecord/explain-proxy branch 4 times, most recently from 2dfe3fe to b1d969f Compare December 29, 2023 15:47
@p8 p8 changed the title Add explain support for calculation methods like count Add explain support for methods like last, pluck and count Dec 29, 2023
@p8 p8 force-pushed the activerecord/explain-proxy branch 6 times, most recently from b18258e to c291af7 Compare January 2, 2024 21:25
activerecord/lib/active_record/relation.rb Show resolved Hide resolved
@@ -3,6 +3,37 @@
module ActiveRecord
# = Active Record \Relation
class Relation
class ExplainProxy # :nodoc:
SUPPORTED_METHODS = [:count, :first, :last, :maximum, :minimum, :pluck, :sum]
Copy link
Member

Choose a reason for hiding this comment

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

if we only support those methods why use method missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the implementation to use regular methods. This also makes sure the method signatures match.

@p8 p8 force-pushed the activerecord/explain-proxy branch 2 times, most recently from 0f43745 to 205c426 Compare January 4, 2024 09:36
`explain` can be called on a relation to explain how the database would
execute a query. Currently `explain` doesn't work for queries using
`last`, `pluck` or `count`, as these return the actual result instead of
a relation. This makes it difficult to optimize these queries.

By letting `explain` return a proxy instead, we can support these
methods.

```ruby
User.all.explain.count
\# => "EXPLAIN SELECT COUNT(*) FROM `users`"

User.all.explain.maximum(:id)
\# => "EXPLAIN SELECT MAX(`users`.`id`) FROM `users`"
```

This breaks the existing behaviour in that it requires calling inspect
after explain.

```ruby
User.all.explain.inspect
\# => "EXPLAIN SELECT `users`.* FROM `users`"
```

However, as `explain` is mostly used from the commandline, this won't be a
problem as inspect is called automatically in IRB.

Co-authored-by: Rafael Mendonça França <rafael@rubyonrails.org>
@p8 p8 force-pushed the activerecord/explain-proxy branch from 205c426 to 1f83af3 Compare January 4, 2024 10:11
@rafaelfranca rafaelfranca merged commit 89693e5 into rails:main Jan 4, 2024
4 checks passed
@p8 p8 deleted the activerecord/explain-proxy branch January 4, 2024 23:23
@fxn
Copy link
Member

fxn commented Jan 5, 2024

Is the first line in the CHANGELOG complete?

@p8
Copy link
Member Author

p8 commented Jan 5, 2024

@fxn Would you want to see all methods?
I think a previous version contained for methods like.

@fxn
Copy link
Member

fxn commented Jan 5, 2024

@p8 The current sentence is

Add explain support for last, pluck and count

As a reader, I'd interpret the support is for exactly those three.

Instead of "like", I believe it would be more informative to list them all. You read the CHANGELOG, and you know what is new without having to think more. See what I mean?

@p8
Copy link
Member Author

p8 commented Jan 5, 2024

@fxn I've created a PR to update the CHANGELOG: #50599

jonathanhefner added a commit that referenced this pull request Jan 7, 2024
Follow-up to #50482.

RDoc does not support backticks the way that Markdown does.  Instead,
inline code must be wrapped with `+` or `<tt>`.
aidanharan added a commit to rails-sqlserver/activerecord-sqlserver-adapter that referenced this pull request Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants