baby_squeel: Breaks with Rails 5.2.1

Thank you for looking at this, baby_squeel team. We are blocked on it too. Is there a solution in the wings or something we can help with?

Okay, so I started digging in here. Polyamorous becoming part of Ransack is inconvenient, but not a dealbreaker. Baby Squeel can simply depend on Ransack in order to bring in Polyamorous.

However, it really sucks when this library breaks when Active Record changes. And, digging through the Active Record source code, duplicating private Active Record functionality isn’t very future proof.

So, I’m proposing a change to Baby Squeel that I hope will remove this problem. Basically, 99% of the complexity comes from predicting a table alias. It’s a cool feature, but I don’t think it’s worth the maintenance effort or the lack of reliability.

When the same table is hit twice in the same query, Active Record will automatically create an alias for that table. In the example below, see the posts table is joined as posts_authors?

irb> Post.joins(author: :posts).where.has { author.posts.id > 0 }
SELECT "posts".* FROM "posts"
INNER JOIN "authors" ON "authors"."id" = "posts"."author_id"
INNER JOIN "posts" "posts_authors" ON "posts_authors"."author_id" = "authors"."id"
WHERE "posts_authors"."id" > 0

In the example above, Baby Squeel had to figure out that authors.posts.id references posts_authors. In future versions of Baby Squeel, you’ll have to explicitly say:

>  Post.joins(author: :posts).where.has { author.posts.as('posts_authors').id > 0 }

I’m going to release a new minor version that will give you a deprecation warning in any scenario where the “automatic aliasing” kicked in. It’ll describe exact steps for how you can remedy your code.

Once that’s released, I’m going to release Baby Squeel 2.0. This version should be able to support 5.2.1 and won’t automatically predict aliases anymore.

@Arpsara You can get latest polyamorous from ransack and then use ar-521

git 'https://github.com/activerecord-hackery/ransack', branch: 'master' do
  gem 'polyamorous'
end
gem 'baby_squeel', github: 'rzane/baby_squeel', branch: 'ar-521

I think having polyamorous as a gem living in the same repo would be the easiest thing to do.

@rtweeks, that would be a viable solution and would massively decrease the complexity of this library. Happy to accept pull requests!

Workable solution for me with Rails 5.2.2:

1. Gemfile:

gem 'baby_squeel'
gem 'polyamorous', '~> 1.3.4', git: 'https://github.com/vintrepid/polyamorous.git'

1. Monkey patch for join_dependency.rb:

module BabySqueel
  module JoinDependency
    class Builder # :nodoc:
      def find_alias(associations)
        join_association = find_join_association(associations)
        table = join_association.table || OpenStruct.new(name: join_association.base_klass.table_name)
        reconstruct_with_type_caster(table, associations)
      end
    end
  end
end

1. Change joining { association.outer } to left_joins(:association)
2. When the same table is hit twice in the same query - use as(:alias_name)

FWIW, I got this working with a mixture of these suggestions.

# Gemfile
 gem 'polyamorous', "~> 1.3.4", git: 'https://github.com/vintrepid/polyamorous.git'
 gem 'ransack', '2.2.1'

# initializer - monkey patch find_alias
module BabySqueel  
  module JoinDependency
    class Builder
      def find_alias(associations)
        join_association = find_join_association(associations)
        reconstruct_with_type_caster(join_association.table || OpenStruct.new(name: join_association.base_klass.table_name), associations)
      end
    end
  end
end

Yeah, that works for me. I’m don’t expect too many new features, and as such, I have no qualms about only supporting one Active Record version at a time.

Yeah, I’m revisiting my previous statements. Now that ransack has to be a dependency, we might be able to take advantage of how they resolve the aliases. Since Ransack is pretty actively maintained, this would allow us greater reliability.

I don’t have a lot of time right now, so I would love it if somebody would step up and try to achieve this. I can push up a branch of what I currently have soon.

I found a quick way to fix a lot of the broken tests (about 10 of them) by directing Active Record’s JoinDependency to construct Arel tables (including the aliases) and attach them the the JoinAssociations contained in the JoinDependency. This still leaves several issues with polymorphic joins and inner- vs. outer-joins. TL;DR I’m still working on it.

My team is just running into this issue now, too. I’m wondering if BabySqueel might just force aliases on associations (and track the ones it creates) rather than trying to discover the aliases assigned dynamically by ActiveRecord? I’d be willing to help with the implementation if this seems like a viable solution.

