Breaks with Rails 6.0.0

[vitalinfo]

Issue

Trying to prepare to Rails 6.0.0 and seems like babe_squeel (even with patch for Rails 5.2.1+) doesn't work

ArgumentError: wrong number of arguments (given 3, expected 2)

Reproduction

require 'bundler/inline'
require 'minitest/spec'
require 'minitest/autorun'

gemfile true do
  source 'https://rubygems.org'
  gem 'rails', '>= 6.0'
  gem 'sqlite3'
  gem 'baby_squeel'
  gem 'polyamorous', '~> 1.3.4', git: 'https://github.com/vintrepid/polyamorous'
end

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')

ActiveRecord::Schema.define do
  create_table :humans, force: true do |t|
    t.string :name
  end
  create_table :dogs, force: true do |t|
    t.string :name
    t.references :user
  end
end

class Human < ActiveRecord::Base
end

class Dog < ActiveRecord::Base
belongs_to :human
end

class BabySqueelTest < Minitest::Spec
  it 'breaks' do
    Dog.joins(:human).to_sql
  end
end

[vitalinfo]

fixed #105

[marnen]

Same issue here with Rails 6.0.3.2. Even the patch from #109 doesn't fix it. Is there hope for Rails 6 support in baby_squeel? Can I help somehow?

[kbighorse]

I enjoy baby_squeel, but it is keeping me from upgrading to Rails 6. The polyamorous gem dependency needs to be removed, beyond @vitalinfo's PR. Is this project still active?

[rzane]

@kbighorse, I haven't had the time to maintain this project. I'm willing to accept PRs, but @vitalinfo's PR doesn't pass the tests.

[marnen]

@rzane That’s worrisome; baby_squeel is essential for any Rails project that does any serious database work, and it’s in all our interest to see it kept compatible with current Rails, until such time as ActiveRecord exposes enough of Arel to make baby_squeel unnecessary.

Can I (or someone who knows the internals better than I do) step up and help you maintain it? Should we establish a way of making donations in order to keep the project viable?

I do not want to see baby_squeel become unusable for lack of maintainer interest, and I’m willing to do what I can to help prevent that from happening.

[vitalinfo]

@kbighorse, my solution was working for us in short term and give us opportunity to update Rails. But in the end we decided just remove this gem

[kbighorse]

@vitalinfo That makes sense, I am having to consider that option as well. @marnen I'm happy to help out, although I'm just one person.

[kbighorse]

@tenderlove, could Rails possibly take over ownership of this project? Perhaps with an eye to integrating it into a future release or opening up Arel? Would really love a solution to get through Rails 6 at least.

[kbighorse]

@rzane could you write tests that would support Rails 6 compatibility? I am happy to take a crack at an implementation. Maybe we can convince someone on the Rails/AR/Arel team to review?

[marnen]

@kbighorse My understanding is that baby_squeel exists because @ernie wrote the original Squeel intending it to be merged into ActiveRecord, the Rails core team didn’t take it, and it was too tiresome to maintain through ActiveRecord changes, or something like that. The idea of baby_squeel, I believe, was to be less closely coupled to ActiveRecord so it wouldn’t break on every Rails upgrade.

But now we’re seeing the same situation happening again. Baby_squeel is breaking on every Rails upgrade, @rzane doesn’t have time to maintain it, and we’re left without a good abstraction of complex Arel queries in current Rails. I really hope the Rails core team will take ownership of this project this time, but if they don’t, then someone in the community needs to step up to keep this project viable. (As I said, I’ll do it if I have to, although I’m not eager to.)

[rzane]

Honestly, the thing that makes this library unmaintainable is trying to infer aliased table names from joined associations by poking at Active Record's JoinDependency. I proposed that in that past, but people seemed pretty adamantly opposed to removing that functionality.

What I'd really like to do, is just create a more succinct, human-friendly layer on top of Arel. If I were to do that, the result would likely be much more maintainable.

[marnen]

@rzane:

people seemed pretty adamantly opposed to removing that functionality.

