From: Peter Rabbitson <ribasushi@cpan.org>
Date: Tue, 16 Sep 2014 06:01:11 +0000 (+0200)
Subject: Yet another loose end on the cond collapser
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=07add744fd8b328dbc83f2a0906aaf6bd0b25674;p=dbsrgits%2FDBIx-Class-Historic.git

Yet another loose end on the cond collapser

Fingercross this is the last one... this transform is turning out ot be a
bad bad bad idea :(((
---

diff --git a/lib/DBIx/Class/Storage/DBIHacks.pm b/lib/DBIx/Class/Storage/DBIHacks.pm
index 17fdbfb..4c0da07 100644
--- a/lib/DBIx/Class/Storage/DBIHacks.pm
+++ b/lib/DBIx/Class/Storage/DBIHacks.pm
@@ -1083,20 +1083,57 @@ sub _collapse_cond {
       }
     }
 
-    return unless $fin_idx;
-
-    $fin = ( keys %$fin_idx == 1 ) ? (values %$fin_idx)[0] : {
-      -or => [ map {
-        # unroll single-element hashes
-        ( ref $fin_idx->{$_} eq 'HASH' and keys %{$fin_idx->{$_}} == 1 )
-          ? %{$fin_idx->{$_}}
-          : $fin_idx->{$_}
-      } sort keys %$fin_idx ]
-    };
+    if (! $fin_idx) {
+      return;
+    }
+    elsif ( keys %$fin_idx == 1 ) {
+      $fin = (values %$fin_idx)[0];
+    }
+    else {
+      my @or;
+
+      # at this point everything is at most one level deep - unroll if needed
+      for (sort keys %$fin_idx) {
+        if ( ref $fin_idx->{$_} eq 'HASH' and keys %{$fin_idx->{$_}} == 1 ) {
+          my ($l, $r) = %{$fin_idx->{$_}};
+
+          if (
+            ref $r eq 'ARRAY'
+              and
+            (
+              ( @$r == 1 and $l =~ /^\-and$/i )
+                or
+              $l =~ /^\-or$/i
+            )
+          ) {
+            push @or, @$r
+          }
+
+          elsif (
+            ref $r eq 'HASH'
+              and
+            keys %$r == 1
+              and
+            $l =~ /^\-(?:and|or)$/i
+          ) {
+            push @or, %$r;
+          }
+
+          else {
+            push @or, $l, $r;
+          }
+        }
+        else {
+          push @or, $fin_idx->{$_};
+        }
+      }
+
+      $fin->{-or} = \@or;
+    }
   }
   else {
     # not a hash not an array
-    $fin = { '' => $where };
+    $fin = { -and => [ $where ] };
   }
 
   # unroll single-element -and's
@@ -1118,6 +1155,10 @@ sub _collapse_cond {
         %$fin, %{$and->[0]}
       };
     }
+    else {
+      $fin->{-and} = $and;
+      last;
+    }
   }
 
   # compress same-column conds found in $fin
diff --git a/t/sqlmaker/dbihacks_internals.t b/t/sqlmaker/dbihacks_internals.t
index b635d6f..cd229fd 100644
--- a/t/sqlmaker/dbihacks_internals.t
+++ b/t/sqlmaker/dbihacks_internals.t
@@ -81,11 +81,18 @@ for my $t (
   },
   {
     where => { -and => [ \'foo=bar',  [ { artistid => { '=', $num } } ], { name => 'Caterwauler McCrae'} ] },
-    cc_result => { '' => \'foo=bar', name => 'Caterwauler McCrae', artistid => $num },
+    cc_result => { -and => [ \'foo=bar' ], name => 'Caterwauler McCrae', artistid => $num },
     sql => 'WHERE foo=bar AND artistid = ? AND name = ?',
     efcc_result => { name => 'Caterwauler McCrae', artistid => $num },
   },
   {
+    where => { -and => [ \'foo=bar',  [ { artistid => { '=', $num } } ], { name => 'Caterwauler McCrae'}, \'buzz=bozz' ] },
+    cc_result => { -and => [ \'foo=bar', \'buzz=bozz' ], name => 'Caterwauler McCrae', artistid => $num },
+    sql => 'WHERE foo=bar AND artistid = ? AND name = ? AND buzz=bozz',
+    collapsed_sql => 'WHERE foo=bar AND buzz=bozz AND artistid = ? AND name = ?',
+    efcc_result => { name => 'Caterwauler McCrae', artistid => $num },
+  },
+  {
     where => { artistid => [ $num ], rank => [ 13, 2, 3 ], charfield => [ undef ] },
     cc_result => { artistid => $num, charfield => undef, rank => [13, 2, 3] },
     sql => 'WHERE artistid = ? AND charfield IS NULL AND ( rank = ? OR rank = ? OR rank = ? )',
@@ -357,15 +364,82 @@ for my $t (
         [ { 'me.title' => 'Spoonful of bees' } ],
     ]},
     cc_result => {
-      '' => \[
+      -and => [ \[
         "LOWER(me.title) LIKE ?",
         '%spoon%',
-      ],
+      ]],
       'me.title' => 'Spoonful of bees',
     },
     sql => 'WHERE LOWER(me.title) LIKE ? AND me.title = ?',
     efcc_result => { 'me.title' => 'Spoonful of bees' },
-  }
+  },
+
+  # crazy literals
+  {
+    where => {
+      -or => [
+        \'foo = bar',
+      ],
+    },
+    sql => 'WHERE foo = bar',
+    cc_result => {
+      -and => [
+        \'foo = bar',
+      ],
+    },
+    efcc_result => {},
+  },
+  {
+    where => {
+      -or => [
+        \'foo = bar',
+        \'baz = ber',
+      ],
+    },
+    sql => 'WHERE foo = bar OR baz = ber',
+    collapsed_sql => 'WHERE baz = ber OR foo = bar',
+    cc_result => {
+      -or => [
+        \'baz = ber',
+        \'foo = bar',
+      ],
+    },
+    efcc_result => {},
+  },
+  {
+    where => {
+      -and => [
+        \'foo = bar',
+        \'baz = ber',
+      ],
+    },
+    sql => 'WHERE foo = bar AND baz = ber',
+    cc_result => {
+      -and => [
+        \'foo = bar',
+        \'baz = ber',
+      ],
+    },
+    efcc_result => {},
+  },
+  {
+    where => {
+      -and => [
+        \'foo = bar',
+        \'baz = ber',
+        x => { -ident => 'y' },
+      ],
+    },
+    sql => 'WHERE foo = bar AND baz = ber AND x = y',
+    cc_result => {
+      -and => [
+        \'foo = bar',
+        \'baz = ber',
+      ],
+      x => { '=' => { -ident => 'y' } }
+    },
+    efcc_result => { x => { -ident => 'y' } },
+  },
 ) {
 
   for my $w (
@@ -390,14 +464,14 @@ for my $t (
     is_same_sql ( $generated_sql, $t->{sql}, "Expected SQL from $name" )
       if exists $t->{sql};
 
-    my $collapsed_cond = $schema->storage->_collapse_cond($w);
-
     is_same_sql(
-      ($sm->where($collapsed_cond))[0],
+      ($sm->where($t->{cc_result}))[0],
       ( $t->{collapsed_sql} || $t->{sql} || $generated_sql ),
       "Collapse did not alter *the semantics* of the final SQL based on $name",
     );
 
+    my $collapsed_cond = $schema->storage->_collapse_cond($w);
+
     is_deeply(
       $collapsed_cond,
       $t->{cc_result},