Skip to content

Has many through association with :order causes a SQL error with PostgreSQL #520

Closed
lancecarlson opened this Issue May 11, 2011 · 16 comments
@lancecarlson

I'm bringing back this old invalid ticket because I think it's relevant (as of rails 3.0.7 anyway):

https://rails.lighthouseapp.com/projects/8994/tickets/1711-has-many-through-association-with-order-causes-a-sql-error-with-postgresql

I made a comment at the bottom of the ticket justifying (I think) why it's still relevant:

I disagree with Jon. Since the assumption is that memberships should be unique (:uniq => true), the Person object should only have one membership and thus should only have one renewal_date to compare against. The intent is pretty explicit in this case and thus I feel AR can make the solid assumption that there won't be ambiguity amongst multiple membership objects.

I think this ticket should be re-opened and solved because I'm pretty sure this behavior works (magically) in MySQL with this exact syntax, so Postgres should behave the same.

The only work around I've found for this has been to eliminate the :uniq property and specify:

:select => 'DISTINCT people.*, memberships.renewal_date', :order => "memberships.renewal_date ASC"

which is not a full solution because with :uniq disabled, multiple memberships objects can get created.

Since my app is heavily JSON based, I've resorted to crafting the JSON instead of trying to hack AR. The end result is more queries, but I had to solve it somehow. :)

+1000 to testing this patch and pulling it in if it works!

Here is the original issue:

A has_many :through association with an :order option generates SQL which causes PostgreSQL (and possibly others) to report the error:

PGError: ERROR: for SELECT DISTINCT, ORDER BY expressions must appear in select list

Take the following example model:

class Group < ActiveRecord::Base
  has_many :memberships
  has_many :people, :through => :memberships, :order => "memberships.renewal_date", :uniq => true
end

I then load a fixture for some People and the Group with id=1. Using the console, I can run:

Group.find(1).people

Which results in the following SQL being generated:

SELECT DISTINCT "people".* FROM "people" INNER JOIN memberships ON people.id = memberships.person_id WHERE (("memberships".group_id = 1)) ORDER BY memberships.renewal_date

The "memberships.renewal_date" column does not appear in the SELECT field list and thus causes the error.

The attached diff appears to fix the issue. Whether or not it does so correctly, I leave to the experts.

Here is the diff just in case the lighthouse ticket dies.

--- has_many_through_association.rb.orig    2009-01-07 17:32:59.000000000 -0500
+++ has_many_through_association.rb 2009-01-07 17:32:40.000000000 -0500
@@ -135,5 +135,7 @@
         def construct_select(custom_select = nil)
           distinct = "DISTINCT " if @reflection.options[:uniq]
-          selected = custom_select || @reflection.options[:select] || "#{distinct}#{@reflection.quoted_table_name}.*"
+          selected = [custom_select || @reflection.options[:select] || "#{distinct}#{@reflection.quoted_table_name}.*"]
+          selected += (@reflection.options[:order] || "").scan(/([\.a-zA-Z_]+)/).flatten.reject {|e| e.match(/asc|desc/i)}
+          selected.join(", ")
         end
@jonleighton
Ruby on Rails member

For the record, my comment on this was the following:

It's not clear what the expected result should be, which is why postgres throws an error.

Due to the use of DISINCT, for any given Person in the output, there may be many different renewal_date values. Which is the correct one to use for the sort? The answer is ambiguous, hence why postgres gives an error.

I'm not in favour of Active Record trying to magically resolve this ambiguity by making an assumption, so I'd say we should close the ticket.

@tenderlove agreed with me in the original ticket.

@lancecarlson says:

Since the assumption is that memberships should be unique (:uniq => true), the Person object should only have one membership and thus should only have one renewal_date to compare against.

The :uniq => true does not indicate that the memberships should be unique, it indicates that the people should be unique. There could realistically be multiple memberships all referencing the same Person object.

It works under mysql because mysql chooses to make an assumption. I just don't want to bring a particular assumption into Rails itself.

