datamapper / dm-core

DataMapper - Core
http://datamapper.org/
MIT License
754 stars 153 forks source link

:unique and :unique_index merge causes some inconsistencies #32

Open solnic opened 13 years ago

solnic commented 13 years ago

For a more complete look, see:

http://gist.github.com/434435

To summarize briefly:

If the @:unique@ option is used on more than one properties, the unique indicies created in the database and the validations created are either

  1. Inconsistent with each other
  2. Result in redundant queries being issued and/or redundant indicies being created

Created by Jonathan Stott (namelessjon) - 2010-06-11 13:18:17 UTC

Original Lighthouse ticket: http://datamapper.lighthouseapp.com/projects/20609/tickets/1316

solnic commented 13 years ago

@namelessjon: I’m going to have to look at this in more detail, but I should note that the value for :unique has nothing to do with he property names. It’s the name of the unique index you want the property to belong to.

by Dan Kubb (dkubb)

solnic commented 13 years ago

I agree that that would preserve the meaning of the old @:unique_index@ behaviour that I was using in 0.10.2. Never the less, that’s not how dm-validations treats it. In dm-validations, it is used as the scope of the uniqueness validator.

by Jonathan Stott (namelessjon)

solnic commented 13 years ago

fwiw, here’s a standalone, demonstrating an issue i have with :unique on more than 1 property: http://pastie.org/1065704.txt

by Martin Gamsjaeger (snusnu)

solnic commented 13 years ago

I am investigating

by Xavier Shay

solnic commented 13 years ago

(from [52373a9b387aeb298384f501bd8c6369fdd8b7e4]) Uniqueness auto-validations now raise a helpful error message when you try to scope by an arbitrarily named group, rather than an actual property.

It would be nice if this behaviour Just Worked, but I’m not confident on the interaction or intention between :unique and :unique_index, so for now this is strictly better than what was there.

[#1316 state:confirmed] https://github.com/datamapper/dm-validations/commit/52373a9b387aeb298384f501bd8c6369fdd8b7e4

by Dan Kubb (dkubb)

solnic commented 13 years ago

So that commit is a bit of a cop out but it’s what I’ve got at the moment. I can’t quite wrap my head around why we need both :unique and :unique_index. Would appreciate others chiming in as to what needs to happen to be able to mark this ticket as resolved.

Removing assignment from myself because I’m not in a position to push this forwards.

by Xavier Shay

solnic commented 13 years ago

[bulk edit]

by Dan Kubb (dkubb)

solnic commented 13 years ago

What I’d like to see is :unique_index deprecated, and :unique used across the board.

The allows values for :unique should be one of:

A Boolean should be a uniqueness constraint containing just the property. The name of the constraint is generated by the system.

A Symbol should be a uniqueness constraint containing all the properties that share the Symbol. The Symbol is used for the name.

An Array of Symbols behaves the same way as a single Symbol, only it defines multiple constrains the property belongs to.

These constraints have no direct side effects. Other plugins can reflect on the data to provide special behaviour, but it’s just for informational purposes as far as dm-core is concerned.

I would like for dm-validations and dm-migrations to be in sync. That is, the database level constraint should never be triggered when a record is validated before saving; except in the case where there is a race condition between the check and the save -- whether this is common or rare is different for each environment; in my case I have rarely seen it.

by Dan Kubb (dkubb)

solnic commented 13 years ago

Sounds good. I’ll tackle this again.

by Xavier Shay

solnic commented 13 years ago

Bowing out sorry, I’ve overcommitted myself.

by Xavier Shay

solnic commented 13 years ago

@namelessjon is this still a problem?