Merge 'trunk' into 'try-tiny'
Peter Rabbitson [Mon, 17 May 2010 12:15:00 +0000 (12:15 +0000)]
r9396@Thesaurus (orig r9382):  rabbit | 2010-05-15 17:50:58 +0200
Fix stupid typo-bug
r9397@Thesaurus (orig r9383):  rabbit | 2010-05-15 18:04:59 +0200
Revert erroneous commit (belongs in a branch)
r9402@Thesaurus (orig r9388):  ash | 2010-05-16 12:28:13 +0200
Fix how Schema::Versioned gets connection attributes
r9408@Thesaurus (orig r9394):  caelum | 2010-05-16 19:29:14 +0200
add sql_maker to @rdbms_specific_methods

Changes
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/SQLAHacks.pm
lib/DBIx/Class/Schema/Versioned.pm
lib/DBIx/Class/Storage/DBI.pm
t/74mssql.t
t/94versioning.t
t/bind/order_by.t
t/resultset/update_delete.t
t/sqlahacks/order_by_func.t

diff --git a/Changes b/Changes
index 94576fa..120be79 100644 (file)
--- a/Changes
+++ b/Changes
@@ -20,10 +20,13 @@ Revision history for DBIx::Class
           the unsafe_subselect_ok attribute, due to optimized queries
         - Fix as_subselect_rs to not inject resultset class-wide where
           conditions outside of the resulting subquery
+        - Fix nasty potentially data-eating bug when deleting/updating
+          a limited resultset
         - Depend on optimized SQL::Abstract (faster SQL generation)
-        - update on row not in database now OK if no changes - 
-          fixes problems with cascaded unnecessary updates
-
+        - update() on row not in_storage no longer throws an exception
+          if there are no dirty columns to update (fixes cascaded update
+          annoyances)
+        - Update Schema::Versioned to respect hashref style of connection_info
 
 0.08121 2010-04-11 18:43:00 (UTC)
         - Support for Firebird RDBMS with DBD::InterBase and ODBC
