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

Rails 5 IN-clause subquery #702

Closed
jaredbeck opened this issue Jul 22, 2016 · 6 comments
Closed

Rails 5 IN-clause subquery #702

jaredbeck opened this issue Jul 22, 2016 · 6 comments

Comments

@jaredbeck
Copy link
Contributor

jaredbeck commented Jul 22, 2016

ransack 1.8.0: In rails 4, it is possible to use a SqlLiteral node as the formatter of a custom ransacker. In rails 5 it is not. There may be an issue with rails 5's new attribute type system.

Consider this contrived, yet complete and executable example. The formatter is a subquery of Post ids.

begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  # gem 'rails', '4.2.7'
  gem 'rails', '5.0.0'
  gem 'ransack', '1.8.0', require: false
  gem 'sqlite3'
end

require 'active_record'
require "ransack"
require 'minitest/autorun'
require 'logger'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
    t.string :title
  end
end

class Post < ActiveRecord::Base
  ransacker(
    :banana,
    callable: -> (x) { x.table[:id] },
    formatter: ->(value) {
      ::Arel::Nodes::SqlLiteral.new(
        arel_table.project(:id).where(arel_table[:title].eq(value)).to_sql
      )
    }
  )
end

class BugTest < Minitest::Test
  def test_association_stuff
    a = Post.create!(title: 'apple')
    b = Post.create!(title: 'banana')
    c = Post.create!(title: 'coconut')

    # rails 4 passes. query is:
    # SELECT "posts".* FROM "posts" WHERE "posts"."id" IN (
    #   SELECT id FROM "posts" WHERE "posts"."title" = 'banana'
    # )

    # rails 5 fails. query is:
    # SELECT "posts".* FROM "posts" WHERE "posts"."id" IN (0)

    assert_equal [b], Post.ransack(banana_in: "banana").result
  end
end

I know there are better ways to use ransack to search post titles :) as I said, this is a contrived example.

@jonatack
Copy link
Contributor

Reproduced. On Rails 5.0.0 and master (5.1.0.alpha) too.

@jonatack
Copy link
Contributor

With both Ransack 1.8.1 (latest release) and also 1.7.0 from a year ago.

@jonatack
Copy link
Contributor

jonatack commented Jul 30, 2016

@jaredbeck This appears to be coming from a change in Arel or Active Record between Rails 4 and 5, not a bug in Ransack. It's a bit of a strange example. You might need to formulate your Arel query differently. Ransackers just provide a conduit to Arel. The Rails team has clarified over the past couple years that Arel methods are :nodoc: and likely to change without warning. So closing for now unless this is a bug in ransackers not correctly providing access to Arel.

P.S. - This works fine, though I imagine you are trying to do something else:

Post.ransack(title_in: 'banana').result

@jaredbeck
Copy link
Contributor Author

I know there are better ways to use ransack to search post titles :) as I said, this is a contrived example.

P.S. - This works fine, though I imagine you are trying to do something else:
Post.ransack(title_in: 'banana').result

Correct, my admittedly contrived example does not require an IN-clause subquery. My app does.

You might need to formulate your Arel query differently. Ransackers just provide a conduit to Arel. The Rails team has clarified over the past couple years that Arel methods are :nodoc: and likely to change without warning.

Yeah, my colleagues keep writing complicated custom ransackers that break every time I upgrade arel. Maybe there should be a warning about this in the custom ransacker docs. Would you like a PR with such a warning?

@jonatack
Copy link
Contributor

jonatack commented Aug 8, 2016

There might be news from the front. If I recall @sgrif's remarks in the latest http://bikeshed.fm/74 podcast discussing Ransack and Arel -- and I could be wrong -- the Arel API is planned to stabilize and return from :nodoc: back to public. Yay! -- to symbolically celebrate I added Sean's vintage Arel blog post to the two other Arel resources in the Ransack wiki today.

PR proposal: There is already a warning in the ransackers section of the wiki about complicated Arel ransackers not being a cure-all with mention of a few alternate solutions, so I think we're fine.

On the other hand, any of the TODO or FIXME comments in the code base would be wonderful PRs to work on.

On Monday, August 8, 2016, Jared Beck notifications@github.com wrote:

I know there are better ways to use ransack to search post titles :) as I
said, this is a contrived example.

P.S. - This works fine, though I imagine you are trying to do something
else:
Post.ransack(title_in: 'banana').result

Correct, my admittedly contrived example does not require an IN-clause
subquery. My app does.

You might need to formulate your Arel query differently. Ransackers just
provide a conduit to Arel. The Rails team has clarified over the past
couple years that Arel methods are :nodoc: and likely to change without
warning.

Yeah, my colleagues keep writing complicated custom ransackers that break
every time I upgrade arel. Maybe there should be a warning about this in
the custom ransacker docs. Would you like a PR with such a warning?


You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#702 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACTbfDn01lA1xcI4okScg0qiRPvBNtq4ks5qd4YYgaJpZM4JSbUn
.

@jaredbeck
Copy link
Contributor Author

.. news from the front.

🎺 🎵📰! 😀

.. the Arel API is planned to stabilize and return from :nodoc: back to public ..

That's great news, thanks.

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

No branches or pull requests

2 participants