Fixes #733 by refactoring the tagged query #744
Conversation
Also adds a simple spec for .tagging.
There was a problem hiding this comment.
Looks good, and thank you for adding a test!
I'm guessing the sql for this will probably look something like:
select distinct s.title
from stories s
join taggings t
on s.id = t.story_id
where s.score >= 0
and t.tag_id in (1, 2)But another way of writing the above would be:
select s.title
from stories s
where s.score >= 0
and s.id in (select story_id from taggings where tag_id in (1, 2))Do you know how the performance comparison would look like?
|
I'm thrilled to see the spec, thanks especially for adding a test. And to add on to @thomasdziedzic's comment, I've slowly learned to be suspicious when I see |
|
Hey, I didn't do any irl benchmarking, but here's a quick explain of both queries (the current query generated via I haven't used mysql in a minute so don't really have an intuition on which is better from eyeballing. The subquery one is more elegant in any case, so here it is! |
lachie
changed the title
|
Yep, they're basically equivalent... except the big difference is that This looks like it'll execute two queries, so there's an extra round-trip to the server. Can you express this in one, perhaps with ActiveRecord#merge? |
|
Hi, # assuming submitter, tag1, tag2
repo = StoryRepository.new(submitter)
tagged = repo.tagged([tag1, tag2])
puts tagged.to_sql
#=>SELECT `stories`.* FROM `stories`
WHERE `stories`.`merged_story_id` IS NULL
AND `stories`.`is_expired` = FALSE
AND ((CAST(upvotes AS signed) - CAST(downvotes AS signed)) >= 0)
AND `stories`.`id` IN (SELECT `taggings`.`story_id` FROM `taggings` WHERE `taggings`.`tag_id` IN (3, 4))
ORDER BY `stories`.`created_at` DESCSorry, is that what you meant by one query? |
|
Huh. Yeah, that's exactly what I meant, and I pulled your branch locally to test it out. I'm surprised that the Well, wonderful! Merging. Thanks so much for running this all the way down with a test. |
Also adds a simple spec for
.tagging.