Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Id is inproperly escaped on postgresql statements when primary key is a string

#6036 
ruby-1.9.2-head > Unit.first.subunits
[
    [0] #<Unit:0xbab77b8> {
                 :id => "00400003",
               :type => "Unit"
    }
]


ruby-1.9.2-head > u = Unit.first.subunits.first

#<Unit:0xba96b80> {
             :id => "00400003",
           :type => "Unit"
}


# Works fine. Generated SQL statement:
# Unit Load (1.2ms)  SELECT "business_objects".* FROM "business_objects" INNER JOIN "relations" ON "business_objects".id = "relations".child_id WHERE ("business_objects"."type" = 'Unit') AND (("relations".parent_id = '00400002') AND (("relations"."type" = 'Containment'))) AND ("business_objects"."id" = '00400003')

ruby-1.9.2-head > Unit.first.subunits.where(:id => u.id)
[#<Unit id: "00400003", type: "Unit>]


# Doesn't work. 
# Generated SQL statment:
# Unit Load (1.2ms)  SELECT "business_objects".* FROM "business_objects" INNER JOIN "relations" 
ON "business_objects".id = "relations".child_id WHERE ("business_objects"."type" = 'Unit') 
AND (("relations".parent_id = '00400002') AND (("relations"."type" = 'Containment'))) 
AND ("business_objects"."id" = '''00400003''')

# Notice this part of SQL statment at the end:  ("business_objects"."id" = '''00400003''')
ruby-1.9.2-head > Unit.first.subunits.where(:id => u)
[]

Such queries works fine when primary key is of integer type.

Reported by Robert Pankowecki · November 22nd, 2010 @ 04:05 PM

State: committed
Milestone: 3.x
Assigned to: Aaron Patterson Aaron Patterson
Importance: Low

Activity

  1. Neeraj Singh
    Neeraj Singh
    • Importance changed from to Low

    Which version of rails you are using. I am not able to reproduce this problem with rails edge.

    November 22nd, 2010 @ 09:36 PM

  2. Robert Pankowecki
    Robert Pankowecki

    Gem 3.0.3

    November 22nd, 2010 @ 09:58 PM

  3. Neeraj Singh
    Neeraj Singh

    With rails edge following is working for me.

       # has_many association
    create_table :cars, :id => false, :force => true do |t|
      t.string :id
      t.string :name
      t.string :color
    end
    create_table :brakes, :id => false, :force => true do |t|
      t.string :id
      t.string :name
      t.string :car_id
    end


   Car.last.brakes.where(:id => Brake.last.id)
   #=> SELECT "brakes".* FROM "brakes" WHERE "brakes"."id" = 'brake_01' AND ("brakes".car_id = 'car_01')
 => [#<Brake id: "brake_01", name: "b1", car_id: "car_01">]

    Can you try your app with rails edge?

    November 22nd, 2010 @ 10:17 PM

  4. Robert Pankowecki
    Robert Pankowecki

    This bug says that:

    Car.last.brakes.where(:id => Brake.last)
    will lead to wrong query, not:
    Car.last.brakes.where(:id => Brake.last.id)

    Is that what you checked?

    I tried your example in 3.0-stable branch. Still failing in the same way:

    ruby-1.9.2-head > c.brakes.where(:id => Brake.last.id)
 => [#<Brake id: "B", name: nil, car_id: "a">] 
ruby-1.9.2-head > c.brakes.where(:id => Brake.last)
 => [] 

Brake Load (0.5ms)  SELECT "brakes".* FROM "brakes" WHERE "brakes"."id" = 'B' AND ("brakes".car_id = 'a')
Brake Load (0.7ms)  SELECT "brakes".* FROM "brakes" ORDER BY brakes.id DESC LIMIT 1
Brake Load (0.5ms)  SELECT "brakes".* FROM "brakes" WHERE "brakes"."id" = '''B''' AND ("brakes".car_id = 'a')

    I could not try to reproduce it on master due to:

    uninitialized constant Rack::Session::Abstract::SessionHash
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb:23:in &lt;module:Session&gt;'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb:8:in<module:ActionDispatch>'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb:7:in &lt;top (required)&gt;'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb:3:in<top (required)>'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application/configuration.rb:126:in const_get'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application/configuration.rb:126:insession_store'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application.rb:158:in block in default_middleware_stack'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application.rb:142:intap'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application.rb:142:in default_middleware_stack'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/engine.rb:399:inapp'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application/finisher.rb:37:in block in &lt;module:Finisher&gt;'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/initializable.rb:25:ininstance_exec'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/initializable.rb:25:in run'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/initializable.rb:50:inblock in run_initializers'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/initializable.rb:49:in each'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/initializable.rb:49:inrun_initializers'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application.rb:93:in initialize!'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/railtie/configurable.rb:30:inmethod_missing'
/home/rupert/test/r3psql/config/environment.rb:6:in &lt;top (required)&gt;'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application.rb:75:inrequire'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application.rb:75:in require_environment!'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bundler/gems/rails-da583df50c32/railties/lib/rails/application.rb:176:inblock in initialize_tasks'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:636:in call'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:636:inblock in execute'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:631:in each'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:631:inexecute'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:597:in block in invoke_with_call_chain'
/home/rupert/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/monitor.rb:201:inmon_synchronize'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:590:in invoke_with_call_chain'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:607:inblock in invoke_prerequisites'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:604:in each'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:604:ininvoke_prerequisites'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:596:in block in invoke_with_call_chain'
/home/rupert/.rvm/rubies/ruby-1.9.2-head/lib/ruby/1.9.1/monitor.rb:201:inmon_synchronize'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:590:in invoke_with_call_chain'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:583:ininvoke'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:2051:in invoke_task'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:2029:inblock (2 levels) in top_level'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:2029:in each'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:2029:inblock in top_level'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:2068:in standard_exception_handling'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:2023:intop_level'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:2001:in block in run'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:2068:instandard_exception_handling'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/lib/rake.rb:1998:in run'
/home/rupert/.rvm/gems/ruby-1.9.2-head/gems/rake-0.8.7/bin/rake:31:in<top (required)>'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bin/rake:19:in load'
/home/rupert/.rvm/gems/ruby-1.9.2-head/bin/rake:19:in<main>'

    November 23rd, 2010 @ 04:46 PM

  5. Neeraj Singh
    Neeraj Singh
    • State changed from new to open
    • Milestone set to 3.x

    You are right with just Brake and not Brake.id I am getting following sql in rails edge.

    SELECT "brakes".* FROM "brakes" WHERE "brakes"."id" = '''brake_01''' AND ("brakes".car_id = 'car_01')

    rails edge requires rack edge.

    I created a tool for myself to check code against rails edge. Give it a try https://github.com/neerajdotname/rails_lighthouse .

    November 23rd, 2010 @ 04:57 PM

  6. Robert Pankowecki
    Robert Pankowecki

    Nice tool. I will use it next time. I'm glad that you were able confirm this bug.

    November 23rd, 2010 @ 05:12 PM

  7. Neeraj Singh
    Neeraj Singh
    • Assigned user set to Aaron Patterson

    Attached is patch with test.

    November 23rd, 2010 @ 08:02 PM

  8. Robert Pankowecki
    Robert Pankowecki
    • Tag changed from arel2, postgresql, primary_key, rails3.0.3, string to arel2, patch, postgresql, primary_key, rails3.0.3, string

    Fix and test looks fine for me. Thank you!

    November 23rd, 2010 @ 10:32 PM

  9. Aaron Patterson
    Aaron Patterson
    • State changed from open to committed

    November 23rd, 2010 @ 10:44 PM

  10. Robert Pankowecki
    Robert Pankowecki

    Could you backport to 3-0-stable ? I would be greatful.

    December 3rd, 2010 @ 02:46 PM

  11. lewy313
    lewy313

    I'm also affected by this ticket and would be very glad to have it backported

    December 9th, 2010 @ 09:30 AM

  12. Aaron Patterson
    Aaron Patterson

    Should be backported now. Thanks guys!

    December 9th, 2010 @ 05:37 PM

  13. Robert Pankowecki
    Robert Pankowecki

    I found out that there is one more test case failing related to this bug.

    Fix and additional test case is available here:

    https://github.com/rails/rails/pull/147

    This is for master.

    I'll also proved a patche for 3.0-stable and I hope that it will be included before 3.0.4 is released.

    December 31st, 2010 @ 01:24 PM

  14. Robert Pankowecki
    Robert Pankowecki

    Here is pull request for 3.0-stable.

    https://github.com/rails/rails/pull/148

    December 31st, 2010 @ 02:15 PM