rails_cursor_pagination icon indicating copy to clipboard operation
rails_cursor_pagination copied to clipboard

Cannot sort by a column on a joined table due to adding selected columns that don't exist

Open jeremyhaile opened this issue 2 years ago • 2 comments
trafficstars

I want to paginate a joined table and sort on a column in the join table:

scope = Pizza.joins(:pizza_users).order("pizza_users.preference")

However, if you do this:

RailsCursorPagination::Paginator.new(
  scope,
  order_by: 'pizza_users.preference',
  order: :asc
)

It will fail because order_field_value doesn't exist because it can't call pizza['pizza_users.preference'] since preference is on the join table.

Ah hah - I know the solution, I'll just select the preference column so that pizza['preference'] will work, like so:

scope = Pizza.joins(:pizza_users).select("pizzas.*, pizza_users.preference AS preference")

RailsCursorPagination::Paginator.new(
  scope,
  order_by: 'preference',
  order: :asc
)

But that will fail with:

ERROR:  column "preference" does not exist

However, it's not failing due to my select clause above. Instead, it's a problem with this code in rails_cursor_pagination/paginator.rb:

  if custom_order_field? && [email protected]_values.include?(@order_field)
    relation = relation.select(@order_field)
  end

which results in this SQL being generated:

SELECT pizzas.*, pizza_users.preference AS preference, preference 
FROM "pizzas" INNER JOIN "pizza_users" ON "pizza_users"."pizza_id" = "pizzas"."id" 
ORDER BY "preference" ASC, "pizzas"."id" ASC LIMIT $2

Even though I already had "preference" defined using "AS" - the code above @relation.select_values.include?(@order_field) is doing an overly simple analysis of selected fields and then appends a non-existent field to my list of fields.

How to solve? Here are my thoughts:

  1. Honestly I don't feel like rails_cursor_pagination should be modifying my select clause at all. I'd rather have an exception raised and fix my query to select the appropriate column. This is my preferred solution b/c there are so many possible ways to muck up queries, especially complex ones. But, this is a potentially breaking change for people who depend on that behavior.
  2. You could improve the logic of deciding whether a selected column is already in the query. Perhaps if that column name text appears anywhere in the select clause, it shouldn't be appended. Or at a minimum it should understand that something AS column_name means that column_name is selected.
  3. You could allow disabling this behavior via a preference flag. You could even make this optional initially, but switch to the default in the future with a breaking change release warning. (if you agree with my thoughts in (1))

Either way, without a bugfix/code modification I can't think of any way to workaround this issue.

jeremyhaile avatar Oct 27 '23 17:10 jeremyhaile

This is a different issue, but is related to this issue: https://github.com/xing/rails_cursor_pagination/issues/106

I do think my suggested solutions above would fix both of these (i.e. don't muck with the select query!)

jeremyhaile avatar Oct 27 '23 17:10 jeremyhaile

You might find this gem helpful - https://github.com/fatkodima/activerecord_cursor_paginate I created it because I wasn't able to find solutions to this and a few other problems in existing gems.

fatkodima avatar Mar 08 '24 13:03 fatkodima