On Tuesday, August 28, 2012 12:35:11 AM UTC-7, Jamie Hodge wrote: > > Hi Jeremy, > > First of all, thank you. I'm really happy that you've unified validations > and constraints. I found their separation to be a source of bugs and > confusion. > > I've moved a small project over to the current version of the > extension/plugin and have three short comments. > > If you look at the snippet below, you'll see that I've purposely left some > redundant elements in the column and validation/constraint specification: > > String :name, null: false > citext :url, null: false, unique: true > > ... > > validate do > presence [:name, :url] > max_length 255, [:name, :url] > ... > unique :url > end > > I *believe* this is not an issue, but my question is if it would be > clearer to extend the behaviour of the existing column options (i.e. null > and unique), once the extension is loaded. Many of the column types also > include implicit size constraints, which could possibly be inferred. >
I purposely chose require constraint validations to be explicit. It is possible to use some implicit information as you mentioned, but it raises additional questions: When do you add to the metadata table? Do you check every time for the existence of the metadata table, and automatically add them if the metadata table is present? Do you only add them if there is a validate block inside the create_table/alter_table? What if you just want the constraint without the validation (for speed reasons), what do you do? > My second comment is a "nice to have," which would be to include basic > numerical checks, i.e. an integer field is greater than 0. > I'm open to adding additional types of constraint validations in the future. I started just mirroring the validations in validation_helpers, and there isn't an existing validation for greater than/less than (though there is one for max/min length. Note that if you have an upper bound, you can use includes with a range of integers. In order for additional constraint validations to be supported, the underlying validations have to be supported by validation_helpers, since that is what the constraint_validations plugin uses internally. > Lastly, and I'm not even sure if this is good practice, I tend to add > validates_presence against association accessors (i.e. thing, not > thing_id), with the purpose of avoiding having the DB raise reference > errors. Would it be possible to use the existing foreign_key helper to > generate minimal does-this-reference-exist checks in the application? > Theoretically, you could. But that requires doing stuff implicitly. Also, there isn't a way to map between foreign key names and association names when creating/altering tables (since associations are a model thing and schema modification is a database thing), so even if you wanted to do that, you'd have to add the association name as an option to the foreign_key call. > Once again, thank you! I've even noticed a speed-up during testing. > Good to hear. Thanks for taking the time to submit feedback. Jeremy -- You received this message because you are subscribed to the Google Groups "sequel-talk" group. To view this discussion on the web visit https://groups.google.com/d/msg/sequel-talk/-/Plaq2bDdrIsJ. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/sequel-talk?hl=en.
