Merge 'null_column_regression' into 'trunk'
Peter Rabbitson [Fri, 15 Jan 2010 00:25:39 +0000 (00:25 +0000)]
r8167@Thesaurus (orig r8155):  ribasushi | 2009-12-19 12:50:13 +0100
New branch for null-only-result fix
r8168@Thesaurus (orig r8156):  ribasushi | 2009-12-19 12:51:21 +0100
Failing test
r8322@Thesaurus (orig r8310):  ribasushi | 2010-01-15 00:48:09 +0100
Correct test order
r8323@Thesaurus (orig r8311):  ribasushi | 2010-01-15 01:15:33 +0100
Generalize the to-node inner-join-er to apply to all related_resultset calls, not just counts
r8324@Thesaurus (orig r8312):  ribasushi | 2010-01-15 01:16:05 +0100
Adjust sql-emitter tests
r8326@Thesaurus (orig r8314):  ribasushi | 2010-01-15 01:25:10 +0100
One more sql-test fix and changes

Changes
lib/DBIx/Class/ResultSet.pm
t/inflate/hri.t
t/prefetch/double_prefetch.t
t/relationship/core.t
t/resultset/nulls_only.t [new file with mode: 0644]
t/search/related_strip_prefetch.t

diff --git a/Changes b/Changes
index 359cb56..a7c383b 100644 (file)
--- a/Changes
+++ b/Changes
@@ -13,6 +13,8 @@ Revision history for DBIx::Class
         - Fix regression in context sensitiveness of deployment_statements
         - Fix regression resulting in overcomplicated query on
           search_related from prefetching resultsets
+        - Fix regression on all-null returning searches (properly switch
+          LEFT JOIN to JOIN in order to distinguish between both cases)
         - Better isolation of RNO-limited queries from the rest of a
           prefetching resultset
         - New MSSQL specific resultset attribute to allow hacky ordered
