This project is archived and is in readonly mode.

#3486 open
Jordan Brough

Alternative to validates_uniqueness_of using db constraints

Reported by Jordan Brough | November 12th, 2009 @ 10:14 PM

This is a patch to enable ActiveRecord to identify db-generated errors for unique constraint violations. For example, it makes the following work without declaring a validates_uniqueness_of:

create_table "users" do |t|
  t.string   "email",   :null => false
end
add_index "users", ["email"], :unique => true

class User < ActiveRecord::Base
end

User.create!(:email => 'asdf@asdf.com')
u = User.create(:email => 'asdf@asdf.com')
u.errors[:email]
=> "has already been taken"

The benefits are speed, ease of use, and completeness --

Speed

With this approach you don't need to do a db lookup to check for uniqueness when saving (which can sometimes be quite slow when the index is missed -- https://rails.lighthouseapp.com/projects/8994/tickets/2503-validate... ). If you really care about validating uniqueness you're going to have to use database constraints anyway so the database will validate uniqueness no matter what and this approach removes an extra query. Checking the index twice isn't a problem for the DB (it's cached the 2nd time around), but saving a DB round-trip from the application is a big win.

Ease of use

Given that you have to have db constraints for true uniqueness anyway, this approach will let everything just happen automatically once the db constraints are in place. You can still use validates_uniqueness_of if you want to.

Completeness

validates_uniqueness_of has always been a bit of a hack -- it can't handle race conditions properly and results in exceptions that must be handled using somewhat redundant error handling logic. (See "Concurrency and integrity" section in http://api.rubyonrails.org/classes/ActiveRecord/Validations/ClassMe...)

Some things to consider with this approach are:

  1. u.valid? returns true before save is called since the save must be attempted for the error to be caught. However, you can still use validates_uniqueness_of in combination with this approach to keep valid? working (as far as it can work).
  2. In order to extract the field names of the unique constraint that was violated I am doing a DB lookup after the save fails (to query the DB indexes). If this is a 'bad thing' we could also investigate pre-caching the indexes so that the DB lookup isn't necessary.
  3. It could be (I have no data on this) that applications with high levels of unique-conflicts might see a performance hit by using this approach instead of validates_uniqueness_of, depending on how the DB handles rolling back from a failed update/insert. However, this patch improves the common case and and anyone with a high percentage of rollbacks that does see a performance hit can just add the validates_uniquenes_of constraint.

I've implemented handling for mysql (tested on 5.0 & 5.1), sqlite (tested on 3.6.11) & postgres (tested on 8.3.6) with graceful failover if the db error message can't be parsed or for other dbs that don't handlers written for them. (Outputs a generic error message in that case).

I added handling for composite unique indexes as follows:

# composite unique index on [:field1, :field2, :field3]
u1 = User.create!(:field1 => 'a', :field2 => 'a', :field3 => 'a')
u2 = User.create(:field1 => 'a', :field2 => 'a', :field3 => 'a')
u2.errors[:field1]
=> ["has already been taken for field2/field3"]

Patches attached for master and 2-3-stable including tests. Thoughts?

NOTE: The 2-3-stable patch depends on applying the following patches from ticket #2419 (which seems like a good idea to me anyway, given that they apply cleanly). Do the follwing before trying to apply to 2-3-stable:

git co 2-3-stable
git cherry-pick 4d614ec0429ce40c4784162c45ed06e125c0d7de
git cherry-pick 605acee71391729ae82ba9012bc37f2f1716fb80
git cherry-pick 9b39032925b68a724bd75174db642bc3d2f891fb
# now apply 2-3-stable patch

