has n, :items, :dependent => (:destroy|:delete|:nullify|:skip)

73 views
Skip to first unread message

Oleg Andreev

unread,
Nov 7, 2008, 9:05:39 AM11/7/08
to datam...@googlegroups.com, RubyOnRails to russian
This module adds support for :dependent option and also
sets :dependent => :destroy as a default strategy (which is a Good
Thing). You may set DestroyAssociations.brain_damaged_project = true
to set :dependent => nil as a default value.

http://pastie.org/309507

0) :delete_all is not supported yet (this is a single-query deletion
of associated records)
1) :delete is not fast, but faster than :destroy
2) :nullify implementation is not fast, but safe. Fast raw sql version
would not be safe. As well as :delete and :delete_all.

See comments in the source for more information.

Dan Kubb (dkubb)

unread,
Nov 8, 2008, 9:31:28 PM11/8/08
to DataMapper
Oleg, we actually created the dm-constraints plugin for precisely this
functionality. Currently it will create some DB level constraints, but
the plan was to make it create code level constraints always, and then
DB level constraints if the storage engine supports it. That way
there's two levels of protection on the data. It's also really nice
for testing because if you forget and delete a parent before a child,
it will blow up and tell you.

Dan
(dkubb)

Oleg Andreev

unread,
Nov 8, 2008, 9:52:35 PM11/8/08
to datam...@googlegroups.com
Thanks! I've just checked that up.

Actually, db-constraints lib doesn't play well in distributed
environment, where tables are located anywhere on the LAN in the
separate repositories. As far as I know, Oracle supports foreign key
constraints across distributed tables, but MySQL and PostgreSQL do
not. Also, I run tests using in-memory SQLite, which doesn't seem to
support foreign key constraints.

Anyway, it seems to be a right thing to keep that code in dm-
constraints library and also be able to switch off DB-level constraints.
Wdyt?

Dan Kubb (dkubb)

unread,
Nov 8, 2008, 10:51:52 PM11/8/08
to DataMapper
Oleg, yeah that sounds like a good idea. I hadn't thought about that
case.

Perhaps an argument to the association like :auto_constrain => true|
false.. where it would default to true (I would want to do the safest
thing possible as much as possible).

I also think the :protect option should be the default when someone
doesn't specify the :dependent constraint. I think it's rare in most
apps that have data you care about to allow orphan records (where the
parent is deleted but not the children). I mean, you wouldn't be
using dm-constraints if that wasn't the case so it's the safest
default :)

Also, we're trying to steer clear of "databasey" terms, so perhaps
this would be better:

:protect|:destroy|:destroy!|:set_nil|:skip # or maybe :nillify ? I
dunno, what do you think?

The :destroy! convention is because in other places within DM we use
the destroy! to indicate we are bypassing any hooks and just deleting
the data directly. We don't even instantiate the resource, we just
kill it.

Dan
(dkubb)

Oleg Andreev

unread,
Nov 8, 2008, 11:26:57 PM11/8/08
to datam...@googlegroups.com

On 09.11.2008, at 6:51, Dan Kubb (dkubb) wrote:

>
> Perhaps an argument to the association like :auto_constrain => true|
> false.. where it would default to true (I would want to do the safest
> thing possible as much as possible).

:sql_constraints => true/false may make more sense, imho.
(or :builtin_constraints maybe?..)

> I also think the :protect option should be the default when someone
> doesn't specify the :dependent constraint. I think it's rare in most
> apps that have data you care about to allow orphan records (where the
> parent is deleted but not the children). I mean, you wouldn't be
> using dm-constraints if that wasn't the case so it's the safest
> default :)

Good point. It is safe in both senses: data consistency and folks'
assumptions.

> Also, we're trying to steer clear of "databasey" terms, so perhaps
> this would be better:
>
> :protect|:destroy|:destroy!|:set_nil|:skip # or maybe :nillify ? I
> dunno, what do you think?

There should be both :set_nil and :set_nil!
:set_nil instantiates the model and sets nil (running all hooks,
validations etc.)
:set_nil! runs bulk UPDATE SQL statement.


:set_nil should rollback current transaction in case of validation
failure.
:protected should rollback current transaction in case of non-empty
set of dependencies.

Therefore, in case of presence of any dependency to be updated/
destroyed, #destroy method MUST start a new transaction (if it was not
started by the parent already).


The good spec looks like this:

DM::Constraints.default_constraints (attr_accessor, :protect by default)
DM::Constraints.sql_constraints (boolean attr_accessor, true by default)

