This project is archived and is in readonly mode.

#6036 ✓committed
Robert Pankowecki

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

Reported by Robert Pankowecki | November 22nd, 2010 @ 04:05 PM | in 3.x

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.

Comments and changes to this ticket

  • Neeraj Singh

    Neeraj Singh November 22nd, 2010 @ 09:36 PM

    • Importance changed from “” to “Low”

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

  • Robert Pankowecki
  • Neeraj Singh

    Neeraj Singh November 22nd, 2010 @ 10:17 PM

    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?

  • Robert Pankowecki

    Robert Pankowecki November 23rd, 2010 @ 04:46 PM

    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>'
    
    
    
  • Neeraj Singh

    Neeraj Singh November 23rd, 2010 @ 04:57 PM

    • 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 .

  • Robert Pankowecki

    Robert Pankowecki November 23rd, 2010 @ 05:12 PM

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

  • Neeraj Singh

    Neeraj Singh November 23rd, 2010 @ 08:02 PM

    • Assigned user set to “Aaron Patterson”

    Attached is patch with test.

  • Robert Pankowecki

    Robert Pankowecki November 23rd, 2010 @ 10:32 PM

    • 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!

  • Aaron Patterson

    Aaron Patterson November 23rd, 2010 @ 10:44 PM

    • State changed from “open” to “committed”
  • Robert Pankowecki

    Robert Pankowecki December 3rd, 2010 @ 02:46 PM

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

  • lewy313

    lewy313 December 9th, 2010 @ 09:30 AM

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

  • Aaron Patterson

    Aaron Patterson December 9th, 2010 @ 05:37 PM

    Should be backported now. Thanks guys!

  • Robert Pankowecki

    Robert Pankowecki December 31st, 2010 @ 01:24 PM

    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.

  • Robert Pankowecki

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Pages