index 7ac07ca..1dc4068 100644 (file)
@@ -1424,7 +1424,7 @@ sub _rs_update_delete {
   my $cond = $rsrc->schema->storage->_strip_cond_qualifiers ($self->{cond});
 
   my $needs_group_by_subq = $self->_has_resolved_attr (qw/collapse group_by -join/);
-  my $needs_subq = $needs_group_by_subq || (not defined $cond) || $self->_has_resolved_attr(qw/row offset/);
+  my $needs_subq = $needs_group_by_subq || (not defined $cond) || $self->_has_resolved_attr(qw/rows offset/);
 
   if ($needs_group_by_subq or $needs_subq) {
 
index c362e91..40bc7a3 100644 (file)
@@ -485,8 +485,6 @@ sub _recurse_fields {
       croak "Malformed select argument - too many keys in hash: " . join (',', keys %$fields );
     }
 
-    $func =~ s/^\-+//;  # strip leading dash, at some point the dash itself should become mandatory
-
     if (lc ($func) eq 'distinct' && ref $args eq 'ARRAY' && @$args > 1) {
       croak (
         'The select => { distinct => ... } syntax is not supported for multiple columns.'
@@ -577,23 +575,6 @@ sub _order_directions {
   ]);
 }
 
-sub _order_by_chunks {
-  my ($self, $arg) = @_;
-  if (  # non-empty hash with neither an -asc or a -desc
-    ref $arg eq 'HASH'
-      &&
-    keys %$arg
-      &&
-    ! exists $arg->{-desc}
-      &&
-    ! exists $arg->{-asc}
-  ) {
-    return $self->_recurse_fields ($arg);
-  }
-
-  return $self->SUPER::_order_by_chunks ($arg);
-}
-
 sub _table {
   my ($self, $from) = @_;
   if (ref $from eq 'ARRAY') {
index fe6c694..a10e1fd 100644 (file)
@@ -559,24 +559,25 @@ To avoid the checks on connect, set the environment var DBIC_NO_VERSION_CHECK or
 sub connection {
   my $self = shift;
   $self->next::method(@_);
-  $self->_on_connect($_[3]);
+  $self->_on_connect();
   return $self;
 }
 
 sub _on_connect
 {
-  my ($self, $args) = @_;
+  my ($self) = @_;
 
-  $args = {} unless $args;
+  my $info = $self->storage->connect_info;
+  my $args = $info->[-1];
 
-  $self->{vschema} = DBIx::Class::Version->connect(@{$self->storage->connect_info()});
+  $self->{vschema} = DBIx::Class::Version->connect(@$info);
   my $vtable = $self->{vschema}->resultset('Table');
 
   # useful when connecting from scripts etc
   return if ($args->{ignore_version} || ($ENV{DBIC_NO_VERSION_CHECK} && !exists $args->{ignore_version}));
 
   # check for legacy versions table and move to new if exists
-  my $vschema_compat = DBIx::Class::VersionCompat->connect(@{$self->storage->connect_info()});
+  my $vschema_compat = DBIx::Class::VersionCompat->connect(@$info);
   unless ($self->_source_exists($vtable)) {
     my $vtable_compat = $vschema_compat->resultset('TableCompat');
     if ($self->_source_exists($vtable_compat)) {
index 62ade80..686d0eb 100644 (file)
@@ -48,6 +48,7 @@ __PACKAGE__->sql_maker_class('DBIx::Class::SQLAHacks');
 my @rdbms_specific_methods = qw/
   deployment_statements
   sqlt_type
+  sql_maker
   build_datetime_parser
   datetime_parser_type
 
index f8b3849..b56c7e0 100644 (file)
@@ -204,6 +204,15 @@ SQL
     ok (($have_rno == $rno_detected),
       'row_number() over support detected correctly');
   }
+
+  {
+    my $schema = DBICTest::Schema->clone;
+    $schema->connection($dsn, $user, $pass);
+
+    like $schema->storage->sql_maker->{limit_dialect},
+      qr/^(?:Top|RowNumberOver)\z/,
+      'sql_maker is correct on unconnected schema';
+  }
 }
 
 # test op-induced autoconnect
index b73d612..ad4657e 100644 (file)
@@ -245,6 +245,33 @@ system( qq($^X -pi -e "s/ALTER/-- this is a comment\nALTER/" $fn->{trans_v23};)
   is($schema_v2->get_db_version(), '3.0', 'Fast deploy/upgrade');
 };
 
+# Check that it Schema::Versioned deals with new/all forms of connect arguments.
+{
+  my $get_db_version_run = 0;
+
+  no warnings qw/once redefine/;
+  local *DBIx::Class::Schema::Versioned::get_db_version = sub {
+    $get_db_version_run = 1;
+    return $_[0]->schema_version;
+  };
+
+  # Make sure the env var isn't whats triggering it
+  local $ENV{DBIC_NO_VERSION_CHECK} = 0;
+
+  DBICVersion::Schema->connect({
+    dsn => $dsn,
+    user => $user, 
+    pass => $pass,
+    ignore_version => 1
+  });
+  
+  ok($get_db_version_run == 0, "attributes pulled from hashref connect_info");
+  $get_db_version_run = 0;
+
+  DBICVersion::Schema->connect( $dsn, $user, $pass, { ignore_version => 1 } );
+  ok($get_db_version_run == 0, "attributes pulled from list connect_info");
+}
+
 unless ($ENV{DBICTEST_KEEP_VERSIONING_DDL}) {
     unlink $_ for (values %$fn);
 }
index 7f8df43..7a8bce6 100644 (file)
@@ -103,3 +103,4 @@ my @tests = (
 plan( tests => scalar @tests * 2 );
 
 test_order($_) for @tests;
+
index 05d245b..542ea07 100644 (file)
@@ -6,9 +6,6 @@ use Test::More;
 use Test::Exception;
 use DBICTest;
 
-#plan tests => 5;
-plan 'no_plan';
-
 my $schema = DBICTest->init_schema();
 
 my $tkfks = $schema->resultset('FourKeys_to_TwoKeys');
@@ -110,3 +107,10 @@ is_deeply (
 $sub_rs->delete;
 
 is ($tkfks->count, $tkfk_cnt -= 2, 'Only two rows deleted');
+
+# make sure limit-only deletion works
+cmp_ok ($tkfk_cnt, '>', 1, 'More than 1 row left');
+$tkfks->search ({}, { rows => 1 })->delete;
+is ($tkfks->count, $tkfk_cnt -= 1, 'Only one row deleted');
+
+done_testing;
index 1caac14..51968ed 100644 (file)
@@ -7,45 +7,29 @@ use DBICTest;
 use DBIC::SqlMakerTest;
 
 my $schema = DBICTest->init_schema();
-$schema->storage->sql_maker->quote_char ('"');
-$schema->storage->sql_maker->name_sep ('.');
 
 my $rs = $schema->resultset('CD')->search({}, {
     'join' => 'tracks',
-    order_by => [
-      { -length => 'me.title' },
-      {
+    order_by => {
         -desc => {
-            count => 'tracks.trackid',
+            count => 'tracks.track_id',
         },
-      },
-    ],
+    },
     distinct => 1,
     rows => 2,
-    page => 2,
+    page => 1,
 });
+my $match = q{
+    SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track FROM cd me
+    GROUP BY me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track
+    ORDER BY COUNT(tracks.trackid) DESC
+};
 
-is_same_sql_bind(
-  $rs->as_query,
-  '(
-    SELECT "me"."cdid", "me"."artist", "me"."title", "me"."year", "me"."genreid", "me"."single_track"
-      FROM cd "me"
-      LEFT JOIN "track" "tracks" ON "tracks"."cd" = "me"."cdid"
-    GROUP BY "me"."cdid", "me"."artist", "me"."title", "me"."year", "me"."genreid", "me"."single_track"
-    ORDER BY
-      LENGTH( "me"."title" ),
-      COUNT( "tracks"."trackid" ) DESC
-    LIMIT 2 OFFSET 2
-  )',
-  [],
-  'order by with func query',
-);
+TODO: {
+    todo_skip 'order_by using function', 2;
+    is_same_sql($rs->as_query, $match, 'order by with func query');
 
-ok($rs->count_rs->next == 2, 'amount of rows return in order by func query');
-is_deeply (
-  [ $rs->get_column ('me.title')->all ],
-  [ "Caterwaulin' Blues", "Come Be Depressed With Us" ],
-  'Correctly ordered stuff by title-length',
-);
+    ok($rs->count == 2, 'amount of rows return in order by func query');
+}
 
 done_testing;