Breaks with Rails 6.0.0 about baby_squeel HOT 28 CLOSED

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.

from baby_squeel.

@rzane This should be solved.

from baby_squeel.

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

from baby_squeel.

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

from baby_squeel.

@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?

from baby_squeel.

@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?

from baby_squeel.

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.

from baby_squeel.

fixed #105

from baby_squeel.

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?

from baby_squeel.

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?

from baby_squeel.

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

from baby_squeel.

@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

from baby_squeel.

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

from baby_squeel.

@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.)

from baby_squeel.

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.

from baby_squeel.

@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?

from baby_squeel.

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.

from baby_squeel.

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.

from baby_squeel.

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

from baby_squeel.

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

from baby_squeel.

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.

from baby_squeel.

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.

from baby_squeel.

@rzane Cool, will take a look. Thanks!

from baby_squeel.

@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.)

from baby_squeel.

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.

from baby_squeel.

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

from baby_squeel.

Good point!

from baby_squeel.

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

from baby_squeel.