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
:references
thing. 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
y
andz
SQLs are ok.@sgrif Sean, do you think someone can take a look at this? It’s biting a lot of people and makes
.or
pretty much useless if you want the.or
query 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?
:references
is 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
.or
is 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
.or
together queries that include.joins
on 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.or
together.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
.or
together queries that include.joins
on rails < 6.1.3 (probably mostly for people on rails 5)Workaround: move your
.joins
out 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
.or
together queries containing.limit
or.distinct
Move the
.limit
s or.distinct
s to the after all your.or
s, similar to the workaround for.joins
for rails 5This is pretty reasonable if you think about trying to write equivalent SQL:
LIMIT
andDISTINCT
can only apply to whole queries, not parts of queries. (unless you useUNION
, but I think that’s outside the intended scope of the.or
operator. My mental model for.or
is that it resembles SQLOR
or 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_comment
to 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
.or
as predicates. I’d wager a guess MSCAU’s thinks of them as collections:More specifically, I’m guessing @MSCAU thinks of
@live_comments
and@requested_comment
as collections, and.or
as union. Under that mental model, I should be able to combine 10@live_comments
and 1@requested_comment
, and get 11 total comments.I think
.or
, treats@live_comments
and@requested_comment
more like predicates. Predicates are properties of individual elements. They are things you might express as clauses of a.where
or or a.select
.status: "live"
is a predicate. An individual comment can bestatus: "live"
.id: requested_id
is 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 wholeSELECT
s). You can’t applyLIMIT
orDISTINCT
to 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
.joins
clauses.I have a legitimate case where I really do want the SQL
UNION
behaviorAlas, this is probably outside the scope of the
.or
operator. Make two queries and union them in memory, or write raw sql.Hacky workaround: do all your
.joins
after the.or
. This hides the offending.joins
from 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?