I remember that discussion. Clearly some people really need that functionality (I’ll have to analyze my own usage patterns and figure out if I’m one of them), so for them, that choice would be little better than leaving the library untouched.

What I'd really like to do, is just create a more succinct, human-friendly layer on top of Arel.

Isn’t that fundamentally what this gem is already? What about this vision would be different?

[rzane]

The main thing that would change is that Baby Squeel would make no attempt to infer table aliases. You'd have to manage those explicitly.

[marnen]

That would probably be a deal-breaker for me, if I understand you correctly. One reason I use baby_squeel is that it does the same sort of bookkeeping that ActiveRecord does for simpler queries.

[marnen]

...however, why does baby_squeel have to infer aliases at all? Can’t it construct its own aliases and keep track of them?

[rzane]

Primarily because you might have joined using plain ol' Active Record: User.joins(profile: :user).selecting { profile.user.id }

[marnen]

Right, that makes sense. Yes, I absolutely do expect that to be transparent, and not having it would remove a lot of the utility of this gem.

[rzane]

Hey @marnen, I took a stab at what a simple/maintainable version of this project would look like and I'm curious what you think. https://github.com/rzane/baby_squeel/tree/experiment

It's just a first pass (doesn't support polymormic associations or scoped associations), but it's still pretty powerful. If I were to release such a thing, I'd probably do it under a different name, to avoid confusion.

[marnen]

@rzane Cool, will take a look. Thanks!

[marnen]

@rzane OK, just took a quick look at the README. I’m not sure I see the point. It looks like the design goal of the experiment is to make the Ruby queries look like SQL rather than like (extended) ActiveRecord. If that’s so, what’s the advantage of that over writing literal SQL fragments? Composability?

I’m curious, too: does such a drastic change to the interface make it easier to decouple from ActiveRecord? (I’ll look at the code when I get a chance, but I wonder what your rationale was here.)

[marnen]

I should mention that one of my long-term plans on my previous contract was to introduce baby_squeel into their Rails 6 application, fixing any remaining compatibility issues as part of the contract (and thus getting paid for that work :) ) and contributing to the community. Unfortunately, while the client liked the idea, that contract ended before we could make it happen.

My current employer is using Rails 5.2, and while I’d like to introduce baby_squeel here too, I want to make sure that we don’t have to drop it as soon as we upgrade Rails (which we’re also hoping to do). I haven’t talked to them yet about the idea of having me fix baby_squeel as part of my job duties, but it may yet come up. (The observant reader will notice that I’m trying to do this as part of a paid job; that’s not my usual procedure for OSS contributions, but I think this is likely to be a good deal of work, and will probably be more fiddly than a typical volunteer contribution.)

I guess the reason that I bring this up is to say that yes, I’m absolutely willing (and I think able) to do the work necessary to make baby_squeel viable for the future, if no one else will. We just need to figure out what that work is.

[rzane]

Hey @marnen (and everyone else), I want to open up more discussion, and a closed issue probably isn't the right place for that 😂 . I've created two discussions:

• This discussion can be used to talk through Active Record 6 support. If you start working on that and need any guidance, maybe I can answer questions there.
• This discussion is a place where we can talk about what a different, more maintainable library might look like.

[marnen]

@rzane Sounds good, but I’ll also add that IMHO this issue should be reopened. The fundamental problem it represents (lack of Rails 6 support) isn’t solved.

[rzane]

Good point!

[pelletencate]

@rzane I know you're contemplating the viability of continuing working on this. That considered, I just stumbled upon https://github.com/camertron/arel-helpers/blob/master/lib/arel-helpers/join_association.rb that seems to be kept up to date pretty well.

Wouldn't using ArelHelpers to back baby_squeel be a good compromise?

[rocket-turtle]

@rzane This should be solved.

[marnen]

Does that mean that this now works with Rails 6? If so, that’s awesome news!

[rocket-turtle]

Yes if I remove the polyamorous from the test in the issue and set the rails version to gem 'rails', '= 6.0' it's green.

If you find any other bugs please provide an example.