Merge 'trunk' into 'mssql_limit_regression'
Peter Rabbitson [Fri, 8 Jan 2010 18:19:41 +0000 (18:19 +0000)]
r8076@Thesaurus (orig r8064):  ribasushi | 2009-12-12 12:31:12 +0100
Even clearer unloaded FK exception
r8078@Thesaurus (orig r8066):  ribasushi | 2009-12-12 14:27:18 +0100
As clear as it gets
r8141@Thesaurus (orig r8129):  ovid | 2009-12-16 17:40:50 +0100
Have has_one/might_have warn if set on nullable columns.

r8143@Thesaurus (orig r8131):  caelum | 2009-12-17 13:30:10 +0100
somewhat better fix for ADO
r8144@Thesaurus (orig r8132):  caelum | 2009-12-17 13:34:20 +0100
minor changes
r8146@Thesaurus (orig r8134):  caelum | 2009-12-17 17:44:34 +0100
cleanup source_bind_attributes for ADO
r8147@Thesaurus (orig r8135):  caelum | 2009-12-17 18:09:55 +0100
more types for ADO fix, and documentation
r8148@Thesaurus (orig r8136):  abraxxa | 2009-12-17 19:54:55 +0100
Cookbook POD fix for add_drop_table instead of add_drop_tables

r8158@Thesaurus (orig r8146):  ribasushi | 2009-12-18 14:55:53 +0100
 r8150@Thesaurus (orig r8138):  abraxxa | 2009-12-17 23:22:07 +0100
 Views without a view_definition won't be added to the SQL::Translator::Schema by the parser + tests

 r8151@Thesaurus (orig r8139):  abraxxa | 2009-12-17 23:23:33 +0100
 test cleanups

 r8153@Thesaurus (orig r8141):  abraxxa | 2009-12-18 14:34:14 +0100
 throw_exception if view_definition is missing instead of silent skipping + test changes

 r8154@Thesaurus (orig r8142):  abraxxa | 2009-12-18 14:40:32 +0100
 use Test::Exception

 r8155@Thesaurus (orig r8143):  abraxxa | 2009-12-18 14:42:00 +0100
 fixed Changes

 r8156@Thesaurus (orig r8144):  abraxxa | 2009-12-18 14:44:52 +0100
 test cleanups

 r8157@Thesaurus (orig r8145):  ribasushi | 2009-12-18 14:46:26 +0100
 Another bitr

r8160@Thesaurus (orig r8148):  ribasushi | 2009-12-18 15:04:34 +0100
Fix no_index entries
r8162@Thesaurus (orig r8150):  abraxxa | 2009-12-18 15:59:58 +0100
Schema POD inprovement for dclone

r8163@Thesaurus (orig r8151):  abraxxa | 2009-12-18 16:07:27 +0100
link to DBIx::Class::Row

r8164@Thesaurus (orig r8152):  abraxxa | 2009-12-18 16:08:56 +0100
fixed typo in Changes

r8165@Thesaurus (orig r8153):  abraxxa | 2009-12-18 16:14:47 +0100
dclone pod take #2

r8169@Thesaurus (orig r8157):  ribasushi | 2009-12-19 18:47:42 +0100
detabify
r8170@Thesaurus (orig r8158):  ribasushi | 2009-12-19 19:41:42 +0100
Fix RT52812
r8171@Thesaurus (orig r8159):  caelum | 2009-12-23 07:16:29 +0100
minor POD fixes
r8175@Thesaurus (orig r8163):  ribasushi | 2009-12-24 09:59:52 +0100
Fix deployment_statements context sensitivity regression
r8176@Thesaurus (orig r8164):  ribasushi | 2009-12-24 10:13:37 +0100
Don't call the PK setter if no PK
r8204@Thesaurus (orig r8192):  caelum | 2009-12-30 22:58:47 +0100
bump CAG dep
r8231@Thesaurus (orig r8219):  matthewt | 2010-01-02 01:41:12 +0100
fix typo in variable name
r8238@Thesaurus (orig r8226):  rafl | 2010-01-02 18:46:40 +0100
Merge branch 'native_traits'

* native_traits:
  Port replicated storage from MXAH to native traits.
  Create branch native_traits
