rzane / baby_squeel

:pig: An expressive query DSL for Active Record
MIT License
500 stars 49 forks source link

Missing constructs #73

Open octavpo opened 7 years ago

octavpo commented 7 years ago

I'm trying to translate from squeel, I couldn't find the equivalent of a few constructs: reorder, preload, includes, eager_load. Have they been implemented? Thanks.

rzane commented 7 years ago

Includes, preload, and eager_load were intentionally excluded. I don't see how baby squeel would actually add any value to those methods, since they just take symbols anyway.

But, you're right about reorder. I definitely missed that one.

octavpo commented 7 years ago

Well, squeel's preload and co. would also take stubs as arguments, so I could write: preload{assignments.students.user_role}. Not a big difference, but a little shorter and more uniform with the rest of the constructs.

rzane commented 7 years ago

I'm going to add this to compatibility mode, but, I don't think there needs to be a DSL just for the sake of having a DSL.

octavpo commented 7 years ago

What about the reorder? And btw there's also a rewhere. Thanks.

rzane commented 7 years ago

Whoops! Forgot about that one! I'll add it.

octavpo commented 7 years ago

I don't understand the reasoning for not adding includes and co. to the DSL. You're basically sending people back to AR. Or to the compatibility mode that you advise against. The whole purpose of squeel and baby_squeel is to make things easier/nicer than AR, after all there's nothing we can't do with just AR itself. Sure if you compare preload{problems} with preload(:problems), there's not much advantage for the first one. But in one place I have in squeel:

preload{[problem_sets_problems.problem_summaries.problem_state, assignments_problem_sets, student_assignments, categories_problem_sets, problem_sets_skills.skill, assets_problem_sets]}

which translates in AR to:

preload(:assignments_problem_sets, :student_assignments, :categories_problem_sets, :assets_problem_sets, problem_sets_problems: {problem_summaries: :problem_state}, problem_sets_skills: :skill)

I think the AR equivalent is definitely less clear, and I can't even keep the original order, I get a syntax error (sure order doesn't matter for the program, but it can matter for the programmer).

Besides that, there's a matter of language consistency. In a chain of baby_squeel calls using one syntax I have to go back and use AR syntax just because some constructs are missing. That's ugly. It would be great if baby_squeel provided a full set of equivalents to all AR query functions. Thanks.

rzane commented 7 years ago

I don't think there's anything wrong with encouraging people to use AR for includes. Using baby_squeel doesn't get you anything except a fancy syntax. I think the purpose of baby_squeel is to give you a more powerful way to generate SQL queries. It's not just "for pretty looks".

That being said, this feature isn't that hard to support, so I'll add it.

octavpo commented 7 years ago

Sounds great, thanks. And since we're at it, it would also be nice to have back the operator aliases (>>, <<) to in and not_in. Thanks again.