Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

ActiveRecord::Base#pluck does not handle joins/includes correctly. #5990

Closed
ahawkins opened this Issue · 22 comments
@ahawkins

Generate a scope with joins or includes:

irb(main):028:0> Todo.includes(:user).where("users.id" => 1)
  SQL (1.0ms)  SELECT "todos"."id" AS t0_r0, "todos"."description" AS t0_r1, "todos"."user_id" AS t0_r2, "todos"."finish_by" AS t0_r3, "todos"."reference_id" AS t0_r4, "todos"."reference_type" AS t0_r5, "todos"."created_at" AS t0_r6, "todos"."updated_at" AS t0_r7, "todos"."kind" AS t0_r8, "todos"."account_id" AS t0_r9, "todos"."campaign_id" AS t0_r10, "todos"."finished" AS t0_r11, "todos"."call_list_id" AS t0_r12, "users"."id" AS t1_r0, "users"."name" AS t1_r1, "users"."email" AS t1_r2, "users"."phone" AS t1_r3, "users"."time_zone" AS t1_r4, "users"."account_id" AS t1_r5, "users"."created_at" AS t1_r6, "users"."updated_at" AS t1_r7, "users"."api_key" AS t1_r8, "users"."locale" AS t1_r9, "users"."public" AS t1_r10, "users"."provider" AS t1_r11, "users"."uid" AS t1_r12, "users"."avatar" AS t1_r13, "users"."admin" AS t1_r14 FROM "todos" LEFT OUTER JOIN "users" ON "users"."id" = "todos"."user_id" WHERE "users"."id" = 1
=> []

Now let's stay you want to pluck the ids from that scope

irb(main):031:0> Todo.includes(:user).where("users.id" => 1).pluck("todos.id")
   (0.5ms)  SELECT todos.id FROM "todos" WHERE "users"."id" = 1
ActiveRecord::StatementInvalid: PG::Error: ERROR:  missing FROM-clause entry for table "users"
LINE 1: SELECT todos.id FROM "todos"  WHERE "users"."id" = 1
                                            ^
: SELECT todos.id FROM "todos"  WHERE "users"."id" = 1

Seems there is an error with pluck (perhaps the postgres adapter) that joins and includes are not included when generating the SQL to select the column for pluck.

Also, using select(:id) seems to have no effect when using joins/includes

irb(main):001:0> Todo.includes(:user).where("users.id" => 1).select(:id)
  SQL (1.7ms)  SELECT "todos"."id" AS t0_r0, "todos"."description" AS t0_r1, "todos"."user_id" AS t0_r2, "todos"."finish_by" AS t0_r3, "todos"."reference_id" AS t0_r4, "todos"."reference_type" AS t0_r5, "todos"."created_at" AS t0_r6, "todos"."updated_at" AS t0_r7, "todos"."kind" AS t0_r8, "todos"."account_id" AS t0_r9, "todos"."campaign_id" AS t0_r10, "todos"."finished" AS t0_r11, "todos"."call_list_id" AS t0_r12, "users"."id" AS t1_r0, "users"."name" AS t1_r1, "users"."email" AS t1_r2, "users"."phone" AS t1_r3, "users"."time_zone" AS t1_r4, "users"."account_id" AS t1_r5, "users"."created_at" AS t1_r6, "users"."updated_at" AS t1_r7, "users"."api_key" AS t1_r8, "users"."locale" AS t1_r9, "users"."public" AS t1_r10, "users"."provider" AS t1_r11, "users"."uid" AS t1_r12, "users"."avatar" AS t1_r13, "users"."admin" AS t1_r14 FROM "todos" LEFT OUTER JOIN "users" ON "users"."id" = "todos"."user_id" WHERE "users"."id" = 1
=> []

Rails 3.2.3

@ahawkins

Seeems the has_many_association_ids method handles this case correctly: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/collection_association.rb#L54

Just need to get that logic into #pluck

