This project is archived and is in readonly mode.
Reduce queries in uniqueness validation
-
Michael Koziarski
- Assigned user set to Pratik
-
José Valim
Looks great! +1
-
Pratik
- Title changed from [PATCH] Reduce queries in uniqueness validation to Reduce queries in uniqueness validation
- State changed from new to incomplete
Missing tests :)
-
Carlos Júnior (xjunior)
How do I test if a query is being executed or not?
Thanks.
-
Pratik
You could use assert_sql and/or assert_queries helpers.
-
Carlos Júnior (xjunior)
Now with lasers, I mean, tests :P
-
Josh Susser
This is interesting. I think in general the approach of not revalidating attributes that haven't changed is probably an optimization worth pursuing. Uniqueness is an odd duck tho, since it's relative to external state. I'm also not sure what you should do if the scope value has changed when the validated attribute has not. Keep working on it though!
-
Nando Vieira
Uniqueness is an odd duck tho, since it's relative to external state.
I think the validation for uniqueness should always be executed, even when the attribute hasn't changed for this reason.
-
Carlos Júnior (xjunior)
I really don't think so. If the attribute was correctly saved (it was unique before), and it has not changed, then, doesn't matter the external state (that if was changed, was compared to the current object).
I just found a problem in my approach, that is when the scope changes (and I'm not validating this). I mean: I have a Product that belongs to a group, and the product name must be unique on that group. Then, I scope a validates_uniqueness_of :name to the group. If I change the group of the product, doesn't matter if I changed or not the :name, it must be validated.
Could you describe a concurrent situation where don't validate an attribute that didn't change can cause a problem (that doesn't include the situation above)?
-
Will Bryant
How about adding support (to all validations which take a *list of columns) for :on => :changed (like :on => :create/:update).
Then people who want to reduce queries and are sure external changes won't screw things up can use it, and the current safe behavior remains for everyone else.
-
Pratik
Carlos : I agree with everything Josh said. Also, partial updates can be turned off per model basis. So we should have tests for that, and always execute the query if partial updates are disabled.
-
CancelProfileIsBroken
- Tag changed from activerecord, database, models, performance, validations to activerecord, bugmash, database, models, performance, validations
-
Rizwan Reza
not reproducible
-1 Tests fail when I applied the patch to 2-3-stable. The patch doesn't apply to master.
-
Blue Box Jesse
BugMash: +1
The patch applies and I don't see the failure in tests that Rizwan saw.
-
Elad Meidar
- Tag changed from activerecord, bugmash, database, models, performance, validations to activerecord, bugmash, bugmash-review, database, models, performance, validations
+1 verified the overhead query, tests pass on 2-3-stable and tests pass. Master on the other hand, fails. i've attached a patch for master. (sorry about the credit, i have no idea how to credit the owner.)
-
sr.iniv.t
+1 verified.
The patches apply fine (Elad's on master and Carlos' on 2-3-stable) and all tests pass.
-
Kieran P
+1 verified Though I do wonder if *_changed? has the same issues mentioned at http://ryandaigle.com/articles/2008/3/31/what-s-new-in-edge-rails-d... (assignment/modification outside of attr=)
-
CancelProfileIsBroken
- Tag changed from activerecord, bugmash, bugmash-review, database, models, performance, validations to activerecord, bugmash-review, database, models, performance, validations
-
Jeremy Kemper
- Milestone changed from 2.x to 3.x
- Tag changed from activerecord, bugmash-review, database, models, performance, validations to activerecord, bugmash-review, database, models, performance, validations
-
Rizwan Reza
- Tag changed from activerecord, bugmash-review, database, models, performance, validations to activerecord, bugmash, database, models, performance, validations
-
Anil Wadghule
Elad Meidar's patch now does not get applied to master. Need to updated it. Elad Meidar can you update it for the master branch?
-
elcuervo
not reproducible in rails3 on current master.
-
jslag
Looks like validates_uniqueness_of was refactored pretty significantly in http://github.com/rails/rails/commit/44cd9e0e7132abe632664377f13f3e...
-
jslag
(which explains why Elad's patch doesn't apply to master any more)
-
Rohit Arondekar
- Importance changed from to
Does the issue still exist or has it been resolved? In master and 2-3-stable?
-
Santiago Pastorino
- State changed from incomplete to open
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 core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.
Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
-
Santiago Pastorino
- State changed from open to stale
