Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

default_scope breaks chained having statements in rails4 #13965

Open
helloenvoy opened this Issue · 18 comments

8 participants

@helloenvoy

A model with default_scope appears not to accept additional having statements. Here's an example:

> rails g migration create_test_model
> rake db:migrate

class TestModel < ActiveRecord::Base; end

> rails c
> TestModel.having('x').having('y').to_sql
 => "SELECT `test_models`.* FROM `test_models`  HAVING x AND y" 

class TestModel < ActiveRecord::Base
  default_scope -> { order('z') } # add a default scope
end

> rails c
> TestModel.having('x').having('y').to_sql
 => "SELECT `test_models`.* FROM `test_models`  HAVING x  ORDER BY z" 

This was a regression somewhere between 3.2.16 and 4.0.2.

@schneems
Collaborator

Don't use default scope. Just don't. Also thanks for reporting the bug. Could you provide us with a sample app that reproduces the problem?

@rafaelfranca
Owner

It is a know behavior of default scopes. See #13870

@rafaelfranca rafaelfranca reopened this
@rafaelfranca
Owner

The weird part is that the having is on the X column, and the expected behavior is to be in the Y column. I'll check

@rafaelfranca rafaelfranca self-assigned this
@diffractometer

@schneems Why no use default scope?

@schneems
Collaborator

@diffractometer i've attempted to use default_scope for the better part of my Rails programming days, and ever time it's bitten me in the butt. I've come to the conclusion like single table inheritance, the cons greatly outweigh the pros. Unfortunately this is anecdotal, so feel free to disregard and have your own opinions.

@helloenvoy

I have been warned about default_scopes. But I thought they just caused unexpected behavior, not incorrect behavior. They do show up in issues enough to make me very wary.

Here's a gist that fails on rails 4.0.2 (and 4.0.0) and passes on 3.2.16:

https://gist.github.com/helloenvoy/8874975

@rafaelfranca
Owner

Related with #13875

@diffractometer

I was wondering what was going on, thanks!

@hubertlepicki

Default scopes are evil... maybe we should have them removed altogether? I'm happy to provide a pull request for that.

@rafaelfranca

default_scope is not going anywhere.

@hubertlepicki

Because basecamp uses it? It's evil, provides major source of fun for developers who still use it, and, apparently, buggy.

@rafaelfranca

Yes. Basecamp uses it and a lot of other people too. I agree it is buggy, but we can fix. This doesn't means we need to remove.

@pallan

Could #13775 be related to this? Both refer to default_scope being injected where it was not expected.

@moxie

@rafaelfranca agreed. I think paranoia is a perfectly good use case for default_scope.

@moxie

A couple work around ideas if you need to use default_scope and this is inhibiting you:

  1. Build having statements in an array and concatenate and inject them into a single #having
  2. Use unscoped and then manually add your default_scope conditions back to the tail end of your relation.

Option 2 is actually quite nice.

@rails-bot
Collaborator

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@helloenvoy helloenvoy added the stale label
@rafaelfranca rafaelfranca removed their assignment
@rafaelfranca rafaelfranca removed the stale label
@rails-bot rails-bot added the stale label
@rails-bot
Collaborator

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.