rails: Inconsistent results with #or in ActiveRecord::Relation with respect to documentation
According to documentation: https://github.com/rails/rails/blob/c577657f6de64b743b12a21108dc9cc5cfc35098/activerecord/lib/active_record/relation/query_methods.rb#L650
In order to use #or Neither relation may have a #limit, #offset, or #distinct set.
But, I could use all of the above like:
>> Post.where(id: 1).distinct.or(Post.where(id: 2).distinct)
=> SELECT DISTINCT "posts".* FROM "posts" WHERE ("posts"."id" = ? OR "posts"."id" = ?) [["id", 1], ["id", 2]]
OR
>> Post.where(id: 1).limit(1).or(Post.where(id: 2).limit(1))
=> SELECT "posts".* FROM "posts" WHERE ("posts"."id" = ? OR "posts"."id" = ?) LIMIT ? [["id", 1], ["id", 2], ["LIMIT", 1]]
Not sure if this is intended behaviour to support #limit, #offset, or #distinct.
If yes, then we need to improve the documentation about above use case. If no, then should it be considered as bug?
I would be happy to look into it and send PR for either case. But, wanted to confirm first for the intended behaviour.
About this issue
- Original URL
- State: closed
- Created 8 years ago
- Reactions: 19
- Comments: 26 (10 by maintainers)
Commits related to this issue
- refactoring deck policy due to https://github.com/rails/rails/issues/24055 — committed to zcallanan/rails-flash_cards by zcallanan 4 years ago
I was wondering what is the status of this issue? The PR that fixes the problem is rejected but this issue is still open, so is it going to be fixed or is it intended behaviour? Because I have the same problem as @ayghor but his work around isn’t working for me and the only other solution I have is using raw sql but I like to avoid that as much as possible
+1 for
:referencesthing. if i dothen
gives expected SQL
gives seemingly compatible (have to test further) but different SQL and
gives exception
which i believe to be bug, since
yandzSQLs are ok.@sgrif Sean, do you think someone can take a look at this? It’s biting a lot of people and makes
.orpretty much useless if you want the.orquery to use a simple join, which is a pretty common use case. There are 57 likes in this issue alone.Why can’t we allow :references? I have scopes I’m trying to combine together. Couple of points:
Just changing that method like this (add :references) works:
Any progress on this issue?
:referencesis already allowed by ea6139101ccaf8be03b536b1293a9f36bc12f2f7.After reading more in-depth and better-understanding some of the comments in this thread, I think the current behavior of
.oris perfectly acceptable.It took me a long while to build enough personal context to parse that out of this thread though, so here are summaries by use case. Hopefully useful to others
I want to
.ortogether queries that include.joinson rails >= 6.1.3I think this should just work! As @kamipo noted, ea61391 allows
:references. I thiiink that means that on rails >= 6.1.3, you can.ortogether.joins-containing scopes, and it will just work.I haven’t tested this myself though, and the test in the referenced commit only tests with
.include, not.joins. Maybe you’ll still need the rails 5 workaround below:I want to
.ortogether queries that include.joinson rails < 6.1.3 (probably mostly for people on rails 5)Workaround: move your
.joinsout to the end of the query. See earlier commentThis gets pretty unergonomic if you have
.joins-containing scopes. Consider upgrading to rails 6.1.3! Alas, I am still on rails 5.2. I rewrote my scopes to take an option to disable the internal.join, and it looks absolutely horrible (but it works).I want to
.ortogether queries containing.limitor.distinctMove the
.limits or.distincts to the after all your.ors, similar to the workaround for.joinsfor rails 5This is pretty reasonable if you think about trying to write equivalent SQL:
LIMITandDISTINCTcan only apply to whole queries, not parts of queries. (unless you useUNION, but I think that’s outside the intended scope of the.oroperator. My mental model for.oris that it resembles SQLORor boolean||–it combines predicates, not result sets.)@MSCAU 's example is illustrative here
Rewrite that last line like so.
This worksand I would argue is “prettier”. (whoops, this doesn’t quite work… needs more tweaking to reorderrequested_commentto the top, to ensure it’s within the bounds of thelimit. Still useful for exploration though…)But beauty is in the eye of the beholder–I think I find the latter expression prettier because it matches my mental model. I am thinking of the inputs to
.oras predicates. I’d wager a guess MSCAU’s thinks of them as collections:More specifically, I’m guessing @MSCAU thinks of
@live_commentsand@requested_commentas collections, and.oras union. Under that mental model, I should be able to combine 10@live_commentsand 1@requested_comment, and get 11 total comments.I think
.or, treats@live_commentsand@requested_commentmore like predicates. Predicates are properties of individual elements. They are things you might express as clauses of a.whereor or a.select.status: "live"is a predicate. An individual comment can bestatus: "live".id: requested_idis a predicate. An individual comment has the requested id or it does not.limit(10)can’t be a predicate. It’s meaningless applied to an individual comment. You can’t tell looking at an individual comment whether your whole result set contains 4 comments or 7 comments or 11 comments.Under this mental model, it’s impossible to limit the subqueries on the arms of the or, because it’s not something I can do by testing individual elements.
In sum, I think this makes more intuitive sense if you think about it in terms of SQL
OR. It combines boolean predicates that test individual rows, not whole sets (i.e. not wholeSELECTs). You can’t applyLIMITorDISTINCTto a subclause ofOR.Subcase: I read the TIL or makandracards articles @MSCAU mentioned. Why don’t they work?
To the best of my understanding, those articles are just wrong. The included code doesn’t work, and the underlying assumed concept is wrong. I’m guessing whoever originated them made a faulty assumption based on the “structurally compatible” bit of the error message, and then published without bothering to test.
I agree though, that the “structurally compatible” wording in the error message encourages wrong fixes. In particular, when I encountered this I also first reached towards making my subqueries “compatible” by making sure they had identical
.joinsclauses.I have a legitimate case where I really do want the SQL
UNIONbehaviorAlas, this is probably outside the scope of the
.oroperator. Make two queries and union them in memory, or write raw sql.Hacky workaround: do all your
.joinsafter the.or. This hides the offending.joinsfrom the checker.That is, the following two lines will both fail
but rearrange as follows, and you can evade the checker:
This works because all the checking happens inside the
.or()method. It’s blissfully unaware of shennanigans on its downstream results.One downside of course is it doesn’t read as nicely.
I was able to work around the problem using Arel with a solution based on @ayghor’s example.
Example of making joins conditional in Rails 5:
I’ve also added a reminder to simplify this when this project has been updated to Rails 6.1+.
👀
Rails 5.2
I cannot OR two ActiveRecord::Relations which both have .limit applied to them:
This is contrary to the advice provided at https://makandracards.com/alexander-m/40723-support-or-in-active-record and at https://til.hashrocket.com/posts/d3b0c7acec-rails-5-use-limit-on-activerecords-with-or.
The Guide is painfully brief on this topic:
https://guides.rubyonrails.org/active_record_querying.html#or-conditions
Is this a bug, and - if so - what’s the likelihood of a fix?