I'm not seeing this as a huge problem, if you don't want the scope applied to the stack then you could use with_exclusive_scope or something in the named_scope declaration?

To me, named_scopes should be composable. If I can't rely on the internals of a named_scope to create the same conditions every time I pass it the same parameters, then it's not reliable enough to use in a composable manner.

We often have search objects where a chain of named_scopes will get passed around, and you don't care what has already been applied, you just know you need to add an additional condition, so you tack on an additional named_scope knowing that it should add the condition regardless of what has already been added to the scope.

But now, the scopes no longer work in isolation. You need to know what scopes have been applied, and how those scopes will affect the implementation details of the scope you are about to apply.

You could, as you mention, construct all of your named_scopes using with_exclusive_scope inside the lambda, but that seems like too much work for what should be the default behavior (and possibly prone to error if there are default_scopes or larger with_scopes that you actually want for some reason).

It's not clear from the tests what the original use case was for this patch, but if it is necessary behavior, it feels like it should not be the default behavior.

I have to agree with Joseph. This change significantly effects the behavior of existing named scopes, and it's not clear what the original motivation is. Changing the internal behavior of a named_scope based on the scope in which it's applied at runtime strikes me as a violation of Least Surprise. Named scopes have always been somewhat order dependent (with regard to SELECT, ORDER BY, etc. clauses), as others have pointed out, but this effect on CONDITIONS takes order dependence to a new extreme.

Perhaps someone could explain the scenario that led to the change, and explain why it's important enough to break existing applications? Unless there's a truly compelling need I fully support rolling this change back.

At this point, I should explain our motivation for this patch. On a couple occasions we have created some named scopes for complex queries that execute their own, auxiliary queries to limit the complexity of any one query for performance reasons. Since the outcome of the named scope chain's final query is dependent on the results of an auxiliary query, we'd like the same conditions to be applied to it – otherwise a named scope with an inner query is not composable.

Admittedly, this is not the most common case, but regardless of the api I would like it to be convenient. I think the with_exclusive_scope is a decent "escape hatch" mechanism, but I would be open to a change in the opposite direction – perhaps using with_chained_scopes to apply the accumulated query options instead of having them available by default. Regardless of the outcome, I'm glad we're discussing the semantics of named scopes now.

Here is a test that shows the original functionality, before the patch.

I could get behind a :with_chained_scopes option to named_scope.

3rd try, not sure why the patch file is not making it to S3

Named scopes can be composable. Conditions and Joins are all merged together. Selects, Order, and Limit are not things that are usually used with named scopes, at least not ones that are intended to be chained with others.

The current behavior is confusing because a named_scope is not a scoping block like a with_scope. In a with_scope I expect that scope to be applied to the block and only the block. A named_scope is something even more targeted, a named_scope is applied directly to a finder, a single target. I would expect the scope to apply only to that target, and the lambda is not the target.

with_exclusive_scope is not something I want to sprinkle around inside any lambda that accesses an association or does a find. Especially with the arrival of default scopes. That will almost certainly lead to bugs.

I think that the usefulness of this original change is very small. If you are actually utilizing the accumulated scopes in your lambda to further restrict the result set size, it should not even change your final result set since that was already going to be constrained by the accumulated scopes. I think this is somewhat evident by the test that was committed with this change, it doesn't actually check the result set, because there is no change in the result set. From what I can tell, this is only a performance optimization for a very specific case.

What it does do however, is cause any associations that you access from inside your lambda, that happen to be the same class that your named_scope operations on, to be restricted by the accumulated scopes. The vast majority of time, this will be non-obvious, non-desirable, and lead to strange bugs.

(from [17f336e2f00f419a41eb7effb817bd7ad3e84f0d]) Revert "Methods invoked within named scope Procs should respect the scope stack. [#1267 state:resolved]"

This reverts commit 6a13376525f34a00e013fc3a6022838329dfe856.

Conflicts:

activerecord/test/cases/named_scope_test.rb

http://github.com/rails/rails/commit/17f336e2f00f419a41eb7effb817bd...

Here is the patch for the new functionality. Since named scopes have either taken a Proc OR a options hash, there is now the ability to pass the Proc as a :proc option.

I'm not terribly happy with how I ended up parsing the options inside named_scope, if somebody has something cleaner it would be great.

I included the tests for both old and new functionality.

I did a rough change to the documentation to explain the new functionality, but I didn't provide an example. Partially because I'm still not totally clear on a use case for this, and partially because I think it would probably take up a lot of space to make an understandable example.

Why does it keep eating my uploads?

Why just not let named_scope to be chained and be able to manipulate with_scope merge_conditions connector ? It would give named_scope new dimension of manipulating scopes, and save a lot of work that now is need to be done to allow match any criteria in ActiveRecord finders, i think right now using simple array condition builders are the best way for such thing, and IMHO for AR this is really shame, when you compare how mature this library should be after so many years of development.

For example it would allow to do something like this:

Ticket.any { pending.in_progress.resolved }.all(:conditions => { :created_at < Time.now })

And it would result with where conditions: "status_id = 1 OR status_id = 2 OR status_id = 3 AND created_at = '1970-01-01 00:00:00'"

This could also work:

Ticket.any(:conditions => { :created_at < Time.now, :status_id => [1,2,3] })

And where conditions should look like "created_at = '1970-01-01 00:00:00' OR status_id IN(1,2,3)"
And of course more deep chains should be also welcome.

Ticket.all { any { }.all { } }

all and #any methods without blocks should act as #find(:all) with AND/OR conditions connector, and with blocks, as part of whole named_scope stack including chained named_scope used inside #any/#all blocks