Comments and changes to this ticket

  • JasonKing

    JasonKing November 19th, 2009 @ 06:21 PM

    • Tag cleared.

    That looks really nice. I don't have time to test it right now, but +1 on a read-through.

  • JasonKing

    JasonKing November 19th, 2009 @ 06:30 PM

    ...although, the core behavior should be consistent - ie. throw an exception on any DB error. The Rails user should have to do something in order to get the special behavior in your patch.

    Maybe you could rewrite so that the special handling of the constraint exception only happens if the user specifies vuo in their model?

  • Greg Hazel

    Greg Hazel December 2nd, 2009 @ 10:42 PM

    +1, this feature looks great! If save! and such which should throw an exception if there is an error do with this patch, then I'm fine using it by default. It's not important to me as a user whether the error was raised in a validator or by the DB itself, as long as the DB is not changed either way.

  • Jeremy Kemper

    Jeremy Kemper December 22nd, 2009 @ 07:21 AM

    • State changed from “new” to “open”
    • Assigned user set to “Jeremy Kemper”

    Great patch. Could you rebase against latest master + 2-3-stable?

  • Jordan Brough

    Jordan Brough December 22nd, 2009 @ 04:12 PM

    Sure thing, attached. 2-3-stable patch now has pre-req patches rolled into it.

  • blythe

    blythe December 25th, 2009 @ 05:00 AM

    +1! Super excited to see this patch! It would be nice to populate the AR error messages for save! before raising the exception as well, to be consistent with standard validation functionality. Those who make heavy use of save!/rescue RecordInvalid will miss out.

    Since this looks wrapped up, I pulled some changes from my similar 2.3 gem, and logged a separate ticket #3614 to resolve this and some of the other inconsistencies mentioned above. It add hooks to validates_uniqueness_of so developers can explicitly declare intent to use this alternative, provide custom error messages, and raises RecordInvalid instead of RecordNotUnique errors consistent with AR validation functionality.

  • Jordan Brough

    Jordan Brough January 16th, 2010 @ 03:51 AM

    Attaching updated patches rebased against latest master + 2-3-stable. Re-ran activerecord tests successfully on mysql (5.0.41, 5.1.40), sqlite (3.6.11) & postgres (8.3.6).

    I've included an extra patch in each to add bang method handling. (thanks to blythe for pointing that out!).

    I don't agree with trying to squeeze validates_uniqueness_of into use here. The model can't configure or enable/disable the DB constraint and I think it's confusing to pretend that it does. The exception will happen regardless of model settings so Rails ought to just handle it automatically as gracefully as possible. Custom error messages can already be configured via config/locales files. e.g., adding this:

    en:
  activerecord:
    errors:
      models:
        user:
          attributes:
            email:
              taken: "has already been taken - custom"
              taken_multiple: "has already been taken for {{context}} - custom"
              taken_generic: "Unique requirement not met - custom"

    to config/locales/en.yml.

  • Jordan Brough

    Jordan Brough January 16th, 2010 @ 04:00 AM

    Oops, had a small typo in one of the test assertion messages. updates attached.

  • Jordan Brough

    Jordan Brough February 9th, 2010 @ 09:25 PM

    Jeremy - attached updated patch rebased against latest master. Still interested in the patch? Any chance of getting it into Rails 3 betas?

    Ran the tests against postgres 8.3.6, mysql 5.1.40 & 5.0.41, and sqlite3 3.6.11 on both master and on 2-3-stable.

  • J.D. Hollis

    J.D. Hollis February 11th, 2010 @ 08:13 PM

    +1! I could use this (today).

  • Christos Zisopoulos

    Christos Zisopoulos February 11th, 2010 @ 09:44 PM

    I would happily +1 if the following caveat was addressed.

    Jordan - at least for MySQL using the NDB (cluster) engine , UNIQUE indexes can throw another error message:

     ERROR 1169 (23000): Can't write, because of unique constraint, to table <table>

    See here: http://bugs.mysql.com/bug.php?id=21881

    I've also come across this error in the MySQL list of error messages, but I am not sure it applies to unique indexes.

     Error: 1291 SQLSTATE: HY000 (ER_DUPLICATED_VALUE_IN_TYPE)

    (list of MySQL errors: http://dev.mysql.com/doc/refman/5.1/en/error-messages-server.html)

  • Jarl Friis

    Jarl Friis February 24th, 2010 @ 07:25 PM

    +1

  • Kyle J. Ginavan

    Kyle J. Ginavan April 2nd, 2010 @ 09:09 PM

    +1

  • Lawrence Pit

    Lawrence Pit April 30th, 2010 @ 08:17 AM

    +1 on the idea, not for the implementation as is. I don't believe these constraint errors are always presented in English Instead of checking for English words like 'unique constraint', 'Duplicate entry', etc. I think it'd be better to check for the error codes. So e.g. for Oracle you'd check for "ORA-00001:", for mysql you'd check for ER_DUP_KEYNAME, ER_DUP_UNIQUE and ER_DUPLICATED_VALUE_IN_TYPE, etc.

  • JasonKing

    JasonKing April 30th, 2010 @ 06:07 PM

    This would require more changes to the AR API, to expose those error numbers before these exceptions are thrown.

    Generally, I think this would be a really good idea. I'm really not that familiar with the other Ruby ORMs, but exposing things like the error numbers, seems like a robust and mature step for AR to take.

  • Jordan Brough

    Jordan Brough May 14th, 2010 @ 08:53 PM

    Updated patches rebased onto latest master & 2-3-stable.

    Lawrence - your complaint is about detecting unique constraint violations, which is something that's already in Rails 3 (see http://github.com/rails/rails/commit/53a3eaa8 and note that error codes are being used in some cases.) Whether to improve that is probably a good item for a separate ticket. This ticket is about using the db (for the reasons above) to enforce uniqueness while making it fit into the standard AR model as nicely as possible. Figuring out which columns caused the unique constraint violation is something that db's can't provide via error numbers. I see your point about the parsing failing in non-english, but this patch was designed to fail gracefully in that case to a generic uniqueness message while still bringing normal AR error handling. Patches that build on this one to add multi-language support seem like a great idea to me.

    Christos - the issue you brings up spans both the earlier commit I mentioned as well as mine. Sounds like a great add-on patch (to address it on both levels) after we get this committed. In any case, this patch won't make the situation any worse for NDB. As I don't have NDB, if you could help out on figuring out an additional patch for NDB handling it'd be much appreciated.

  • Arfon Smith

    Arfon Smith October 24th, 2010 @ 11:06 PM

    • Importance changed from “” to “”

    Did this ever make it into 2.3.X ?

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:23 PM

    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.

  • Jordan Brough

    Jordan Brough February 5th, 2011 @ 06:46 AM

    I'm attaching updated patches rebased on top of latest master and 2-3-stable.

    Fwiw, we've been using this in production at animoto.com with Rails 2.3 for over a year now. This patch makes it easy to have reliable and fast uniqueness in a way that plays nicely with Rails. The most common cases are covered well and the less common cases have sensible fallbacks, or at least don't make things worse than they were before.

    I'd love to get feedback from the Rails team on whether they think this is something they would consider adding. Jeremy commented initially that it sounded like a good idea and I've continued to rebase and attach patches (and use them at Animoto) but would love to hear some feedback. Any thoughts?

    All tests pass for me with MySQL 5.1.50, PostgreSQL 8.4.6 and SQLite 3.7.0.1.

    [state:open]

  • Jordan Brough

    Jordan Brough February 5th, 2011 @ 06:49 AM

    • no changes were found...
  • Arfon Smith

    Arfon Smith February 5th, 2011 @ 08:49 PM

    +1

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Referenced by

Pages