will_paginate icon indicating copy to clipboard operation
will_paginate copied to clipboard

Using will_paginate HABTM (Bookmark HasMany Tags through Taggings) results in 3 queries, while the 2nd has an issue

Open ionas opened this issue 14 years ago • 1 comments

Hello,

Using will_paginate on a HABTM association (Bookmark HasMany Tags through Taggings) results in 3 queries, while the 2nd has following issue:

def self.search(search, page, order_by, order_direction, per_page)
  # Treat spaces and commas and semicolons as wildcards
  search = search.gsub(' ', '%').gsub(',', '%').gsub(';', '%') unless search == nil
  # BUG: Gets only tag names that match
  paginate :per_page => per_page, :page => page,
           :conditions => ['bookmarks.url LIKE ? OR tags.name LIKE ?', "%#{search}%", "%#{search}%"],
           :order => 'bookmarks.' + order_by + ' ' + order_direction,
           :include => [:taggings, :tags]
end

The issue is if /search/ matches a part of an Bookmark.url (or part of it) the returned records will hold any Tag.names that get across the HABTM relation BUT if /search/ matches a Tag.name (or part of it) only that Tag.name(s) that match will be returned.

These are the 3 queries that get executed:

Bookmark Load (6.9ms)
SELECT DISTINCT `bookmarks`.id FROM `bookmarks` LEFT OUTER JOIN `taggings` ON `taggings`.`bookmark_id` = `bookmarks`.`id` LEFT OUTER JOIN `taggings` `tags_bookmarks_join` ON `bookmarks`.`id` = `tags_bookmarks_join`.`bookmark_id` LEFT OUTER JOIN `tags` ON `tags`.`id` = `tags_bookmarks_join`.`tag_id` WHERE (bookmarks.url LIKE '%rails%' OR tags.name LIKE '%rails%') ORDER BY bookmarks.updated_at desc LIMIT 0, 3

Bookmark Load (3.0ms)
SELECT `bookmarks`.`id` AS t0_r0, `bookmarks`.`url` AS t0_r1, `bookmarks`.`created_at` AS t0_r2, `bookmarks`.`updated_at` AS t0_r3, `taggings`.`id` AS t1_r0, `taggings`.`bookmark_id` AS t1_r1, `taggings`.`tag_id` AS t1_r2, `taggings`.`created_at` AS t1_r3, `taggings`.`updated_at` AS t1_r4, `tags`.`id` AS t2_r0, `tags`.`name` AS t2_r1, `tags`.`created_at` AS t2_r2, `tags`.`updated_at` AS t2_r3 FROM `bookmarks` LEFT OUTER JOIN `taggings` ON `taggings`.`bookmark_id` = `bookmarks`.`id` LEFT OUTER JOIN `taggings` `tags_bookmarks_join` ON `bookmarks`.`id` = `tags_bookmarks_join`.`bookmark_id` LEFT OUTER JOIN `tags` ON `tags`.`id` = `tags_bookmarks_join`.`tag_id` WHERE (bookmarks.url LIKE '%rails%' OR tags.name LIKE '%rails%') AND (`bookmarks`.`id` IN (24, 4, 12)) ORDER BY bookmarks.updated_at desc

SQL (9.0ms)
SELECT COUNT(DISTINCT `bookmarks`.`id`) AS count_id FROM `bookmarks` LEFT OUTER JOIN `taggings` ON `taggings`.`bookmark_id` = `bookmarks`.`id` LEFT OUTER JOIN `taggings` `tags_bookmarks_join` ON `bookmarks`.`id` = `tags_bookmarks_join`.`bookmark_id` LEFT OUTER JOIN `tags` ON `tags`.`id` = `tags_bookmarks_join`.`tag_id` WHERE (bookmarks.url LIKE '%rails%' OR tags.name LIKE '%rails%')
  • The first query seems to get all the IDs of the "primary" model (e.g. Bookmark.ids). It uses the :condition option
  • The second query uses WHERE ... Bookmark.id IN (some Bookmark.ids) to filter the Bookmarks. But it also applys :condition AGAIN
  • The third query is a count query that gets the amount of Bookmarks. (No Idea if its really required, cause Bookmark.ids could be counted)

The reason for that behavior is here: WHERE (bookmarks.url LIKE '%rails%' OR tags.name LIKE '%rails%') AND (bookmarks.id IN (24, 4, 12))

It would be complete suffient to have (bookmarks.id IN (24, 4, 12)) for the SELECTION of the data records. There is as far as I understand, no reason to apply :condition AGAIN to the data fetching query.

I am not sure if its an issue with will_paginate, but I am not sure either why the 2nd query is build the right way.

ionas avatar Sep 29 '10 12:09 ionas

While I still think what I described is bug resulting from an unnecessary condition in the second query, this is the solution I came up with:

class Bookmark < ActiveRecord::Base
    has_many :taggings, :dependent => :destroy
    has_many :tags, :through => :taggings

# ...

def self.search(search, page, order_by, order_direction, per_page)
  # Treat spaces and commas and semicolons as wildcards
  search = search.gsub(' ', '%').gsub(',', '%').gsub(';', '%') unless search == nil
  # ^ Better: LIKE %PART1% OR %PART2% / Better yet: weighted?!
  if search == '' # If its empty, we can use a simple find: LIKE "%%" not good.
    paginate :per_page => per_page, :page => page,
      :order => order_by + ' ' + order_direction,
      :include => [:taggings, :tags]
  else 
    # Also: order_by and order_direction require SQL injection protection, in before
    paginate_by_sql ['
      SELECT bookmarks.id, bookmarks.url, bookmarks.updated_at, tags.name
      FROM bookmarks
      LEFT OUTER JOIN taggings ON taggings.bookmark_id = bookmarks.id
      LEFT OUTER JOIN tags ON tags.id = taggings.tag_id
      LEFT OUTER JOIN taggings AS taggings_join ON bookmarks.id = taggings_join.bookmark_id
      LEFT OUTER JOIN tags AS tags_join ON taggings_join.tag_id = tags_join.id
      WHERE (
        bookmarks.url LIKE ? # by url without tags
        OR ( # by url with tags
          bookmarks.url LIKE ?
          AND bookmarks.id = taggings.bookmark_id
          AND taggings.tag_id = tags.id
          AND tags.id = tags_join.id
        )
      ) OR ( # by tags
      tags_join.name LIKE ?
        AND tags_join.id = taggings_join.tag_id
        AND taggings_join.bookmark_id = bookmarks.id
        AND bookmarks.id = taggings.bookmark_id
        AND taggings.tag_id = tags.id
      )
      GROUP BY bookmarks.id
      ORDER BY ' + order_by + ' ' + order_direction,
      "%#{search}%", "%#{search}%", "%#{search}%"], :page => page, :per_page => per_page
  end
end

ionas avatar Sep 29 '10 13:09 ionas