Note: I really need these globals to set default rules for the whole
app. In current application I would use the following:
DM::Constraints.default_constraints = :destroy
DM::Constraints.sql_constraints = false

Association options:

:sql_constraints => true|false # default is
DM::Constraints.sql_constraints
:dependent => # default is DM::Constraints.default_constraints
:protect, # aborts transaction when set is not empty
:destroy, # safe destroy in a loop
:destroy!, # unsafe bulk delete
:set_nil, :nullify, :nillify, :nil, # safe update with
validations (all of these are aliases :-)
:set_nil!, :nullify!, :nillify!, :nil!, # unsafe bulk update
nil, :skip # skip foreign key
constraints (aliases)

+ automatic transactions with raising an error in case of invalid
record or a protected set.
+ infinite loop detection (cyclic dependencies): when detected,
transaction is aborted. User should specify appropriate #destroy hooks
or specific dependency options to avoid such situation.

I think it is going to be awesome!


PS. I don't know much about built-in sql constraints, but it might be
interesting to make destroy/nullify/skip options affect these also.

Dan Kubb (dkubb)

unread,
Nov 9, 2008, 1:48:58 AM11/9/08
to DataMapper
Oleg,

> > Perhaps an argument to the association like :auto_constrain => true|
> > false.. where it would default to true (I would want to do the safest
> > thing possible as much as possible).
>
> :sql_constraints => true/false may make more sense, imho.
> (or :builtin_constraints maybe?..)

I wouldn't want to name anything with "sql" because even though the
primary target for this feature is DBs, its not inconceivable that
other non-RDBMS engines could have something similar to foreign key
constraints.

The main reason for this flag is to ensure that the auto-migrate/
upgrade steps create the constraint. Given that I'd probably suggest
naming it :auto_migrate_constraint or something similar.

> There should be both :set_nil and :set_nil!
> :set_nil instantiates the model and sets nil (running all hooks,  
> validations etc.)
> :set_nil! runs bulk UPDATE SQL statement.

That makes sense to me.

> :set_nil should rollback current transaction in case of validation  
> failure.
> :protected should rollback current transaction in case of non-empty  
> set of dependencies.

I'm not sure the library code should automatically rollback a
transaction directly. I'd probably rather have it throw an
exception. If the code is inside a transaction block, and the
developer doesn't catch the exception, when it bubbles up the stack
the transaction will be rolled back. It's a subtle difference, but it
puts the control of the exception handling in the hands of the
developer. I'm wary of making an ORM that tries to be "too smart".

> Therefore, in case of presence of any dependency to be updated/
> destroyed, #destroy method MUST start a new transaction (if it was not  
> started by the parent already).

As stated above, I don't think DataMapper should handle too many
exceptional cases automatically. I don't think destroying a resource
should have any side effects like starting a transaction.

> The good spec looks like this:
>
> DM::Constraints.default_constraints (attr_accessor, :protect by default)
> DM::Constraints.sql_constraints (boolean attr_accessor, true by default)
>
> Note: I really need these globals to set default rules for the whole  
> app. In current application I would use the following:
>    DM::Constraints.default_constraints = :destroy
>    DM::Constraints.sql_constraints = false

Currently DM doesn't have any global variables like this. You'll
notice in dm-constraints that it mixes in some behavior into each
adapter.. I would add constants to those files that you could override
if need be.

> Association options:
>
> :sql_constraints => true|false  # default is  
> DM::Constraints.sql_constraints
> :dependent =>  # default is DM::Constraints.default_constraints
>    :protect,  # aborts transaction when set is not empty
>    :destroy,  # safe destroy in a loop
>    :destroy!, # unsafe bulk delete
>    :set_nil,  :nullify,  :nillify,  :nil,  # safe update with  
> validations (all of these are aliases :-)
>    :set_nil!, :nullify!, :nillify!, :nil!, # unsafe bulk update
>    nil, :skip                              # skip foreign key  
> constraints (aliases)

I would probably pick between :set_nil, :nullify and :nil. I think
there should be one correct way to do something like this: both to
keep the API clean and simple, and to keep the implementation as clean
and concise as possible.

> + automatic transactions with raising an error in case of invalid  
> record or a protected set.
> + infinite loop detection (cyclic dependencies): when detected,  
> transaction is aborted. User should specify appropriate #destroy hooks  
> or specific dependency options to avoid such situation.

I'm not entirely sure how you plan to approach the last point.

