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

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

Open
jeremyhaile opened this issue Oct 27, 2023 · 2 comments

Comments

@jeremyhaile
Copy link

jeremyhaile commented Oct 27, 2023

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
Copy link
Author

This is a different issue, but is related to this issue: #106

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

@fatkodima
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants