#1285 Reduce queries in uniqueness validation - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#1285 ✓stale

Reduce queries in uniqueness validation

Reported by Carlos Júnior (xjunior) | October 28th, 2008 @ 02:11 PM | in 3.x

When you're doing a uniqueness validation, you don't have to do it if the value haven't changed. Even if it's nil, because the validation should be done by the presence validation.

reduce_queries_in_uniqueness_validation.diff 721 Bytes

Comments and changes to this ticket

Michael Koziarski October 29th, 2008 @ 01:26 PM
- Assigned user set to “Pratik”
José Valim October 29th, 2008 @ 01:38 PM
Looks great! +1
Pratik October 29th, 2008 @ 01:40 PM
- Title changed from “[PATCH] Reduce queries in uniqueness validation” to “Reduce queries in uniqueness validation”
- State changed from “new” to “incomplete”
Missing tests :)
You flagged this item as spam.
Carlos Júnior (xjunior) October 29th, 2008 @ 03:53 PM
How do I test if a query is being executed or not?

Thanks.
Pratik October 29th, 2008 @ 03:56 PM
You could use assert_sql and/or assert_queries helpers.
You flagged this item as spam.
Carlos Júnior (xjunior) October 29th, 2008 @ 04:17 PM
Now with lasers, I mean, tests :P
- reduce_queries_in_uniqueness_validation.diff 1.4 KB
You flagged this item as spam.
Josh Susser October 29th, 2008 @ 11:37 PM
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!
You flagged this item as spam.
Nando Vieira October 30th, 2008 @ 10:52 AM
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.
You flagged this item as spam.
Carlos Júnior (xjunior) October 30th, 2008 @ 11:09 AM
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)?
You flagged this item as spam.
Will Bryant December 4th, 2008 @ 11:57 PM
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 December 6th, 2008 @ 12:06 AM
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 August 4th, 2009 @ 05:10 PM
- Tag changed from “activerecord, database, models, performance, validations” to “activerecord, bugmash, database, models, performance, validations”
Rizwan Reza August 10th, 2009 @ 12:07 AM
not reproducible

-1 Tests fail when I applied the patch to 2-3-stable. The patch doesn't apply to master.
You flagged this item as spam.
Blue Box Jesse September 27th, 2009 @ 12:37 AM
BugMash: +1

The patch applies and I don't see the failure in tests that Rizwan saw.
You flagged this item as spam.
Elad Meidar September 27th, 2009 @ 05:47 AM
- 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.)
- LH1285-reduce_validates_uniqueness_queries-master.diff 1.9 KB
You flagged this item as spam.
sr.iniv.t September 27th, 2009 @ 06:32 AM
+1 verified.

The patches apply fine (Elad's on master and Carlos' on 2-3-stable) and all tests pass.
You flagged this item as spam.
Kieran P September 27th, 2009 @ 07:06 AM
+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 September 27th, 2009 @ 12:31 PM
- Tag changed from “activerecord, bugmash, bugmash-review, database, models, performance, validations” to “activerecord, bugmash-review, database, models, performance, validations”
Jeremy Kemper May 4th, 2010 @ 06:48 PM
- 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”
[bulk edit]
Rizwan Reza May 15th, 2010 @ 06:45 PM
- Tag changed from “activerecord, bugmash-review, database, models, performance, validations” to “activerecord, bugmash, database, models, performance, validations”
[bulk edit]
You flagged this item as spam.
Anil Wadghule May 15th, 2010 @ 07:39 PM
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?
You flagged this item as spam.
elcuervo May 15th, 2010 @ 08:05 PM
not reproducible in rails3 on current master.
You flagged this item as spam.
jslag May 15th, 2010 @ 10:08 PM
Looks like validates_uniqueness_of was refactored pretty significantly in http://github.com/rails/rails/commit/44cd9e0e7132abe632664377f13f3e...
You flagged this item as spam.
jslag May 15th, 2010 @ 10:08 PM
(which explains why Elad's patch doesn't apply to master any more)
Rohit Arondekar July 4th, 2010 @ 03:14 AM
- Importance changed from “” to “”
Does the issue still exist or has it been resolved? In master and 2-3-stable?
Santiago Pastorino February 2nd, 2011 @ 05:01 PM
- 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 February 2nd, 2011 @ 05:01 PM
- State changed from “open” to “stale”