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

Most upvoted comments

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 do

x = A.left_outer_joins(:bs, :cs)

then

x.where('bs.a_id = 1 OR cs.a_id = 1')

gives expected SQL

y = x.where('bs.a_id = 1')
z = x.where('cs.a_id = 1')
y.or(z)

gives seemingly compatible (have to test further) but different SQL and

y = x.where(bs: {a_id: 1})
z = x.where(cs: {a_id: 1})
y.or(z)

gives exception

ArgumentError: Relation passed to #or must be structurally compatible. Incompatible values: [:references]

which i believe to be bug, since y and z 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:

  1. I’d like not to care about if one does eager loading and other doesn’t. Load or throw out the loading, I could easily fix it up at another level, but if .or() throws an error then I’m back to the bad old days of using to_sql
  2. Some of the scopes have where clauses that reference associations, which means they cause LEFT OUTER JOINS, and cause ActiveRecord::QueryMethods#structurally_incompatible_values_for_or to complain about :references. This in turn causes or!() to raise. Thing is, it’s wrong - there are no conditions in the LEFT OUTER JOINS, all the conditions are still in the .where and are correctly “OR”'d together. So the check is unnecessarily gimping “or!():”

Just changing that method like this (add :references) works:

def structurally_incompatible_values_for_or(other)
  Relation::SINGLE_VALUE_METHODS.reject { |m| send("#{m}_value") == other.send("#{m}_value") } +
    (Relation::MULTI_VALUE_METHODS - [:eager_load, :references, :extending]).reject { |m| send("#{m}_values") == other.send("#{m}_values") } +
    (Relation::CLAUSE_METHODS - [:having, :where]).reject { |m| send("#{m}_clause") == other.send("#{m}_clause") }
end

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

I 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 comment

This 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 .limits or .distincts to the after all your .ors, similar to the workaround for .joins for rails 5

This is pretty reasonable if you think about trying to write equivalent SQL: LIMIT and DISTINCT can only apply to whole queries, not parts of queries. (unless you use UNION, but I think that’s outside the intended scope of the .or operator. My mental model for .or is that it resembles SQL OR or boolean ||–it combines predicates, not result sets.)

@MSCAU 's example is illustrative here

@live_comments = Comment.where(status: "live")
@requested_comment = Comment.where(id: requested_id)
@live_comments.limit(COMMENT_PAGE_SIZE - 1).or(@requested_comment.limit(1))

Rewrite that last line like so.

@live_comments.or(@requested_comment).limit(COMMENT_PAGE_SIZE)

This works and I would argue is “prettier”. (whoops, this doesn’t quite work… needs more tweaking to reorder requested_comment to the top, to ensure it’s within the bounds of the limit. 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 be status: "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 whole SELECTs). You can’t apply LIMIT or DISTINCT to a subclause of OR.

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 behavior

Alas, 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

A.joins(:b).where(b: b_query).or(A.where(query))  # error! 😞 
A.where(query).or(A.joins(:b).where(b: b_query))  # error! 😞 

but rearrange as follows, and you can evade the checker:

A.where(query).or(A.where(b: b_query)).joins(:b)  # works 😈 

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.

A.joins(:b)
  .where(
    A.arel_table[:something_a].eq('xxx').or(B.arel_table[:something_b].eq('yyy'))
  )

Example of making joins conditional in Rails 5:

scope :complex_stuff, -> {
  foo(joins: false).or(other_scope).joins_for_foo.any_other_joins_go_here
}

scope :foo, ->(joins: true) {
  base = joins ? joins_for_foo : self
  base.where("stuff")
}

scope :joins_for_foo, -> { joins(:bar) }

scope :other_scope, -> { where("other stuff") }

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:

Relation passed to #or must be structurally compatible

@comments = Comment.where(status: "live")
@requested_comment = Comment.where(id: requested_id)
@comments.limit(COMMENT_PAGE_SIZE - 1).or(@requested_comment.limit(1))

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?