This project is archived and is in readonly mode.
Alternative to validates_uniqueness_of using db constraints
-
JasonKing
- Tag cleared.
That looks really nice. I don't have time to test it right now, but +1 on a read-through.
-
JasonKing
...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
+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
- 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
Sure thing, attached. 2-3-stable patch now has pre-req patches rolled into it.
-
blythe
+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_ofso 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
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_ofinto 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
Oops, had a small typo in one of the test assertion messages. updates attached.
-
Jordan Brough
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
+1! I could use this (today).
-
Christos Zisopoulos
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
+1
-
Lawrence Pit
+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
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
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.
-
Santiago Pastorino
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
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]
-
Arfon Smith
+1