r8244@Thesaurus (orig r8232):  caelum | 2010-01-04 00:30:51 +0100
fix _rebless into sybase/mssql/nobindvars
r8247@Thesaurus (orig r8235):  caelum | 2010-01-05 13:54:56 +0100
 r22328@hlagh (orig r8201):  caelum | 2009-12-31 12:29:51 -0500
 new branch to fix table aliases in queries over the 30char limit
 r22329@hlagh (orig r8202):  caelum | 2009-12-31 12:55:50 -0500
 failing test
 r22330@hlagh (orig r8203):  caelum | 2009-12-31 13:00:35 -0500
 switch oracle tests to done_testing()
 r22331@hlagh (orig r8204):  caelum | 2009-12-31 15:02:50 -0500
 got something working
 r22332@hlagh (orig r8205):  caelum | 2009-12-31 15:08:30 -0500
 POD touchups
 r22343@hlagh (orig r8216):  caelum | 2010-01-01 07:42:03 -0500
 fix uninitialized warning and a bug in ResultSet
 r22419@hlagh (orig r8234):  caelum | 2010-01-05 07:53:18 -0500
 append half of a base64 MD5 to shortened table aliases for Oracle

r8249@Thesaurus (orig r8237):  caelum | 2010-01-05 15:27:40 +0100
minor change: use more of the hash if possible for oracle table alias shortening
r8251@Thesaurus (orig r8239):  caelum | 2010-01-06 02:20:17 +0100
bump perl_version to 5.8.1
r8252@Thesaurus (orig r8240):  caelum | 2010-01-06 02:21:41 +0100
remove alignment mark on base64 md5
r8260@Thesaurus (orig r8248):  ribasushi | 2010-01-07 11:21:55 +0100
5.8.1 is minimum required perl
r8261@Thesaurus (orig r8249):  ribasushi | 2010-01-07 11:22:42 +0100
Minor optimization
r8262@Thesaurus (orig r8250):  ribasushi | 2010-01-07 11:23:35 +0100
Wrong title
r8265@Thesaurus (orig r8253):  ribasushi | 2010-01-08 17:48:50 +0100
Resolve problem reported by http://lists.scsys.co.uk/pipermail/dbix-class/2009-December/008699.html
r8266@Thesaurus (orig r8254):  ribasushi | 2010-01-08 17:52:01 +0100
Put utf8columns in line with the store_column fix
r8267@Thesaurus (orig r8255):  ribasushi | 2010-01-08 19:03:26 +0100
Tests while hunting for something else
r8268@Thesaurus (orig r8256):  ribasushi | 2010-01-08 19:14:42 +0100
Make test look even more like http://lists.scsys.co.uk/pipermail/dbix-class/2009-November/008599.html

Changes
lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/SQLAHacks.pm
lib/DBIx/Class/Storage/DBI.pm
lib/DBIx/Class/Storage/DBI/MSSQL.pm
t/746mssql.t
t/search/related_strip_prefetch.t [new file with mode: 0644]

diff --git a/Changes b/Changes
index 24e4967..3099a42 100644 (file)
--- a/Changes
+++ b/Changes
@@ -10,6 +10,8 @@ Revision history for DBIx::Class
           parsed by SQL::Translator::Parser::DBIx::Class
         - Schema POD improvement for dclone
         - Fix regression in context sensitiveness of deployment_statements
+        - Fix regression resulting in overcomplicated query on
+          search_related from prefetching resultsets
 
 0.08115 2009-12-10 09:02:00 (CST)
         - Real limit/offset support for MSSQL server (via Row_Number)