This may not be a solution for everyone but we found that we only used baby_squeel for a few things and I found some ways to have a decent syntax without it…

Most of our queries that used it were simple greater than or less then. I created an extension that could help like this.

module QueryExtensions
  extend ActiveSupport::Concern

  included do
    scope :where_gt, ->(column, value) { where("#{column} > ?", value) }
    scope :where_gte, ->(column, value) { where("#{column} >= ?", value) }
    scope :where_lt, ->(column, value) { where("#{column} < ?", value) }
    scope :where_lte, ->(column, value) { where("#{column} <= ?", value) }
  end
end

Then I included in my ApplicationRecord class that all my models inherit from.

class ApplicationRecord < ActiveRecord::Base
  include QueryExtensions
end

This allowed us to convert queries that were like this

User.where.has { created_at > 10.days.ago }

to

User.where_gt(:created_at, 10.days.ago)

And for joining we just used standard joins or left_joins depending on if it was outer or not.

c = customer # Needed for name conflict 
GiftCard
        .joining { customer }
        .where.has {
          (customer.id == c.id) | (phone == c.phone)
        }

To something like

query = GiftCard.joins(:customer)
query.where(gift_cards: { customers: { id: customer.id } })
  .or(query.where(gift_cards: { phone: customer.phone }))

Depending on how deeply embedded baby_squeel is in your application this may not be an option. The downside is that the queries are a little harder to read in my opinion but with active record ‘.or’ and ‘.merge’ you can get most things working without use of Arel tables. Also, the column names are not escaped but since this is not dynamic data passed in from user in our case should not be much of a problem

Disclaimer: Active Record >= 5.2.1 is currently broken. While the ar-521 branch might appear to work, I promise you, there are some very real problems on that branch. I do not recommend using it.

If this was simply a matter of updating dependencies, I would have released a new version. There are numerous test failures on that branch, all stemming from the inability to locate the correct alias for a table.

Unfortunately, I don’t have the time right now to dig in and figure out what is causing those issues. This is the line that is causing all of the problems. I would really appreciate help figuring out what the issue is.

@typhoon2099 polyamorous is released separately now, so you can just try:

gem 'polyamorous', '2.3.0'

Great to see all of this engagement. Keeping Polyamorous as it’s own gem inside Ransack works for me.

@gregmolnar 's point about aligning versions is a good one. Rails versions are moving pretty fast these days, and I think it is a fair policy for an open source project to support the currently supported Ruby & Rails versions only.

It would be a different story if there were many PRs to these projects, but in fact there are few and additionally the Ransack codebase needs a cleanup. It is always possible to lock an older app to an older version of the gem.

I really like Baby Squeel - it’s a great gem, I am going to start using it in my projects.

@rzane One thing we should iron out is, we are supporting different versions of Active Record. In ransack, we don’t really want to support any non-supported version of Active Record(and I mean full support, not security patches) because it is just too much extra work and hopefully it will make more people be on recent versions. Of course, as long as it doesn’t mean any extra work, we will depend > 5, but if it takes too much effort to make something work for an unsupported version, we will raise the dependency probably. As far as I see baby squeel seeems to support > 4.2.
My proposal is:

• we make polyamourous it’s own gem, move it under the ransack repository
• they will also have the same version and will be released together.
• we create a new version(2.2 probably) including support for old versions of Active Record
• we branch that off and later on we release 3.0 dropping support for old versions of Active Record
• we accept bugfixes to the 2.2 branch and backport PRs, but we won’t actively work on that version

You could first depend on polyamorous 2.2, then later or decide whether you want to stay on that, or follow the same policy as we do and jump on 3.0. Would you be happy with that?

/cc @seanfcarroll

We introduced baby_squeel because we have a lot of complex queries which were mostly written in plain SQL. Transforming them to plain ActiveRecord Relations was simply not possible and using Arel was far too complex for such huge queries. baby_squeel does this now for us and we are quite happy 🎉

Loosing one of the main features of baby_squeel is really a setback - but understandable 😞

@rzane i am willing to help if you have some ideas, because we rely on this awesome gem!

So, I’m proposing a change to Baby Squeel that I hope will remove this problem. Basically, 99% of the complexity comes from predicting a table alias. It’s a cool feature, but I don’t think it’s worth the maintenance effort or the lack of reliability.

To me, that was of the main benefits of Baby Squeel. Preloaded associations are straightforward enough but eager loaded association aliases are hard to predict. My opinion isn’t worth too much though, I’ve just left the company where I was using Baby Squeel.