I spent some time, and changed my opinion.


Now I'm pretty sure that this is just redundant code, effectively
no-op, and it can be safely deleted. Patch attached as
DBIx-Class-TimeStamp1.patch. It's impossible to create additional test
which will show what is not working without patching, because this
redundant code doesn't break anything. All current test after patching
is passed smoothly.



There is another possibility also - I think that original author
wanted to write something different, and I include this as
DBIx-Class-TimeStamp2.patch.

This could mean something like: "if we have both set_on_update and
dynamic_default_on_create, and dynamic_default_on_create is not from
our implementation, then use this foreign implementation for
set_on_update also". But this is not very good imho, at least because
we can't have any guarantee that method which is suitable for
dynamic_default_on_create, will be also suitable for
dynamic_default_on_update.

>From the other hand, if someone write:
    t_updated => {
        data_type => 'datetime',
        is_nullable => 0,
        dynamic_default_on_create => "somefunc",
        set_on_update => 1,
    },
and not set dynamic_default_on_update explicitly, then why
DBIx::Class::TimeStamp should make such assumptions?

So, I'm sure that second patch could lead to undesirable side effects
and could change expected (current) behaviour, and so should not be
applied.



Also, in DBIx-Class-TimeStamp3.patch - updates to POD, which is not
related to my question about code. Feel free to correct my "true
Oxford's English" ;)

-- 
Sincerely yours,
Oleg Kostyuk (CUB-UANIC)
Index: lib/DBIx/Class/TimeStamp.pm
===================================================================
--- lib/DBIx/Class/TimeStamp.pm	(revision 7427)
+++ lib/DBIx/Class/TimeStamp.pm	(working copy)
@@ -54,12 +54,6 @@
 
         if ( delete $info->{set_on_update} ) {
             $info->{dynamic_default_on_update} = 'get_timestamp';
-
-            if ( defined $info->{dynamic_default_on_create} and
-                 $info->{dynamic_default_on_create} eq 'get_timestamp'
-             ) {
-                $info->{dynamic_default_on_update} = 'get_timestamp';
-            }
         }
 
         push @columns, $col => $info;
Index: lib/DBIx/Class/TimeStamp.pm
===================================================================
--- lib/DBIx/Class/TimeStamp.pm	(revision 7427)
+++ lib/DBIx/Class/TimeStamp.pm	(working copy)
@@ -56,9 +56,9 @@
             $info->{dynamic_default_on_update} = 'get_timestamp';
 
             if ( defined $info->{dynamic_default_on_create} and
-                 $info->{dynamic_default_on_create} eq 'get_timestamp'
+                 $info->{dynamic_default_on_create} ne 'get_timestamp'
              ) {
-                $info->{dynamic_default_on_update} = 'get_timestamp';
+                $info->{dynamic_default_on_update} = $info->{dynamic_default_on_create};
             }
         }
 
Index: lib/DBIx/Class/TimeStamp.pm
===================================================================
--- lib/DBIx/Class/TimeStamp.pm	(revision 7427)
+++ lib/DBIx/Class/TimeStamp.pm	(working copy)
@@ -39,6 +39,25 @@
 This is effectively trigger emulation to get consistent behavior across
 databases that either implement them poorly or not at all.
 
+BE WARNED, that for C<set_on_update> this trigger emulation will work ONLY
+if row object already have some dirty columns. If row object don't have
+any dirty columns, then C<set_on_update> WILL NOT be triggered. If you want
+C<set_on_update> work always, even when you don't have any dirty columns,
+then you should also set C<always_update> to true value:
+
+ __PACKAGE__->add_columns(
+    ........
+    t_updated => {
+        data_type => 'datetime',
+        set_on_create => 1,
+        set_on_update => 1,
+        always_update => 1,
+    },
+    ........
+ );
+
+For more details about C<always_update> see L<DBIx::Class::DynamicDefault>
+
 =cut
 
 sub add_columns {
_______________________________________________
List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
IRC: irc.perl.org#dbix-class
SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
Searchable Archive: http://www.grokbase.com/group/[email protected]

Reply via email to