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

Breaking change on using associations in subqueries #12409

Closed
dnagir opened this issue Sep 30, 2013 · 5 comments
Closed

Breaking change on using associations in subqueries #12409

dnagir opened this issue Sep 30, 2013 · 5 comments

Comments

@dnagir
Copy link
Contributor

dnagir commented Sep 30, 2013

The example below works in Rails 3 but fails in Rails 4.

The reason is that the Relation returned by accessing the association now uses actual database parameter rather than interpolating the "owning" id.

class Blog < ActiveRecord::Base
  has_many :posts, :dependent => :destroy
end


class Post < ActiveRecord::Base
  belongs_to :blog
end


b = Blog.first # provided something exists of course
query = Post.where(id: b.posts.where("'complicated query' = ''")); 1 # just to avoid printing in console

puts query.to_sql

# SELECT "posts".* FROM "posts"  WHERE "posts"."id" IN (SELECT "posts"."id" FROM "posts"  WHERE "posts"."blog_id" = $1 AND ('complicated query' = ''))


query.to_a

# raises the error:
# PG::UndefinedParameter: ERROR:  there is no parameter $1
# LINE 1: ...M "posts"  WHERE "posts"."blog_id" = $1 AND 
#                                                 ^

Note the $1 parameter in the subquery which is obviously not provided from the main query (thus the error).

So the issue is that it is not possible to use subqueries the way we could in AR 3.

Although the idea of using actual database parameter is good, but unfortunately this is too big of a change from AR 3.

Also I had the assumption in Rails 3 that #to_sql always returns ready-to-execute query which is now incorrect.

@pixeltrix
Copy link
Contributor

It looks like something in the bit marked 'complicated query' is triggering the error because this script works okay:

gem 'activerecord', '4.0.0'
require 'active_record'
require 'minitest/autorun'
require 'logger'

ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Base.establish_connection(
  :adapter  => 'postgresql',
  :username => 'andyw',
  :database => 'test'
)

ActiveRecord::Schema.define do
  create_table :blogs, :force => true do |t|
  end

  create_table :posts, :force => true do |t|
    t.integer :blog_id
  end
end

class Blog < ActiveRecord::Base
  has_many :posts, :dependent => :destroy
end

class Post < ActiveRecord::Base
  belongs_to :blog
end

class TestSubquery < MiniTest::Unit::TestCase
  def test_subquery
    blog = Blog.create!
    post = blog.posts.create!
    posts = Post.where(id: blog.posts.where('posts.id > 0')).to_a

    assert_kind_of Array, posts
    assert_equal 1, posts.size
    assert_equal post, posts.first
  end
end

Can you take this script and adapt it to reproduce your problem - thanks.

@rafaelfranca
Copy link
Member

Although the idea of using actual database parameter is good, but unfortunately this is too big of a change from AR 3.

This is why we released a Rails 4 not 3.3

Also I had the assumption in Rails 3 that #to_sql always returns ready-to-execute query which is now incorrect.

This was never true. It returns a statement that can be a prepared statement. If you don't want prepared statement you can wrap you code inside a unprepared_statement block. But the right thing to do is pass the binds to your query.

@rafaelfranca
Copy link
Member

Closed by mistake 😅

@dnagir
Copy link
Contributor Author

dnagir commented Oct 1, 2013

@pixeltrix thanks for the help. Yes, that does indeed work correctly. Just bear with me for a little, I'll try to narrow down the issue to confirm (or not) this without posting full app. Suspecting squeel...

@dnagir
Copy link
Contributor Author

dnagir commented Oct 1, 2013

Okay. It is definitely squeel that breaks it, activerecord-hackery/squeel#272

Thanks for the help guys. Closing it.

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

No branches or pull requests

3 participants