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

Use a nested query to save a rountrip #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

casperisfine
Copy link

There might be a reason that escapes me, but I don't get why the gem is doing a full roundtrip to fetch the ids. Active Record supports nested queries just fine.

@mscoutermarsh
Copy link
Member

Thanks @casperisfine!

I have it separate because MySQL doesn't support subqueries with limit.

This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery'

The single query would be great though, maybe we could get AR to produce this:

SELECT * FROM posts 
INNER JOIN(select id from posts ORDER BY created_at DESC LIMIT 50 OFFSET 10000) 
AS lim USING(id);

Which would work with MySQL. I'll try playing with it later 😄

@pusewicz
Copy link

@mscoutermarsh Hey! Any update on getting this somehow merged? Maybe as an option? It seems to be working just fine. I'm wondering which version of MySQL were you talking about, when saying that the limit subquery does not work?

@mscoutermarsh
Copy link
Member

@pusewicz I tried it on 8.0.27 I believe.

I believe it's only an issue with 8+. Did you try with an older version?

@pusewicz
Copy link

@mscoutermarsh Correct! I've been running similar queries on 5.7.

@mscoutermarsh
Copy link
Member

Thanks! I also confirmed it works with MySQL 5.7. But not newer versions.

Here’s what I’m thinking. Since 5.7 is end-of-life this year, I’m not sure it’s worth doing the work of adding a config option. I personally haven’t benchmarked it though, maybe you all are seeing a great perf increase from the single query?

Might make sense to run a fork of the gem with just this change in it to see if it’s worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants