#120 ✓resolved
wildchild

Association with finder_sql raises TypeError

Reported by wildchild | September 29th, 2007 @ 05:27 AM

In case when finder_sql || counter_sql is used, attempt to paginate association raises TypeError: can't convert nil into Array

Attached failing test case.

Comments and changes to this ticket

  • wildchild

    wildchild September 29th, 2007 @ 05:31 AM

    • Assigned user changed from “Chris Wanstrath” to “Mislav”
  • Mislav

    Mislav September 29th, 2007 @ 11:09 AM

    • State changed from “new” to “open”
    • Title changed from “Paginate association with finder_sql raises TypeError” to “Association with finder_sql raises TypeError”

    Damn. Gotta investigate how this should/could work. Thanks for the report!

  • Mislav

    Mislav October 7th, 2007 @ 09:04 PM

    • State changed from “open” to “invalid”

    I've given it a go, but it seems too hard to support right now. Fixing ActiveRecord would be involved and I'm not sure if I should do it.

    So, if you have manual finders and counters for your associations you'll also have to do the pagination manually. WillPaginate::Collection can help you with calculating limits and offsets.

  • Dmitri Zhuchkov

    Dmitri Zhuchkov December 31st, 2007 @ 03:18 PM

    I hit the same problem today. It seems I was able to work around the problem by something like this:

    class Post < ActiveRecord::Base
      has_many :comments,
               :finder_sql => 'SELECT * FROM comments WHERE comments.post_id = #{id}'
               :counter_sql => 'SELECT COUNT(*) FROM comments WHERE comments.post_id = #{id}' do
        def find(*args)
          options = args.extract_options!
          sql = @finder_sql
          
          sql += sanitize_sql [" LIMIT ?", options[:limit]] if options[:limit]
          sql += sanitize_sql [" OFFSET ?", options[:offset]] if options[:offset]
          
          Comment.find_by_sql(sql)
        end
      end
    end
    

    When you make a call to

    @post.comments.paginate(:page => params[:page], :per_page => 10)
    

    then custom find method will be called which will update the finder SQL by LIMIT and OFFSET and perform the find_by_sql.

  • Adam Tretkowski

    Adam Tretkowski November 11th, 2008 @ 11:14 AM

    • Tag set to will_paginate

    In my model only some relationships use finder_sql so I modified a bit your solution to make possible to chose which finder should be used and to make it accessible for all models.

    In the plugin's finder.rb file I introduced 'custom_sql' option to indicate that find_by_sql should be used

    
       WillPaginate::Collection.create(page, per_page, total_entries) do |pager|
              count_options = options.except :page, :per_page, :total_entries, :finder
              find_options = count_options.except(:count).update(:offset => pager.offset, :limit => pager.per_page)
    
              if options[:custom_sql].nil?
                args << find_options
                # @options_from_last_find = nil
    
                pager.replace send(finder, *args, &block)
              else
                sql = @finder_sql
                sql += sanitize_sql [" LIMIT ?", pager.per_page]
                sql += sanitize_sql [" OFFSET ?", pager.offset]
    
                pager.replace send("find_by_sql", sql, &block)
              end
    
              # magic counting for user convenience:
              pager.total_entries = wp_count(count_options, args, finder) unless pager.total_entries
            end
    

    Probably it is not perfect but works for me.

    @mails = @user.inbox.paginate :page => page, :order => "sent_at DESC", :per_page => 10, :custom_sql => true

  • Eric Mill

    Eric Mill February 5th, 2009 @ 07:33 PM

    Dimitri and Adam - those are awesome solutions, thank you. I was in the same situation.

    Mislav, this would be an awesome thing to build into WillPaginate.

  • Cameron Booth

    Cameron Booth August 19th, 2009 @ 01:42 AM

    I might have found a cleaner solution, build off of what you've got Adam, combined with a few other hints. This appears to be working, though I have only given it cursory tests.

    I took what you did Adam, except instead of passing in ":custom_sql => true", I simply replaced the "if options[:custom_sql].nil?" with "if proxy_reflection.options[:finder_sql].nil?"

    Like I said, seems to be working so far, I'll report back if I find any issues with it

  • John Wood

    John Wood August 27th, 2009 @ 04:38 PM

    Hi everybody. Thanks for providing a solution to this issue.

    I took the code you guys put together here, committed it to my will_paginate fork, and submitted a pull request. My fork can be found at http://github.com/jwood/will_paginate/tree/master, in case you're interested. It uses Cameron's version with one small modification, checking to see if the object responds to proxy_reflection before calling it (since it's Rails only).

  • wildchild

    wildchild January 13th, 2010 @ 05:26 PM

    Awesome, John.

    Any status update?

  • Kandada Boggu

    Kandada Boggu April 8th, 2011 @ 01:18 AM

    • Assigned user cleared.
    • Milestone order changed from “0” to “0”

    I tried John's solution, and the exception went away. I noticed that total_entries didn't return the expected results. I have modified the fix to address this issue.

    module WillPaginate
      module Finder
        module ClassMethods
          def paginate_with_finder_sql(*args)
            if respond_to?(:proxy_reflection) && !proxy_reflection.options[:finder_sql].nil?
              # note: paginate_by_sql ignores the blocks. So don't pass the block
              paginate_by_sql(@finder_sql, args.extract_options!) 
            else
              paginate_without_finder_sql(*args)
            end
          end
          # patch to deal with the custom_sql scenario
          alias_method_chain :paginate, :finder_sql
        end
      end
    end
    

    PS: It looks like this issue is fixed in will_paginate 3.x.pre. I can't move to the latest version of as I faced the issue on a live system. So I had to resort to monkey patching the gem.

  • Mislav

    Mislav September 27th, 2011 @ 03:45 PM

    • State changed from “invalid” to “resolved”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

Everyone's favorite Ruby library for pagination of practically anything!

Attachments

Referenced by

Pages