index 13df20d..ae55dfb 100644 (file)
@@ -2640,10 +2640,19 @@ sub _chain_relationship {
       ||
     $self->_has_resolved_attr (@force_subq_attrs)
   ) {
+    # Nuke the prefetch (if any) before the new $rs attrs
+    # are resolved (prefetch is useless - we are wrapping
+    # a subquery anyway).
+    my $rs_copy = $self->search;
+    $rs_copy->{attrs}{join} = $self->_merge_attr (
+      $rs_copy->{attrs}{join},
+      delete $rs_copy->{attrs}{prefetch},
+    );
+
     $from = [{
       -source_handle => $source->handle,
       -alias => $attrs->{alias},
-      $attrs->{alias} => $self->as_query,
+      $attrs->{alias} => $rs_copy->as_query,
     }];
     delete @{$attrs}{@force_subq_attrs, 'where'};
     $seen->{-relation_chain_depth} = 0;
index 02c336c..ac3ae4a 100644 (file)
@@ -59,10 +59,12 @@ sub _RowNumberOver {
   # whatever is left
   my $group_having = $self->_order_by($order);
 
-  $sql = sprintf (<<'EOS', $order_by, $sql, $group_having, $offset + 1, $offset + $rows, );
+  my $qalias = $self->_quote ($self->{_dbic_rs_attrs}{alias});
+
+  $sql = sprintf (<<'EOS', $qalias, $order_by, $sql, $group_having, $qalias, $offset + 1, $offset + $rows, );
 
 SELECT * FROM (
-  SELECT orig_query.*, ROW_NUMBER() OVER(%s ) AS rno__row__index FROM (%s%s) orig_query
+  SELECT %s.*, ROW_NUMBER() OVER(%s ) AS rno__row__index FROM (%s%s) %s
 ) rno_subq WHERE rno__row__index BETWEEN %d AND %d
 
 EOS
index 2939f4a..f621aad 100644 (file)
@@ -1768,11 +1768,24 @@ sub _select_args {
 
   my @limit;
 
-  # see if we need to tear the prefetch apart (either limited has_many or grouped prefetch)
-  # otherwise delegate the limiting to the storage, unless software limit was requested
+  # see if we need to tear the prefetch apart otherwise delegate the limiting to the
+  # storage, unless software limit was requested
   if (
+    #limited has_many
     ( $attrs->{rows} && keys %{$attrs->{collapse}} )
        ||
+    # limited prefetch with RNO subqueries
+    (
+      $attrs->{rows}
+        &&
+      $sql_maker->limit_dialect eq 'RowNumberOver'
+        &&
+      $attrs->{_prefetch_select}
+        &&
+      @{$attrs->{_prefetch_select}}
+    )
+      ||
+    # grouped prefetch
     ( $attrs->{group_by}
         &&
       @{$attrs->{group_by}}
@@ -1782,7 +1795,6 @@ sub _select_args {
       @{$attrs->{_prefetch_select}}
     )
   ) {
-
     ($ident, $select, $where, $attrs)
       = $self->_adjust_select_args_for_complex_prefetch ($ident, $select, $where, $attrs);
   }
index cc831e8..178a007 100644 (file)
@@ -179,8 +179,9 @@ sub _execute {
 sub last_insert_id { shift->_identity }
 
 #
-# MSSQL is retarded wrt ordered subselects. One needs to add a TOP 100%
-# to *all* subqueries, do it here.
+# MSSQL is retarded wrt ordered subselects. One needs to add a TOP
+# to *all* subqueries, but one also can't use TOP 100 PERCENT
+# http://sqladvice.com/forums/permalink/18496/22931/ShowThread.aspx#22931
 #
 sub _select_args_to_query {
   my $self = shift;
@@ -190,7 +191,8 @@ sub _select_args_to_query {
   # see if this is an ordered subquery
   my $attrs = $_[3];
   if ( scalar $self->sql_maker->_order_by_chunks ($attrs->{order_by}) ) {
-    $sql =~ s/^ \s* SELECT \s/SELECT TOP 100 PERCENT /xi;
+    my $max = 2 ** 32;
+    $sql =~ s/^ \s* SELECT \s/SELECT TOP $max /xi;
   }
 
   return wantarray
index e34d0f0..c32797a 100644 (file)
@@ -178,10 +178,10 @@ is $rs->find($row->id)->amount, undef,'updated money value to NULL round-trip';
 
 $schema->storage->dbh_do (sub {
     my ($storage, $dbh) = @_;
-    eval { $dbh->do("DROP TABLE Owners") };
-    eval { $dbh->do("DROP TABLE Books") };
+    eval { $dbh->do("DROP TABLE owners") };
+    eval { $dbh->do("DROP TABLE books") };
     $dbh->do(<<'SQL');
-CREATE TABLE Books (
+CREATE TABLE books (
    id INT IDENTITY (1, 1) NOT NULL,
    source VARCHAR(100),
    owner INT,
@@ -189,7 +189,7 @@ CREATE TABLE Books (
    price INT NULL
 )
 
-CREATE TABLE Owners (
+CREATE TABLE owners (
    id INT IDENTITY (1, 1) NOT NULL,
    name VARCHAR(100),
 )
@@ -205,10 +205,10 @@ lives_ok ( sub {
     [qw/1   wiggle/],
     [qw/2   woggle/],
     [qw/3   boggle/],
-    [qw/4   fREW/],
-    [qw/5   fRIOUX/],
-    [qw/6   fROOH/],
-    [qw/7   fRUE/],
+    [qw/4   fRIOUX/],
+    [qw/5   fRUE/],
+    [qw/6   fREW/],
+    [qw/7   fROOH/],
     [qw/8   fISMBoC/],
     [qw/9   station/],
     [qw/10   mirror/],
@@ -220,11 +220,12 @@ lives_ok ( sub {
   ]);
 }, 'populate with PKs supplied ok' );
 
+
 lives_ok (sub {
   # start a new connection, make sure rebless works
   # test an insert with a supplied identity, followed by one without
   my $schema = DBICTest::Schema->connect($dsn, $user, $pass);
-  for (1..2) {
+  for (2, 1) {
     my $id = $_ * 20 ;
     $schema->resultset ('Owners')->create ({ id => $id, name => "troglodoogle $id" });
     $schema->resultset ('Owners')->create ({ name => "troglodoogle " . ($id + 1) });
@@ -256,18 +257,110 @@ lives_ok ( sub {
 
 # make sure ordered subselects work
 {
+  my $owners = $schema->resultset ('Owners')->search ({}, { order_by => 'name', offset => 2, rows => 3 });
+
+  my $al = $owners->current_source_alias;
+  my $sealed_owners = $owners->result_source->resultset->search (
+    {},
+    {
+      alias => $al,
+      from => [{
+        -alias => $al,
+        -source_handle => $owners->result_source->handle,
+        $al => $owners->as_query,
+      }],
+    },
+  );
+
+  is_deeply (
+    [ map { $_->name } ($sealed_owners->all) ],
+    [ map { $_->name } ($owners->all) ],
+    'Sort preserved from within a subquery',
+  );
+
+
+  my $corelated_owners = $owners->result_source->resultset->search (
+    {
+      id => { -in => $owners->get_column('id')->as_query },
+    },
+    {
+      order_by => 'name'
+    },
+  );
+
+  is_deeply (
+    [ map { $_->name } ($corelated_owners->all) ],
+    [ map { $_->name } ($owners->all) ],
+    'Sort preserved from within a corelated subquery',
+  );
+}
+
+TODO: {
+  local $TODO = "This porbably will never work, but it isn't critical either afaik";
+
   my $book_owner_ids = $schema->resultset ('BooksInLibrary')
-                               ->search ({}, { join => 'owner', distinct => 1, order_by => { -desc => 'owner'} })
+                               ->search ({}, { join => 'owner', distinct => 1, order_by => 'owner.name' })
                                 ->get_column ('owner');
 
-  my $owners = $schema->resultset ('Owners')->search ({
+  my $book_owners = $schema->resultset ('Owners')->search ({
     id => { -in => $book_owner_ids->as_query }
   });
 
-  is ($owners->count, 8, 'Correct amount of book owners');
-  is ($owners->all, 8, 'Correct amount of book owner objects');
+  is_deeply (
+    [ map { $_->id } ($book_owners->all) ],
+    [ $book_owner_ids->all ],
+    'Sort is preserved across IN subqueries',
+  );
 }
 
+# make sure right-join-side single-prefetch ordering limit works
+{
+  my $rs = $schema->resultset ('BooksInLibrary')->search (
+    {
+      'owner.name' => { '!=', 'woggle' },
+    },
+    {
+      prefetch => 'owner',
+      order_by => 'owner.name',
+    }
+  );
+  # this is the order in which they should come from the above query
+  my @owner_names = qw/boggle fISMBoC fREW fRIOUX fROOH fRUE wiggle wiggle/;
+
+  is ($rs->all, 8, 'Correct amount of objects from right-sorted joined resultset');
+  is_deeply (
+    [map { $_->owner->name } ($rs->all) ],
+    \@owner_names,
+    'Rows were properly ordered'
+  );
+
+  my $limited_rs = $rs->search ({}, {rows => 7, offset => 2});
+  is ($limited_rs->count, 6, 'Correct count of limited right-sorted joined resultset');
+  is ($limited_rs->count_rs->next, 6, 'Correct count_rs of limited right-sorted joined resultset');
+
+  my $queries;
+  $schema->storage->debugcb(sub { $queries++; });
+  $schema->storage->debug(1);
+
+  is_deeply (
+    [map { $_->owner->name } ($limited_rs->all) ],
+    [@owner_names[2 .. 7]],
+    'Limited rows were properly ordered'
+  );
+  is ($queries, 1, 'Only one query with prefetch');
+
+  $schema->storage->debugcb(undef);
+  $schema->storage->debug(0);
+
+
+  is_deeply (
+    [map { $_->name } ($limited_rs->search_related ('owner')->all) ],
+    [@owner_names[2 .. 7]],
+    'Rows are still properly ordered after search_related'
+  );
+}
+
+
 #
 # try a prefetch on tables with identically named columns
 #
@@ -337,38 +430,13 @@ $schema->storage->_sql_maker->{name_sep} = '.';
   is ($books->page(2)->count_rs->next, 1, 'Prefetched grouped search returns correct count_rs');
 }
 
-# make sure right-join-side ordering limit works
-{
-  my $rs = $schema->resultset ('BooksInLibrary')->search (
-    {
-      'owner.name' => [qw/wiggle woggle/],
-    },
-    {
-      join => 'owner',
-      order_by => { -desc => 'owner.name' },
-    }
-  );
-
-  is ($rs->all, 3, 'Correct amount of objects from right-sorted joined resultset');
-  my $limited_rs = $rs->search ({}, {rows => 3, offset => 1});
-  is ($limited_rs->count, 2, 'Correct count of limited right-sorted joined resultset');
-  is ($limited_rs->count_rs->next, 2, 'Correct count_rs of limited right-sorted joined resultset');
-  is ($limited_rs->all, 2, 'Correct amount of objects from limited right-sorted joined resultset');
-
-  is_deeply (
-    [map { $_->name } ($limited_rs->search_related ('owner')->all) ],
-    [qw/woggle wiggle/],    # there is 1 woggle library book and 2 wiggle books, the limit gets us one of each
-    'Rows were properly ordered'
-  );
-}
-
 done_testing;
 
 # clean up our mess
 END {
   if (my $dbh = eval { $schema->storage->_dbh }) {
     eval { $dbh->do("DROP TABLE $_") }
-      for qw/artist money_test Books Owners/;
+      for qw/artist money_test books owners/;
   }
 }
 # vim:sw=2 sts=2
diff --git a/t/search/related_strip_prefetch.t b/t/search/related_strip_prefetch.t
new file mode 100644 (file)
index 0000000..10621ae
--- /dev/null
@@ -0,0 +1,43 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Exception;
+
+use lib qw(t/lib);
+use DBIC::SqlMakerTest;
+use DBICTest;
+
+my $schema = DBICTest->init_schema();
+
+my $rs = $schema->resultset('CD')->search (
+  { 'tracks.id' => { '!=', 666 }},
+  { join => 'artist', prefetch => 'tracks', rows => 2 }
+);
+
+my $rel_rs = $rs->search_related ('tags', { 'tags.tag' => { '!=', undef }}, { distinct => 1});
+
+is_same_sql_bind (
+  $rel_rs->as_query,
+  '(
+    SELECT tags.tagid, tags.cd, tags.tag
+      FROM (
+        SELECT me.cdid, me.artist, me.title, me.year, me.genreid, me.single_track
+          FROM cd me
+          JOIN artist artist ON artist.artistid = me.artist
+          LEFT JOIN track tracks ON tracks.cd = me.cdid
+        WHERE ( tracks.id != ? )
+        LIMIT 2
+      ) me
+      JOIN artist artist ON artist.artistid = me.artist
+      LEFT JOIN track tracks ON tracks.cd = me.cdid
+      LEFT JOIN tags tags ON tags.cd = me.cdid
+    WHERE ( tags.tag IS NOT NULL )
+    GROUP BY tags.tagid, tags.cd, tags.tag
+  )',
+
+  [ [ 'tracks.id' => 666 ] ],
+  'Prefetch spec successfully stripped on search_related'
+);
+
+done_testing;