If the model was defined like so:

class Group < ActiveRecord::Base
  has_many :memberships, :order => "memberships.renewal_date"
  has_many :people, :through => :memberships, :uniq => true
end

Then perhaps we could add the memberships.renewal_date to the select query because in this case you have explicitly defined an ordering for the join records. (Rather than ordering the people records by some attribute on the join table.) @tenderlove, WDYT?

I don't know if the above does anything useful on 3-0-stable or master - feel free to test and provide a patch!

@lancecarlson

@jonleighton thank you for filling in the gaps with this ticket feed! Are you suggesting that after you define the associations like you describe, that when you call .people, it would return the correct order with memberships.renewal_date added to the query? I think this makes sense for my use case and would def. solve my issue, though I wonder if it is confusing for some when they call .people and don't expect it to inherit the order of memberships. I frankly would expect it to, but you never know.

@jonleighton jonleighton was assigned May 12, 2011
@jgarber
jgarber commented Oct 7, 2011

I just ran into this issue in my own app, exactly as described above (that was handy for Googling!). I expected—it being Rails 3.1 :)—that the order on memberships would apply when I accessed the people collection. I exposed that it did not in my spec, so I added the order on people, which is when I got the PostgreSQL error above.

I would expect #people it to inherit the order of #memberships as well. If I wanted a different order for #people (last_name, say), I would specify it on the has_many :through, which I would expect to override the memberships order.

@mikeycgto

Just hit this "issue" as well. Was able to workaround it by supplying a :select option as recommended by this SO question:
http://stackoverflow.com/questions/1714736/postgresql-rails-and-order-problem

Kept the :uniq => true to ensure no duplicates were present when creating relations.

@justinhammack

Bumped into this as well. Rails 3.2.1

@ksz2k
ksz2k commented Feb 6, 2012

Same problem for me, on both - Rails 3.1.3 and 3.2.1

@AndrewRadev

I'm having the same issue on rails 3.2.1, but there's no :uniq => true option on my relation. I tried poking around a bit and found the following line in lib/active_record/associations/collection_association.rb, line 63:

relation.uniq.pluck(column)

This happens in the method ids_reader. Executing a uniq at this point makes perfect sense for ids, since you don't want duplicates under any situation (that I can think of). I think the problem is that uniq has been reimplemented for relations to generate a DISTINCT query, which wasn't expected at the time this code was written.

Moving the uniq after pluck(column) seems to fix my issue, but I don't know if it won't cause performance issues or if we can't do something cleverer here. I'll investigate this further a bit later and issue a pull request if I come up with something that passes tests. Unless someone fixes it in the meantime, of course :).

@jonleighton
Ruby on Rails member

@AndrewRadex: that issue is already fixed so it'll be in the next patch release.

@AndrewRadev

Ah, okay, then. Should've checked master as well.

@ouyangzhiping

Same problem, Rails3.2.1.

@jeffmcfadden

Same here, 3.2.1

@scomma
scomma commented Feb 27, 2012

Same here but with habtm where the foreign model has a default_scope.

@kennyj
kennyj commented Mar 4, 2012

@AndrewRadev
and Everyone.

Hi! Rails3.2.2 is released. Could you check it ?

@AndrewRadev

Yeah, seems to work just fine now, at least for me. Sorry for taking so long to check it.

@kennyj
kennyj commented Mar 8, 2012

Thank you for checking. I'm closing this issue.

@kennyj kennyj closed this Mar 8, 2012
@codez
codez commented Apr 2, 2012

This same Postgresql error also occurs if an order clause is defined as default_scope on the association mode (in combination with the :uniq option)l:

class Group < ActiveRecord::Base
  has_many :memberships
  has_many :people, :through => :memberships, :uniq => true
end

class Person < ActiveRecord::Base
  default_scope order(:lastname)
end
@jpdesigndev jpdesigndev referenced this issue in jpdesigndev/spree_asset_variant_options Jan 12, 2014
Closed

ORDER BY Expressions must appear in select list #4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.