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

3.x fix for MySQL adapter to prevent MySQL 'LIMIT & IN/ALL/ANY/SOME subquery'" error #457

Closed
wants to merge 3 commits into from

Conversation

dannguyen
Copy link

In regards to bug report 450:
#450

This pull contains a fix for a MySQL error generated by a combination of limit,tag_counts, andsum`

ActiveRecord::StatementInvalid: Mysql2::Error: This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery':

TaggableModel.new(:name => "Barb Jones").tap{|t| t.tag_list = ['awesome', 'fun'] }.save
TaggableModel.new(:name => "John Doe").tap{|t| t.tag_list = ['cool', 'fun', 'hella'] }.save
TaggableModel.new(:name => "Jo Doe").tap{|t| t.tag_list = ['curious', 'young', 'naive', 'sharp'] }.save
TaggableModel.all.each{|t| t.save }

expect( TaggableModel.limit(2).tag_counts.sum('tags_count') ).to eq 5

In other words, limit the model to the first 2 records, find tag counts for the 2 records, then sum those tag counts

MySQL generated 2.x

SELECT tags.*, taggings.tags_count AS count FROM "tags" JOIN (SELECT taggings.tag_id, COUNT(taggings.tag_id) 
  AS tags_count FROM "taggings" 
  INNER JOIN taggable_models ON taggable_models.id = taggings.taggable_id 
  WHERE (taggings.taggable_type = 'TaggableModel' 
    AND taggings.context = 'tags') 
    AND (taggings.taggable_id 

  IN(10,11)) 

  GROUP BY taggings.tag_id HAVING COUNT(taggings.tag_id) > 0) 
  AS taggings ON taggings.tag_id = tags.id

MySQL generated in 3.x

re: ActiveRecord::StatementInvalid: Mysql2::Error: This version of
MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery':

SELECT tags.*, taggings.tags_count AS count FROM `tags` JOIN (SELECT taggings.tag_id, COUNT(taggings.tag_id) 
  AS tags_count FROM `taggings` 
    INNER JOIN taggable_models ON taggable_models.id = taggings.taggable_id 
    WHERE (taggings.taggable_type = 'TaggableModel' 
      AND taggings.context = 'tags') 
      AND (taggings.taggable_id 


    IN(SELECT  taggable_models.id FROM `taggable_models`  LIMIT 2)) 

    GROUP BY taggings.tag_id HAVING COUNT(taggings.tag_id) > 0) 
    AS taggings ON taggings.tag_id = tags.id

Solution: Add a ActsAsTaggableOn::Tag.using_mysql? method and then revert to the 2.x way of
doing a group count in .all_tags and .all_tags_count

Also, collection.rb now has a private class method to switch between the 2.x and 3.x code. I've tentatively referred to it as .generate_tagging_scope_in_clause until a better patch is put in place. I say "better" as in, this patch fixes the broken behavior and passes all the specs, but if there were performance reasons for the breaking behavior, this patch reverts that as it basically does the counting query for Mysql as it did back in acts-as-taggable 2.x

@@ -15,6 +15,11 @@ def using_sqlite?
::ActiveRecord::Base.connection && ::ActiveRecord::Base.connection.adapter_name == 'SQLite'
end

def using_mysql?
::ActiveRecord::Base.connection && ::ActiveRecord::Base.connection.adapter_name =~ /Mysql/
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you could use this in Tag, that would be great https://github.com/mbleigh/acts-as-taggable-on/blob/master/lib/acts_as_taggable_on/tag.rb#L110

Any reason you didn't match case insensitive or some other way?

@bf4
Copy link
Collaborator

bf4 commented Jan 13, 2014

bump

@dannguyen
Copy link
Author

Er yeah...no reason for my specific implementation, just general ignorance of what enumerated strings ActiveRecord might allow for adapter_name :)

Let me make sure I understand what you want before I submit the pull request. Basically,just wrap binary?

def using_mysql?
     !binary.nil?
end

@seuros
Copy link
Collaborator

seuros commented Mar 17, 2014

Can you rebase ?

This pull request was closed.
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.

3 participants