> PS. I don't know much about built-in sql constraints, but it might be  
> interesting to make destroy/nullify/skip options affect these also.

Oh, that's always the intention. With dm-constraints we want to make
it so the code-level hooks are *always* set up. Then we want to make
sure the DB level constraints match this whenever possible. Since our
code-level hooks take care of deleting/nullification the DB's won't
ever kick in, which is fine.

We consider the DB constraints to be more like a safety net.

Feel free to drop into #dm-hacking or #datamapper on IRC if you have
any questions or concerns. I'm dkubb on IRC.

Dan
(dkubb)

Oleg Andreev

unread,
Nov 9, 2008, 7:04:14 AM11/9/08
to datam...@googlegroups.com

On 09.11.2008, at 9:48, Dan Kubb (dkubb) wrote:
> I wouldn't want to name anything with "sql" because even though the
> primary target for this feature is DBs, its not inconceivable that
> other non-RDBMS engines could have something similar to foreign key
> constraints.

Oh, I got it!

> The main reason for this flag is to ensure that the auto-migrate/
> upgrade steps create the constraint. Given that I'd probably suggest
> naming it :auto_migrate_constraint or something similar.

Okay.

> I'm not sure the library code should automatically rollback a
> transaction directly. I'd probably rather have it throw an
> exception. If the code is inside a transaction block, and the
> developer doesn't catch the exception, when it bubbles up the stack
> the transaction will be rolled back.
> It's a subtle difference, but it
> puts the control of the exception handling in the hands of the
> developer. I'm wary of making an ORM that tries to be "too smart".

Yes, sure.

>> Therefore, in case of presence of any dependency to be updated/
>> destroyed, #destroy method MUST start a new transaction (if it was
>> not
>> started by the parent already).
>
> As stated above, I don't think DataMapper should handle too many
> exceptional cases automatically. I don't think destroying a resource
> should have any side effects like starting a transaction.

Just an example: if you don't have any transaction and you throw and
exception somewhere in the middle of destroying associations, you will
loose half of the data. More of that, since there should be
before(:destroy) callback, not after(), your DB will come to a
consistent state (sic!), so you probably won't even notice that!
That's very dangerous, IMO. This implies to use initial transaction as
a default decision. Which can be switched off, of course.

> Currently DM doesn't have any global variables like this. You'll
> notice in dm-constraints that it mixes in some behavior into each
> adapter.. I would add constants to those files that you could override
> if need be.

Okay. However global accessors might be nicer than remove_const/
cons_set.
It is not a hardcore "global variables programming", it's just a
configuration interface.

>> Association options:
>>
>> :sql_constraints => true|false # default is
>> DM::Constraints.sql_constraints
>> :dependent => # default is DM::Constraints.default_constraints
>> :protect, # aborts transaction when set is not empty
>> :destroy, # safe destroy in a loop
>> :destroy!, # unsafe bulk delete
>> :set_nil, :nullify, :nillify, :nil, # safe update with
>> validations (all of these are aliases :-)
>> :set_nil!, :nullify!, :nillify!, :nil!, # unsafe bulk update
>> nil, :skip # skip foreign key
>> constraints (aliases)
>
> I would probably pick between :set_nil, :nullify and :nil. I think
> there should be one correct way to do something like this: both to
> keep the API clean and simple, and to keep the implementation as clean
> and concise as possible.

:nullify is a well-known name from AR, but i'm comfortable with
anything else too.

Btw, :nil is a bad idea. It just one character away from nil which
might be interpreted incorrectly.


>> + infinite loop detection (cyclic dependencies): when detected,
>> transaction is aborted. User should specify appropriate #destroy
>> hooks
>> or specific dependency options to avoid such situation.
>
> I'm not entirely sure how you plan to approach the last point.

Me too =) That was just an idea.

You can pass around a stack of models (e.g. in a
Thread[:dm_constraints_stack]) to detect a moment when you come to
repeated model.

> Oh, that's always the intention. With dm-constraints we want to make
> it so the code-level hooks are *always* set up. Then we want to make
> sure the DB level constraints match this whenever possible. Since our
> code-level hooks take care of deleting/nullification the DB's won't
> ever kick in, which is fine.
> We consider the DB constraints to be more like a safety net.

Sure.

> Feel free to drop into #dm-hacking or #datamapper on IRC if you have
> any questions or concerns. I'm dkubb on IRC.

Okay. I'm oleganza.

Reply all
Reply to author
Forward
0 new messages