1

I am building a Rails app with the following models:

# vote.rb
class Vote < ApplicationRecord
  belongs_to :person
  belongs_to :show
  scope :fulfilled, -> { where(fulfilled: true) }
  scope :unfulfilled, -> { where(fulfilled: false) }
end

# person.rb
class Person < ApplicationRecord
  has_many :votes, dependent: :destroy

  def self.order_by_votes(show = nil)
    count = 'nullif(votes.fulfilled, true)'
    count = "case when votes.show_id = #{show.id} AND NOT votes.fulfilled then 1 else null end" if show
    people = left_joins(:votes).group(:id).uniq!(:group)
    people = people.select("people.*, COUNT(#{count}) AS people.vote_count")
    people.order('people.vote_count DESC')
  end
end

The idea behind order_by_votes is to sort People by the number of unfulfilled votes, either counting all votes, or counting only votes associated with a given Show.

This seem to work fine when I test against SQLite. But when I switch to Postgres I get this error:

Error:
PeopleControllerIndexTest#test_should_get_previously_on_show:
ActiveRecord::StatementInvalid: PG::UndefinedColumn: ERROR:  column people.vote_count does not exist
LINE 1: ...s"."show_id" = $1 GROUP BY "people"."id" ORDER BY people.vot...
                                                             ^

If I dump the SQL using @people.to_sql, this is what I get:

SELECT people.*, COUNT(nullif(votes.fulfilled, true)) AS people.vote_count FROM "people" LEFT OUTER JOIN "votes" ON "votes"."person_id" = "people"."id" GROUP BY "people"."id" ORDER BY people.vote_count DESC

Why is this failing on Postgres but working on SQLite? And what should I be doing instead to make it work on Postgres?

(PS: I named the field people.vote_count, with a dot, so I can access it in my view without having to do another SQL query to actually view the vote count for each person in the view (not sure if this works) but I get the same error even if I name the field simply vote_count.)

(PS2: I recently added the .uniq!(:group) because of some deprecation warning for Rails 6.2, but I couldn't find any documentation for it so I am not sure I am doing it right, still the error is there without that part.)

mu is too short
  • 426,620
  • 70
  • 833
  • 800
Christoffer Reijer
  • 1,925
  • 2
  • 21
  • 40
  • The column not existing could be a symptom of a migration not having run (or a case where you edited the migration to add the column after running it). The fact you're switching between database engines makes it likely that something simply got mixed up. If you did edit a migration, run a db:rollback and then db:migrate again. – Robert Nubel Sep 20 '21 at 19:43
  • No, there is no migration. The column does not exist in the database, it is created in the SQL statement: `SELECT ... AS people.vote_count`. SQLite accepts this, but not Postgres. :/ – Christoffer Reijer Sep 20 '21 at 20:54
  • Oh sorry, I didn't read closely enough. Like Mu mentioned below the alias name in Postgres has to be just a column name, but the reason it doesn't help is because Pagy is [blowing away](https://github.com/ddnexus/pagy/blob/master/lib/pagy/backend.rb#L23) all of your SELECT clause when it calculates the collection size. That nukes the alias so Postgres gets confused. As a quick fix, I would try `people.order("COUNT(#{count}) DESC")` to avoid using the alias. – Robert Nubel Sep 21 '21 at 04:04
  • Yes, I have pinned it down to appending `count(:all)` at the end. If I try the more elaborate count (count only unfulfilled votes, or count only votes for a given show), then I get `ActiveRecord::UnknownAttributeReference`. I have unfortunately had to scale the ambition down to just doing `left_joins(:votes).group(:id).uniq!(:group).order('COUNT(votes.id)')`. Maybe later I can find out how to do proper sorting on different kind of votes. :/ – Christoffer Reijer Sep 21 '21 at 17:35
  • Robert, Pagy is not "blowing away" anything. It's calling the `count(:all)` on whatever scope you pass to it. Pretty standard way to get the count of a collection. However, if the scope blows up when you try to count it, then... there is definitely a problem with the scope... and that has nothing to do with Pagy. – user712557 Sep 23 '21 at 00:07

3 Answers3

1

Are you sure you're not getting a syntax error from PostgreSQL somewhere? If you do something like this:

select count(*) as t.vote_count from t ... order by t.vote_count

I get a syntax error before PostgreSQL gets to complain about there being no t.vote_count column.

No matter, the solution is to not try to put your vote_count in the people table:

people = people.select("people.*, COUNT(#{count}) AS vote_count")
...
people.order(vote_count: :desc)

You don't need it there, you'll still be able to reference the vote_count just like any "normal" column in people. Anything in the select list will appear as an accessor in the resultant model instances whether they're columns or not, they won't show up in the #inspect output (since that's generated based on the table's columns) but you call the accessor methods nonetheless.

mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • I removed the `people.` part but I still get the error. I have tracked down the error to Pagy. It seems to appear when I call `pagy(@people, ...)` to paginate my collection. If I just inspect the collection as it is, it looks fine. – Christoffer Reijer Sep 20 '21 at 22:10
  • Sounds like you've discovered a bug in Pagy. Sorry, I don't know anything about Pagy so I can't say much. – mu is too short Sep 20 '21 at 22:31
1

Historically there have been quite a few AR problems (and bugs) in getting the right count by just using count on a scope, and I am not sure they are actually all gone.

That depends on the scope (AR version, relations, group, sort, uniq, etc). A defaut count call that a gem has to generically use on a scope is not a one-fit-all solution. For that known reason Pagy allows you to pass the right count to its pagy method as explained in the Pagy documentation.

Your scope might become complex and the default pagy collection.count(:all) may not get the actual count. In that case you can get the right count with some custom statement, and pass it to pagy.

@pagy, @records = pagy(collection, count: your_count)

Notice: pagy will efficiently skip its internal count query and will just use the passed :count variable.

So... just get your own calculated count and pass it to pagy, and it will not even try to use the default.

EDIT: I forgot to mention: you may want to try the pagy arel extra that:

adds specialized pagination for collections from sql databases with GROUP BY clauses, by computing the total number of results with COUNT(*) OVER ().

user712557
  • 327
  • 2
  • 5
  • Thank you, that is really helpful. Upvoted. But I think I have a way to successfully make my scope, exactly how I want it according to the question, in a way that is compatible with Pagy. If I succeed I'll post that as the answer to the question, but this is also good. – Christoffer Reijer Sep 22 '21 at 11:11
1

Thanks to all the comments and answers I have finally found a solution which I think is the best way to solve this.

First of, the issue occurred when I called pagy which tried to count my scope by appending .count(:all). This is what caused the errors. The solution was to not create a "field" in select() and use it in .order().

So here is the proper code:

def self.order_by_votes(show = nil)
  count = if show
            "case when votes.show_id = #{show.id} AND NOT votes.fulfilled then 1 else null end"
          else
            'nullif(votes.fulfilled, true)'
          end
  left_joins(:votes).group(:id)
                    .uniq!(:group)
                    .select("people.*, COUNT(#{count}) as vote_count")
                    .order(Arel.sql("COUNT(#{count}) DESC"))
end

This sorts the number of people on the number of unfulfilled votes for them, with the ability to count only votes for a given show, and it works with pagy(), and pagy_arel() which in my case is a much better fit, so the results can be properly paginated.

Christoffer Reijer
  • 1,925
  • 2
  • 21
  • 40