def ids_reader
        if loaded? || options[:finder_sql]
          load_target.map do |record|
            record.send(reflection.association_primary_key)
          end
        else
          column  = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}"
          relation = scoped

          # voila, here is the good stuff!
          including = (relation.eager_load_values + relation.includes_values).uniq

          if including.any?
            join_dependency = ActiveRecord::Associations::JoinDependency.new(reflection.klass, including, [])
            relation = join_dependency.join_associations.inject(relation) do |r, association|
              association.join_relation(r)
            end
          end

          relation.pluck(column)
        end
      end
@ahawkins

@aderyabin can you make a PR for this?

@ahawkins ahawkins referenced this issue from a commit in ahawkins/rails
twinturbo Close #5990 bdedc7a
@ahawkins ahawkins referenced this issue from a commit in ahawkins/rails
twinturbo Close #5990 Against 3.2 stable b882a61
@rafaelfranca rafaelfranca referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@rafaelfranca rafaelfranca referenced this issue from a commit in rafaelfranca/omg-rails
@aderyabin aderyabin handle joins/includes correctly for pluck and calculation.
Fix #5990
1f98493
@contentfree

There seems to still be a bug with this. With Rails 3.2.8:

t1 = Thing.select("things.*, count(things.id) as thing_count")
t1.first.thing_count #=> works (outputs the count)

t2 = Thing.select("things.*, count(things.id) as thing_count")
          .joins(:related_model)
t2.first.thing_count #=> works (outputs the count)

t3 = Thing.select("things.*, count(things.id) as thing_count")
          .includes(:related_model)
t3.first.thing_count #=> works (outputs the count)

t4 = Thing.select("things.*, count(things.id) as thing_count")
          .joins(:related_model)
          .includes(:related_model)
t4.first.thing_count #=> NoMethodError: undefined method `thing_count' for #<Thing:0x00000106af6f48>

Ideas @aderyabin @twinturbo ? And do you know of a temporary workaround?

@steveklabnik steveklabnik reopened this
@steveklabnik
Collaborator

Yeah, from what I can see, 1f98493 never actually made it into the codebase, just in the possible-railties-fix branch.

@contentfree

My example above shouldn't trigger anything in Calculations though, right? Seems like the fix needs to go into other places, too.

@contentfree

Just another note that this worked fine in 3.0.9…

@joevandyk

Running into this also.

@bartiaco

This appears to still be an issue in 3.2.8.

@rafaelfranca

OMG! How @github closed this issue when I only pushed to my fork? O_O

Sorry for the mess

@rafaelfranca

Hey guys. Looking again to this issue I saw that I closed it at a7e715e only in master.

The fix is not trivial so I didn't backported to 3.2.8. If anyone want please try to backport it to 3-2-stable.

@bartiaco

I haven't done extensive testing yet, but I applied twinturbo's patch above and it appears to work

ahawkins@b882a61

@rafaelfranca

@bartiaco could you open a pull request with the @twinturbo fix and the a7e715e tests?

I'll put in my icebox but I don't have too much time right now.

@parndt parndt referenced this issue from a commit in parndt/rails
twinturbo Close #5990 Against 3.2 stable
Conflicts:
	activerecord/lib/active_record/relation/calculations.rb
	activerecord/test/cases/calculations_test.rb
a08069a
@parndt

@rafaelfranca is parndt@rails:3-2-stable...parndt:issue_5990 what's required for your request or do you also want a7e715e merged in?

@rafaelfranca

@parndt yes, but I want only the a7e715e tests (if it is possible)

@Papipo

Is this merged in the 3.2 branch? I have just been hit by it in 3.2.12, but only in production.

@parndt
@Papipo

I'll see if I can update my application and try.

@mariovisic

I have the same issue on both rails 3.2.13 as well as 4.0.0
To replicate you just find any active record query that generates an alias and add a select with an alias:

Model.select('1 as foo').order('foo') 
#=> results

complex_scope.select('1 as foo').order('foo') 
#=> PG::Error: ERROR:  column "foo" does not exist

# The SQL comes out broken like this:
SELECT foo AS alias_0,  ...... ORDER BY foo
@clemenst

+1

Can somebody reopen?

Rails 3.2.14 does not pass all tests with twinturbos patch.

@matthew-andrews

+1 Also struggling against this...

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.