index baf6be2..5827fb4 100644 (file)
@@ -974,19 +974,6 @@ sub _construct_object {
 sub _collapse_result {
   my ($self, $as_proto, $row) = @_;
 
-  # if the first row that ever came in is totally empty - this means we got
-  # hit by a smooth^Wempty left-joined resultset. Just noop in that case
-  # instead of producing a {}
-  #
-  my $has_def;
-  for (@$row) {
-    if (defined $_) {
-      $has_def++;
-      last;
-    }
-  }
-  return undef unless $has_def;
-
   my @copy = @$row;
 
   # 'foo'         => [ undef, 'foo' ]
@@ -1247,11 +1234,6 @@ sub _count_rs {
   $tmp_attrs->{select} = $rsrc->storage->_count_select ($rsrc, $tmp_attrs);
   $tmp_attrs->{as} = 'count';
 
-  # read the comment on top of the actual function to see what this does
-  $tmp_attrs->{from} = $self->result_source->schema->storage->_straight_join_to_node (
-    $tmp_attrs->{from}, $tmp_attrs->{alias}
-  );
-
   my $tmp_rs = $rsrc->resultset_class->new($rsrc, $tmp_attrs)->get_column ('count');
 
   return $tmp_rs;
@@ -1279,11 +1261,6 @@ sub _count_subq_rs {
 
   $sub_attrs->{select} = $rsrc->storage->_subq_count_select ($rsrc, $sub_attrs);
 
-  # read the comment on top of the actual function to see what this does
-  $sub_attrs->{from} = $self->result_source->schema->storage->_straight_join_to_node (
-    $sub_attrs->{from}, $sub_attrs->{alias}
-  );
-
   # this is so that the query can be simplified e.g.
   # * non-limiting joins can be pruned
   # * ordering can be thrown away in things like Top limit
@@ -2510,10 +2487,11 @@ sub related_resultset {
 
   $self->{related_resultsets} ||= {};
   return $self->{related_resultsets}{$rel} ||= do {
-    my $rel_info = $self->result_source->relationship_info($rel);
+    my $rsrc = $self->result_source;
+    my $rel_info = $rsrc->relationship_info($rel);
 
     $self->throw_exception(
-      "search_related: result source '" . $self->result_source->source_name .
+      "search_related: result source '" . $rsrc->source_name .
         "' has no such relationship $rel")
       unless $rel_info;
 
@@ -2524,6 +2502,13 @@ sub related_resultset {
     my $alias = $self->result_source->storage
         ->relname_to_table_alias($rel, $join_count);
 
+    # since this is search_related, and we already slid the select window inwards
+    # (the select/as attrs were deleted in the beginning), we need to flip all 
+    # left joins to inner, so we get the expected results
+    # read the comment on top of the actual function to see what this does
+    $attrs->{from} = $rsrc->schema->storage->_straight_join_to_node ($attrs->{from}, $alias);
+
+
     #XXX - temp fix for result_class bug. There likely is a more elegant fix -groditi
     delete @{$attrs}{qw(result_class alias)};
 
@@ -2536,7 +2521,7 @@ sub related_resultset {
       }
     }
 
-    my $rel_source = $self->result_source->related_source($rel);
+    my $rel_source = $rsrc->related_source($rel);
 
     my $new = do {
 
@@ -2687,7 +2672,6 @@ sub _chain_relationship {
   # the join in question so we could tell it *is* the search_related)
   my $already_joined;
 
-
   # we consider the last one thus reverse
   for my $j (reverse @requested_joins) {
     if ($rel eq $j->[0]{-join_path}[-1]) {
@@ -2696,7 +2680,6 @@ sub _chain_relationship {
       last;
     }
   }
-
 # alternative way to scan the entire chain - not backwards compatible
 #  for my $j (reverse @$from) {
 #    next unless ref $j eq 'ARRAY';
index 292c943..69fa4ff 100644 (file)
@@ -45,7 +45,7 @@ sub check_cols_of {
             my @dbic_reltable = $dbic_obj->$col;
             my @hashref_reltable = @{$datahashref->{$col}};
   
-            is (scalar @hashref_reltable, scalar @dbic_reltable, 'number of related entries');
+            is (scalar @dbic_reltable, scalar @hashref_reltable, 'number of related entries');
 
             # for my $index (0..scalar @hashref_reltable) {
             for my $index (0..scalar @dbic_reltable) {
index 6142098..d82f4c4 100644 (file)
@@ -27,7 +27,7 @@ is_same_sql(
       single_track_2.trackid, single_track_2.cd, single_track_2.position, single_track_2.title, single_track_2.last_updated_on, single_track_2.last_updated_at, single_track_2.small_dt,
       cd.cdid, cd.artist, cd.title, cd.year, cd.genreid, cd.single_track
     FROM artist me
-      LEFT JOIN cd cds ON cds.artist = me.artistid
+      JOIN cd cds ON cds.artist = me.artistid
       LEFT JOIN track single_track ON single_track.trackid = cds.single_track
       LEFT JOIN track single_track_2 ON single_track_2.trackid = cds.single_track
       LEFT JOIN cd cd ON cd.cdid = single_track_2.cd
index 90e49a3..62776fa 100644 (file)
@@ -268,7 +268,7 @@ is_same_sql_bind (
   '(
     SELECT artist_undirected_maps.id1, artist_undirected_maps.id2
       FROM artist me
-      LEFT JOIN artist_undirected_map artist_undirected_maps
+      JOIN artist_undirected_map artist_undirected_maps
         ON artist_undirected_maps.id1 = me.artistid OR artist_undirected_maps.id2 = me.artistid
     WHERE ( artistid = ? )
   )',
diff --git a/t/resultset/nulls_only.t b/t/resultset/nulls_only.t
new file mode 100644 (file)
index 0000000..facf299
--- /dev/null
@@ -0,0 +1,29 @@
+use strict;
+use warnings;
+
+use lib qw(t/lib);
+use Test::More;
+use Test::Exception;
+use DBICTest;
+
+my $schema = DBICTest->init_schema();
+
+
+my $cd_rs = $schema->resultset('CD')->search ({ genreid => undef }, { columns => [ 'genreid' ]} );
+my $count = $cd_rs->count;
+cmp_ok ( $count, '>', 1, 'several CDs with no genre');
+
+my @objects = $cd_rs->all;
+is (scalar @objects, $count, 'Correct amount of objects without limit');
+isa_ok ($_, 'DBICTest::CD') for @objects;
+
+is_deeply (
+  [ map { values %{{$_->get_columns}} } (@objects) ],
+  [ (undef) x $count ],
+  'All values are indeed undef'
+);
+
+
+isa_ok ($cd_rs->search ({}, { rows => 1 })->single, 'DBICTest::CD');
+
+done_testing;
index 10621ae..c65e696 100644 (file)
@@ -31,7 +31,7 @@ is_same_sql_bind (
       ) 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
+      JOIN tags tags ON tags.cd = me.cdid
     WHERE ( tags.tag IS NOT NULL )
     GROUP BY tags.tagid, tags.cd, tags.tag
   )',