Extract two stateless DBIHacks routines into a utility package
Peter Rabbitson [Wed, 10 Aug 2016 15:16:32 +0000 (17:16 +0200)]
Further commits will need them in places where $storage isn't yet available.
There are zero functional changes

Best read under -C -C -M --color-words

lib/DBIx/Class/ResultSet.pm
lib/DBIx/Class/ResultSource.pm
lib/DBIx/Class/SQLMaker/OracleJoins.pm
lib/DBIx/Class/SQLMaker/Util.pm [new file with mode: 0644]
lib/DBIx/Class/Storage/DBIHacks.pm
xt/extra/internals/namespaces_cleaned.t
xt/extra/internals/sqla_condition_parsers.t [moved from t/sqlmaker/dbihacks_internals.t with 64% similarity]

index 3d06065..97cfe50 100644 (file)
@@ -13,6 +13,7 @@ use DBIx::Class::_Util qw(
   dbic_internal_try dump_value
   fail_on_internal_wantarray fail_on_internal_call UNRESOLVABLE_CONDITION
 );
+use DBIx::Class::SQLMaker::Util qw( normalize_sqla_condition extract_equality_conditions );
 use Try::Tiny;
 
 BEGIN {
@@ -662,7 +663,7 @@ sub _stack_cond {
     return undef
   }
   else {
-    return $self->result_source->schema->storage->_collapse_cond({ -and => [$left, $right] });
+    return normalize_sqla_condition({ -and => [$left, $right] });
   }
 }
 
@@ -2618,7 +2619,7 @@ sub _merge_with_rscond {
     @cols_from_relations = keys %{ $implied_data || {} };
   }
   else {
-    my $eqs = $self->result_source->schema->storage->_extract_fixed_condition_columns($self->{cond}, 'consider_nulls');
+    my $eqs = extract_equality_conditions( $self->{cond}, 'consider_nulls' );
     $implied_data = { map {
       ( ($eqs->{$_}||'') eq UNRESOLVABLE_CONDITION ) ? () : ( $_ => $eqs->{$_} )
     } keys %$eqs };
index 4d33970..c598b1d 100644 (file)
@@ -21,6 +21,7 @@ use DBIx::Class::_Util qw(
   dbic_internal_try fail_on_internal_call
   refdesc emit_loud_diag
 );
+use DBIx::Class::SQLMaker::Util qw( normalize_sqla_condition extract_equality_conditions );
 use SQL::Abstract 'is_literal_value';
 use Devel::GlobalDestruction;
 use Scalar::Util qw( blessed weaken isweak refaddr );
@@ -1928,7 +1929,7 @@ sub _minimal_valueset_satisfying_constraint {
 
   $args->{columns_info} ||= $self->columns_info;
 
-  my $vals = $self->schema->storage->_extract_fixed_condition_columns(
+  my $vals = extract_equality_conditions(
     $args->{values},
     ($args->{carp_on_nulls} ? 'consider_nulls' : undef ),
   );
@@ -1942,7 +1943,7 @@ sub _minimal_valueset_satisfying_constraint {
       $cols->{$args->{carp_on_nulls} ? 'undefined' : 'missing'}{$col} = undef;
     }
     else {
-      # we need to inject back the '=' as _extract_fixed_condition_columns
+      # we need to inject back the '=' as extract_equality_conditions()
       # will strip it from literals and values alike, resulting in an invalid
       # condition in the end
       $cols->{present}{$col} = { '=' => $vals->{$col} };
@@ -2304,7 +2305,7 @@ sub _resolve_relationship_condition {
           qw( columns relationships )
         ;
 
-        my $equivalencies = $storage->_extract_fixed_condition_columns(
+        my $equivalencies = extract_equality_conditions(
           $args->{foreign_values},
           'consider nulls',
         );
@@ -2520,10 +2521,10 @@ sub _resolve_relationship_condition {
       and
     $ret->{join_free_condition} ne UNRESOLVABLE_CONDITION
       and
-    my $jfc = $storage->_collapse_cond( $ret->{join_free_condition} )
+    my $jfc = normalize_sqla_condition( $ret->{join_free_condition} )
   ) {
 
-    my $jfc_eqs = $storage->_extract_fixed_condition_columns($jfc, 'consider_nulls');
+    my $jfc_eqs = extract_equality_conditions( $jfc, 'consider_nulls' );
 
     if (keys %$jfc_eqs) {
 
@@ -2563,7 +2564,7 @@ sub _resolve_relationship_condition {
   # (may already be there, since easy to calculate on the fly in the HASH case)
   if ( ! $ret->{identity_map} ) {
 
-    my $col_eqs = $storage->_extract_fixed_condition_columns($ret->{condition});
+    my $col_eqs = extract_equality_conditions($ret->{condition});
 
     my $colinfos;
     for my $lhs (keys %$col_eqs) {
index 0f50467..00e58fb 100644 (file)
@@ -81,8 +81,8 @@ sub _recurse_oracle_joins {
     }
 
     # FIXME - the code below *UTTERLY* doesn't work with custom conds... sigh
-    # for the time being do not do any processing with the likes of _collapse_cond
-    # instead only unroll the -and hack if present
+    # for the time being do not do any processing with the likes of
+    # normalize_sqla_condition(), instead only unroll the -and hack if present
     $on = $on->{-and}[0] if (
       ref $on eq 'HASH'
         and
diff --git a/lib/DBIx/Class/SQLMaker/Util.pm b/lib/DBIx/Class/SQLMaker/Util.pm
new file mode 100644 (file)
index 0000000..ec83edd
--- /dev/null
@@ -0,0 +1,447 @@
+package   #hide from PAUSE
+  DBIx::Class::SQLMaker::Util;
+
+use strict;
+use warnings;
+
+use base 'Exporter';
+our @EXPORT_OK = qw(
+  normalize_sqla_condition
+  extract_equality_conditions
+);
+
+use DBIx::Class::Carp;
+use Carp 'croak';
+use SQL::Abstract qw( is_literal_value is_plain_value );
+use DBIx::Class::_Util qw( UNRESOLVABLE_CONDITION serialize dump_value );
+
+
+# Attempts to flatten a passed in SQLA condition as much as possible towards
+# a plain hashref, *without* altering its semantics.
+#
+# FIXME - while relatively robust, this is still imperfect, one of the first
+# things to tackle when we get access to a formalized AST. Note that this code
+# is covered by a *ridiculous* amount of tests, so starting with porting this
+# code would be a rather good exercise
+sub normalize_sqla_condition {
+  my ($where, $where_is_anded_array) = @_;
+
+  my $fin;
+
+  if (! $where) {
+    return;
+  }
+  elsif ($where_is_anded_array or ref $where eq 'HASH') {
+
+    my @pairs;
+
+    my @pieces = $where_is_anded_array ? @$where : $where;
+    while (@pieces) {
+      my $chunk = shift @pieces;
+
+      if (ref $chunk eq 'HASH') {
+        for (sort keys %$chunk) {
+
+          # Match SQLA 1.79 behavior
+          unless( length $_ ) {
+            is_literal_value($chunk->{$_})
+              ? carp 'Hash-pairs consisting of an empty string with a literal are deprecated, use -and => [ $literal ] instead'
+              : croak 'Supplying an empty left hand side argument is not supported in hash-pairs'
+            ;
+          }
+
+          push @pairs, $_ => $chunk->{$_};
+        }
+      }
+      elsif (ref $chunk eq 'ARRAY') {
+        push @pairs, -or => $chunk
+          if @$chunk;
+      }
+      elsif ( ! length ref $chunk) {
+
+        # Match SQLA 1.79 behavior
+        croak("Supplying an empty left hand side argument is not supported in array-pairs")
+          if $where_is_anded_array and (! defined $chunk or ! length $chunk);
+
+        push @pairs, $chunk, shift @pieces;
+      }
+      else {
+        push @pairs, '', $chunk;
+      }
+    }
+
+    return unless @pairs;
+
+    my @conds = _normalize_cond_unroll_pairs(\@pairs)
+      or return;
+
+    # Consolidate various @conds back into something more compact
+    for my $c (@conds) {
+      if (ref $c ne 'HASH') {
+        push @{$fin->{-and}}, $c;
+      }
+      else {
+        for my $col (sort keys %$c) {
+
+          # consolidate all -and nodes
+          if ($col =~ /^\-and$/i) {
+            push @{$fin->{-and}},
+              ref $c->{$col} eq 'ARRAY' ? @{$c->{$col}}
+            : ref $c->{$col} eq 'HASH' ? %{$c->{$col}}
+            : { $col => $c->{$col} }
+            ;
+          }
+          elsif ($col =~ /^\-/) {
+            push @{$fin->{-and}}, { $col => $c->{$col} };
+          }
+          elsif (exists $fin->{$col}) {
+            $fin->{$col} = [ -and => map {
+              (ref $_ eq 'ARRAY' and ($_->[0]||'') =~ /^\-and$/i )
+                ? @{$_}[1..$#$_]
+                : $_
+              ;
+            } ($fin->{$col}, $c->{$col}) ];
+          }
+          else {
+            $fin->{$col} = $c->{$col};
+          }
+        }
+      }
+    }
+  }
+  elsif (ref $where eq 'ARRAY') {
+    # we are always at top-level here, it is safe to dump empty *standalone* pieces
+    my $fin_idx;
+
+    for (my $i = 0; $i <= $#$where; $i++ ) {
+
+      # Match SQLA 1.79 behavior
+      croak(
+        "Supplying an empty left hand side argument is not supported in array-pairs"
+      ) if (! defined $where->[$i] or ! length $where->[$i]);
+
+      my $logic_mod = lc ( ($where->[$i] =~ /^(\-(?:and|or))$/i)[0] || '' );
+
+      if ($logic_mod) {
+        $i++;
+        croak("Unsupported top-level op/arg pair: [ $logic_mod => $where->[$i] ]")
+          unless ref $where->[$i] eq 'HASH' or ref $where->[$i] eq 'ARRAY';
+
+        my $sub_elt = normalize_sqla_condition({ $logic_mod => $where->[$i] })
+          or next;
+
+        my @keys = keys %$sub_elt;
+        if ( @keys == 1 and $keys[0] !~ /^\-/ ) {
+          $fin_idx->{ "COL_$keys[0]_" . serialize $sub_elt } = $sub_elt;
+        }
+        else {
+          $fin_idx->{ "SER_" . serialize $sub_elt } = $sub_elt;
+        }
+      }
+      elsif (! length ref $where->[$i] ) {
+        my $sub_elt = normalize_sqla_condition({ @{$where}[$i, $i+1] })
+          or next;
+
+        $fin_idx->{ "COL_$where->[$i]_" . serialize $sub_elt } = $sub_elt;
+        $i++;
+      }
+      else {
+        $fin_idx->{ "SER_" . serialize $where->[$i] } = normalize_sqla_condition( $where->[$i] ) || next;
+      }
+    }
+
+    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 = { -and => [ $where ] };
+  }
+
+  # unroll single-element -and's
+  while (
+    $fin->{-and}
+      and
+    @{$fin->{-and}} < 2
+  ) {
+    my $and = delete $fin->{-and};
+    last if @$and == 0;
+
+    # at this point we have @$and == 1
+    if (
+      ref $and->[0] eq 'HASH'
+        and
+      ! grep { exists $fin->{$_} } keys %{$and->[0]}
+    ) {
+      $fin = {
+        %$fin, %{$and->[0]}
+      };
+    }
+    else {
+      $fin->{-and} = $and;
+      last;
+    }
+  }
+
+  # compress same-column conds found in $fin
+  for my $col ( grep { $_ !~ /^\-/ } keys %$fin ) {
+    next unless ref $fin->{$col} eq 'ARRAY' and ($fin->{$col}[0]||'') =~ /^\-and$/i;
+    my $val_bag = { map {
+      (! defined $_ )                          ? ( UNDEF => undef )
+    : ( ! length ref $_ or is_plain_value $_ ) ? ( "VAL_$_" => $_ )
+    : ( ( 'SER_' . serialize $_ ) => $_ )
+    } @{$fin->{$col}}[1 .. $#{$fin->{$col}}] };
+
+    if (keys %$val_bag == 1 ) {
+      ($fin->{$col}) = values %$val_bag;
+    }
+    else {
+      $fin->{$col} = [ -and => map { $val_bag->{$_} } sort keys %$val_bag ];
+    }
+  }
+
+  return keys %$fin ? $fin : ();
+}
+
+sub _normalize_cond_unroll_pairs {
+  my $pairs = shift;
+
+  my @conds;
+
+  while (@$pairs) {
+    my ($lhs, $rhs) = splice @$pairs, 0, 2;
+
+    if (! length $lhs) {
+      push @conds, normalize_sqla_condition($rhs);
+    }
+    elsif ( $lhs =~ /^\-and$/i ) {
+      push @conds, normalize_sqla_condition($rhs, (ref $rhs eq 'ARRAY'));
+    }
+    elsif ( $lhs =~ /^\-or$/i ) {
+      push @conds, normalize_sqla_condition(
+        (ref $rhs eq 'HASH') ? [ map { $_ => $rhs->{$_} } sort keys %$rhs ] : $rhs
+      );
+    }
+    else {
+      if (ref $rhs eq 'HASH' and ! keys %$rhs) {
+        # FIXME - SQLA seems to be doing... nothing...?
+      }
+      # normalize top level -ident, for saner extract_fixed_condition_columns code
+      elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{-ident}) {
+        push @conds, { $lhs => { '=', $rhs } };
+      }
+      elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{-value} and is_plain_value $rhs->{-value}) {
+        push @conds, { $lhs => $rhs->{-value} };
+      }
+      elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{'='}) {
+        if ( length ref $rhs->{'='} and is_literal_value $rhs->{'='} ) {
+          push @conds, { $lhs => $rhs };
+        }
+        else {
+          for my $p (_normalize_cond_unroll_pairs([ $lhs => $rhs->{'='} ])) {
+
+            # extra sanity check
+            if (keys %$p > 1) {
+              local $Data::Dumper::Deepcopy = 1;
+              croak(
+                "Internal error: unexpected collapse unroll:"
+              . dump_value { in => { $lhs => $rhs }, out => $p }
+              );
+            }
+
+            my ($l, $r) = %$p;
+
+            push @conds, (
+              ! length ref $r
+                or
+              # the unroller recursion may return a '=' prepended value already
+              ref $r eq 'HASH' and keys %$rhs == 1 and exists $rhs->{'='}
+                or
+              is_plain_value($r)
+            )
+              ? { $l => $r }
+              : { $l => { '=' => $r } }
+            ;
+          }
+        }
+      }
+      elsif (ref $rhs eq 'ARRAY') {
+        # some of these conditionals encounter multi-values - roll them out using
+        # an unshift, which will cause extra looping in the while{} above
+        if (! @$rhs ) {
+          push @conds, { $lhs => [] };
+        }
+        elsif ( ($rhs->[0]||'') =~ /^\-(?:and|or)$/i ) {
+          croak("Value modifier not followed by any values: $lhs => [ $rhs->[0] ] ")
+            if @$rhs == 1;
+
+          if( $rhs->[0] =~ /^\-and$/i ) {
+            unshift @$pairs, map { $lhs => $_ } @{$rhs}[1..$#$rhs];
+          }
+          # if not an AND then it's an OR
+          elsif(@$rhs == 2) {
+            unshift @$pairs, $lhs => $rhs->[1];
+          }
+          else {
+            push @conds, { $lhs => [ @{$rhs}[1..$#$rhs] ] };
+          }
+        }
+        elsif (@$rhs == 1) {
+          unshift @$pairs, $lhs => $rhs->[0];
+        }
+        else {
+          push @conds, { $lhs => $rhs };
+        }
+      }
+      # unroll func + { -value => ... }
+      elsif (
+        ref $rhs eq 'HASH'
+          and
+        ( my ($subop) = keys %$rhs ) == 1
+          and
+        length ref ((values %$rhs)[0])
+          and
+        my $vref = is_plain_value( (values %$rhs)[0] )
+      ) {
+        push @conds, { $lhs => { $subop => $$vref } }
+      }
+      else {
+        push @conds, { $lhs => $rhs };
+      }
+    }
+  }
+
+  return @conds;
+}
+
+# Analyzes a given condition and attempts to extract all columns
+# with a definitive fixed-condition criteria. Returns a hashref
+# of k/v pairs suitable to be passed to set_columns(), with a
+# MAJOR CAVEAT - multi-value (contradictory) equalities are still
+# represented as a reference to the UNRESOVABLE_CONDITION constant
+# The reason we do this is that some codepaths only care about the
+# codition being stable, as opposed to actually making sense
+#
+# The normal mode is used to figure out if a resultset is constrained
+# to a column which is part of a unique constraint, which in turn
+# allows us to better predict how ordering will behave etc.
+#
+# With the optional "consider_nulls" boolean argument, the function
+# is instead used to infer inambiguous values from conditions
+# (e.g. the inheritance of resultset conditions on new_result)
+#
+sub extract_equality_conditions {
+  my ($where, $consider_nulls) = @_;
+  my $where_hash = normalize_sqla_condition($where);
+
+  my $res = {};
+  my ($c, $v);
+  for $c (keys %$where_hash) {
+    my $vals;
+
+    if (!defined ($v = $where_hash->{$c}) ) {
+      $vals->{UNDEF} = $v if $consider_nulls
+    }
+    elsif (
+      ref $v eq 'HASH'
+        and
+      keys %$v == 1
+    ) {
+      if (exists $v->{-value}) {
+        if (defined $v->{-value}) {
+          $vals->{"VAL_$v->{-value}"} = $v->{-value}
+        }
+        elsif( $consider_nulls ) {
+          $vals->{UNDEF} = $v->{-value};
+        }
+      }
+      # do not need to check for plain values - normalize_sqla_condition did it for us
+      elsif(
+        length ref $v->{'='}
+          and
+        (
+          ( ref $v->{'='} eq 'HASH' and keys %{$v->{'='}} == 1 and exists $v->{'='}{-ident} )
+            or
+          is_literal_value($v->{'='})
+        )
+       ) {
+        $vals->{ 'SER_' . serialize $v->{'='} } = $v->{'='};
+      }
+    }
+    elsif (
+      ! length ref $v
+        or
+      is_plain_value ($v)
+    ) {
+      $vals->{"VAL_$v"} = $v;
+    }
+    elsif (ref $v eq 'ARRAY' and ($v->[0]||'') eq '-and') {
+      for ( @{$v}[1..$#$v] ) {
+        my $subval = extract_equality_conditions({ $c => $_ }, 'consider nulls');  # always fish nulls out on recursion
+        next unless exists $subval->{$c};  # didn't find anything
+        $vals->{
+          ! defined $subval->{$c}                                        ? 'UNDEF'
+        : ( ! length ref $subval->{$c} or is_plain_value $subval->{$c} ) ? "VAL_$subval->{$c}"
+        : ( 'SER_' . serialize $subval->{$c} )
+        } = $subval->{$c};
+      }
+    }
+
+    if (keys %$vals == 1) {
+      ($res->{$c}) = (values %$vals)
+        unless !$consider_nulls and exists $vals->{UNDEF};
+    }
+    elsif (keys %$vals > 1) {
+      $res->{$c} = UNRESOLVABLE_CONDITION;
+    }
+  }
+
+  $res;
+}
+
+1;
index c700d54..317dbd8 100644 (file)
@@ -5,7 +5,7 @@ package   #hide from PAUSE
 # This module contains code supporting a battery of special cases and tests for
 # many corner cases pushing the envelope of what DBIC can do. When work on
 # these utilities began in mid 2009 (51a296b402c) it wasn't immediately obvious
-# that these pieces, despite their misleading on-first-sighe-flakiness, will
+# that these pieces, despite their misleading on-first-sight-flakiness, will
 # become part of the generic query rewriting machinery of DBIC, allowing it to
 # both generate and process queries representing incredibly complex sets with
 # reasonable efficiency.
@@ -29,8 +29,10 @@ use base 'DBIx::Class::Storage';
 use mro 'c3';
 
 use Scalar::Util 'blessed';
-use DBIx::Class::_Util qw(UNRESOLVABLE_CONDITION serialize dump_value);
-use SQL::Abstract qw(is_plain_value is_literal_value);
+use DBIx::Class::_Util qw(
+  dump_value fail_on_internal_call
+);
+use DBIx::Class::SQLMaker::Util 'extract_equality_conditions';
 use DBIx::Class::Carp;
 use namespace::clean;
 
@@ -992,7 +994,7 @@ sub _order_by_is_stable {
 
   my @cols = (
     ( map { $_->[0] } $self->_extract_order_criteria($order_by) ),
-    ( $where ? keys %{ $self->_extract_fixed_condition_columns($where) } : () ),
+    ( $where ? keys %{ extract_equality_conditions( $where ) } : () ),
   ) or return 0;
 
   my $colinfo = $self->_resolve_column_info($ident, \@cols);
@@ -1057,9 +1059,9 @@ sub _extract_colinfo_of_stable_main_source_order_by_portion {
       if $colinfo->{-source_alias} eq $attrs->{alias};
   }
 
-  # FIXME the condition may be singling out things on its own, so we
-  # conceivable could come back wi "stable-ordered by nothing"
-  # not confient enough in the parser yet, so punt for the time being
+  # FIXME: the condition may be singling out things on its own, so we
+  # conceivably could come back with "stable-ordered by nothing"
+  # not confident enough in the parser yet, so punt for the time being
   return unless $seen_main_src_cols;
 
   my $main_src_fixed_cols_from_cond = [ $attrs->{where}
@@ -1070,7 +1072,7 @@ sub _extract_colinfo_of_stable_main_source_order_by_portion {
           ? $colinfos->{$_}{-colname}
           : ()
       }
-      keys %{ $self->_extract_fixed_condition_columns($attrs->{where}) }
+      keys %{ extract_equality_conditions( $attrs->{where} ) }
     )
     : ()
   ];
@@ -1081,434 +1083,20 @@ sub _extract_colinfo_of_stable_main_source_order_by_portion {
   ]) ? $colinfos_to_return : ();
 }
 
-# Attempts to flatten a passed in SQLA condition as much as possible towards
-# a plain hashref, *without* altering its semantics. Required by
-# create/populate being able to extract definitive conditions from preexisting
-# resultset {where} stacks
-#
-# FIXME - while relatively robust, this is still imperfect, one of the first
-# things to tackle when we get access to a formalized AST. Note that this code
-# is covered by a *ridiculous* amount of tests, so starting with porting this
-# code would be a rather good exercise
-sub _collapse_cond {
-  my ($self, $where, $where_is_anded_array) = @_;
-
-  my $fin;
-
-  if (! $where) {
-    return;
-  }
-  elsif ($where_is_anded_array or ref $where eq 'HASH') {
-
-    my @pairs;
-
-    my @pieces = $where_is_anded_array ? @$where : $where;
-    while (@pieces) {
-      my $chunk = shift @pieces;
-
-      if (ref $chunk eq 'HASH') {
-        for (sort keys %$chunk) {
-
-          # Match SQLA 1.79 behavior
-          unless( length $_ ) {
-            is_literal_value($chunk->{$_})
-              ? carp 'Hash-pairs consisting of an empty string with a literal are deprecated, use -and => [ $literal ] instead'
-              : $self->throw_exception("Supplying an empty left hand side argument is not supported in hash-pairs")
-            ;
-          }
-
-          push @pairs, $_ => $chunk->{$_};
-        }
-      }
-      elsif (ref $chunk eq 'ARRAY') {
-        push @pairs, -or => $chunk
-          if @$chunk;
-      }
-      elsif ( ! length ref $chunk) {
-
-        # Match SQLA 1.79 behavior
-        $self->throw_exception("Supplying an empty left hand side argument is not supported in array-pairs")
-          if $where_is_anded_array and (! defined $chunk or ! length $chunk);
-
-        push @pairs, $chunk, shift @pieces;
-      }
-      else {
-        push @pairs, '', $chunk;
-      }
-    }
-
-    return unless @pairs;
-
-    my @conds = $self->_collapse_cond_unroll_pairs(\@pairs)
-      or return;
-
-    # Consolidate various @conds back into something more compact
-    for my $c (@conds) {
-      if (ref $c ne 'HASH') {
-        push @{$fin->{-and}}, $c;
-      }
-      else {
-        for my $col (sort keys %$c) {
-
-          # consolidate all -and nodes
-          if ($col =~ /^\-and$/i) {
-            push @{$fin->{-and}},
-              ref $c->{$col} eq 'ARRAY' ? @{$c->{$col}}
-            : ref $c->{$col} eq 'HASH' ? %{$c->{$col}}
-            : { $col => $c->{$col} }
-            ;
-          }
-          elsif ($col =~ /^\-/) {
-            push @{$fin->{-and}}, { $col => $c->{$col} };
-          }
-          elsif (exists $fin->{$col}) {
-            $fin->{$col} = [ -and => map {
-              (ref $_ eq 'ARRAY' and ($_->[0]||'') =~ /^\-and$/i )
-                ? @{$_}[1..$#$_]
-                : $_
-              ;
-            } ($fin->{$col}, $c->{$col}) ];
-          }
-          else {
-            $fin->{$col} = $c->{$col};
-          }
-        }
-      }
-    }
-  }
-  elsif (ref $where eq 'ARRAY') {
-    # we are always at top-level here, it is safe to dump empty *standalone* pieces
-    my $fin_idx;
-
-    for (my $i = 0; $i <= $#$where; $i++ ) {
-
-      # Match SQLA 1.79 behavior
-      $self->throw_exception(
-        "Supplying an empty left hand side argument is not supported in array-pairs"
-      ) if (! defined $where->[$i] or ! length $where->[$i]);
-
-      my $logic_mod = lc ( ($where->[$i] =~ /^(\-(?:and|or))$/i)[0] || '' );
-
-      if ($logic_mod) {
-        $i++;
-        $self->throw_exception("Unsupported top-level op/arg pair: [ $logic_mod => $where->[$i] ]")
-          unless ref $where->[$i] eq 'HASH' or ref $where->[$i] eq 'ARRAY';
-
-        my $sub_elt = $self->_collapse_cond({ $logic_mod => $where->[$i] })
-          or next;
-
-        my @keys = keys %$sub_elt;
-        if ( @keys == 1 and $keys[0] !~ /^\-/ ) {
-          $fin_idx->{ "COL_$keys[0]_" . serialize $sub_elt } = $sub_elt;
-        }
-        else {
-          $fin_idx->{ "SER_" . serialize $sub_elt } = $sub_elt;
-        }
-      }
-      elsif (! length ref $where->[$i] ) {
-        my $sub_elt = $self->_collapse_cond({ @{$where}[$i, $i+1] })
-          or next;
-
-        $fin_idx->{ "COL_$where->[$i]_" . serialize $sub_elt } = $sub_elt;
-        $i++;
-      }
-      else {
-        $fin_idx->{ "SER_" . serialize $where->[$i] } = $self->_collapse_cond( $where->[$i] ) || next;
-      }
-    }
-
-    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 = { -and => [ $where ] };
-  }
-
-  # unroll single-element -and's
-  while (
-    $fin->{-and}
-      and
-    @{$fin->{-and}} < 2
-  ) {
-    my $and = delete $fin->{-and};
-    last if @$and == 0;
-
-    # at this point we have @$and == 1
-    if (
-      ref $and->[0] eq 'HASH'
-        and
-      ! grep { exists $fin->{$_} } keys %{$and->[0]}
-    ) {
-      $fin = {
-        %$fin, %{$and->[0]}
-      };
-    }
-    else {
-      $fin->{-and} = $and;
-      last;
-    }
-  }
+sub _collapse_cond :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+  carp_unique("_collapse_cond() is deprecated, ask on IRC for a better alternative");
 
-  # compress same-column conds found in $fin
-  for my $col ( grep { $_ !~ /^\-/ } keys %$fin ) {
-    next unless ref $fin->{$col} eq 'ARRAY' and ($fin->{$col}[0]||'') =~ /^\-and$/i;
-    my $val_bag = { map {
-      (! defined $_ )                          ? ( UNDEF => undef )
-    : ( ! length ref $_ or is_plain_value $_ ) ? ( "VAL_$_" => $_ )
-    : ( ( 'SER_' . serialize $_ ) => $_ )
-    } @{$fin->{$col}}[1 .. $#{$fin->{$col}}] };
-
-    if (keys %$val_bag == 1 ) {
-      ($fin->{$col}) = values %$val_bag;
-    }
-    else {
-      $fin->{$col} = [ -and => map { $val_bag->{$_} } sort keys %$val_bag ];
-    }
-  }
-
-  return keys %$fin ? $fin : ();
+  shift;
+  DBIx::Class::SQLMaker::Util::normalize_sqla_condition(@_);
 }
 
-sub _collapse_cond_unroll_pairs {
-  my ($self, $pairs) = @_;
-
-  my @conds;
-
-  while (@$pairs) {
-    my ($lhs, $rhs) = splice @$pairs, 0, 2;
-
-    if (! length $lhs) {
-      push @conds, $self->_collapse_cond($rhs);
-    }
-    elsif ( $lhs =~ /^\-and$/i ) {
-      push @conds, $self->_collapse_cond($rhs, (ref $rhs eq 'ARRAY'));
-    }
-    elsif ( $lhs =~ /^\-or$/i ) {
-      push @conds, $self->_collapse_cond(
-        (ref $rhs eq 'HASH') ? [ map { $_ => $rhs->{$_} } sort keys %$rhs ] : $rhs
-      );
-    }
-    else {
-      if (ref $rhs eq 'HASH' and ! keys %$rhs) {
-        # FIXME - SQLA seems to be doing... nothing...?
-      }
-      # normalize top level -ident, for saner extract_fixed_condition_columns code
-      elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{-ident}) {
-        push @conds, { $lhs => { '=', $rhs } };
-      }
-      elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{-value} and is_plain_value $rhs->{-value}) {
-        push @conds, { $lhs => $rhs->{-value} };
-      }
-      elsif (ref $rhs eq 'HASH' and keys %$rhs == 1 and exists $rhs->{'='}) {
-        if ( length ref $rhs->{'='} and is_literal_value $rhs->{'='} ) {
-          push @conds, { $lhs => $rhs };
-        }
-        else {
-          for my $p ($self->_collapse_cond_unroll_pairs([ $lhs => $rhs->{'='} ])) {
-
-            # extra sanity check
-            if (keys %$p > 1) {
-              local $Data::Dumper::Deepcopy = 1;
-              $self->throw_exception(
-                "Internal error: unexpected collapse unroll:"
-              . dump_value { in => { $lhs => $rhs }, out => $p }
-              );
-            }
-
-            my ($l, $r) = %$p;
-
-            push @conds, (
-              ! length ref $r
-                or
-              # the unroller recursion may return a '=' prepended value already
-              ref $r eq 'HASH' and keys %$rhs == 1 and exists $rhs->{'='}
-                or
-              is_plain_value($r)
-            )
-              ? { $l => $r }
-              : { $l => { '=' => $r } }
-            ;
-          }
-        }
-      }
-      elsif (ref $rhs eq 'ARRAY') {
-        # some of these conditionals encounter multi-values - roll them out using
-        # an unshift, which will cause extra looping in the while{} above
-        if (! @$rhs ) {
-          push @conds, { $lhs => [] };
-        }
-        elsif ( ($rhs->[0]||'') =~ /^\-(?:and|or)$/i ) {
-          $self->throw_exception("Value modifier not followed by any values: $lhs => [ $rhs->[0] ] ")
-            if  @$rhs == 1;
-
-          if( $rhs->[0] =~ /^\-and$/i ) {
-            unshift @$pairs, map { $lhs => $_ } @{$rhs}[1..$#$rhs];
-          }
-          # if not an AND then it's an OR
-          elsif(@$rhs == 2) {
-            unshift @$pairs, $lhs => $rhs->[1];
-          }
-          else {
-            push @conds, { $lhs => [ @{$rhs}[1..$#$rhs] ] };
-          }
-        }
-        elsif (@$rhs == 1) {
-          unshift @$pairs, $lhs => $rhs->[0];
-        }
-        else {
-          push @conds, { $lhs => $rhs };
-        }
-      }
-      # unroll func + { -value => ... }
-      elsif (
-        ref $rhs eq 'HASH'
-          and
-        ( my ($subop) = keys %$rhs ) == 1
-          and
-        length ref ((values %$rhs)[0])
-          and
-        my $vref = is_plain_value( (values %$rhs)[0] )
-      ) {
-        push @conds, { $lhs => { $subop => $$vref } }
-      }
-      else {
-        push @conds, { $lhs => $rhs };
-      }
-    }
-  }
-
-  return @conds;
-}
-
-# Analyzes a given condition and attempts to extract all columns
-# with a definitive fixed-condition criteria. Returns a hashref
-# of k/v pairs suitable to be passed to set_columns(), with a
-# MAJOR CAVEAT - multi-value (contradictory) equalities are still
-# represented as a reference to the UNRESOVABLE_CONDITION constant
-# The reason we do this is that some codepaths only care about the
-# codition being stable, as opposed to actually making sense
-#
-# The normal mode is used to figure out if a resultset is constrained
-# to a column which is part of a unique constraint, which in turn
-# allows us to better predict how ordering will behave etc.
-#
-# With the optional "consider_nulls" boolean argument, the function
-# is instead used to infer inambiguous values from conditions
-# (e.g. the inheritance of resultset conditions on new_result)
-#
-sub _extract_fixed_condition_columns {
-  my ($self, $where, $consider_nulls) = @_;
-  my $where_hash = $self->_collapse_cond($_[1]);
-
-  my $res = {};
-  my ($c, $v);
-  for $c (keys %$where_hash) {
-    my $vals;
-
-    if (!defined ($v = $where_hash->{$c}) ) {
-      $vals->{UNDEF} = $v if $consider_nulls
-    }
-    elsif (
-      ref $v eq 'HASH'
-        and
-      keys %$v == 1
-    ) {
-      if (exists $v->{-value}) {
-        if (defined $v->{-value}) {
-          $vals->{"VAL_$v->{-value}"} = $v->{-value}
-        }
-        elsif( $consider_nulls ) {
-          $vals->{UNDEF} = $v->{-value};
-        }
-      }
-      # do not need to check for plain values - _collapse_cond did it for us
-      elsif(
-        length ref $v->{'='}
-          and
-        (
-          ( ref $v->{'='} eq 'HASH' and keys %{$v->{'='}} == 1 and exists $v->{'='}{-ident} )
-            or
-          is_literal_value($v->{'='})
-        )
-       ) {
-        $vals->{ 'SER_' . serialize $v->{'='} } = $v->{'='};
-      }
-    }
-    elsif (
-      ! length ref $v
-        or
-      is_plain_value ($v)
-    ) {
-      $vals->{"VAL_$v"} = $v;
-    }
-    elsif (ref $v eq 'ARRAY' and ($v->[0]||'') eq '-and') {
-      for ( @{$v}[1..$#$v] ) {
-        my $subval = $self->_extract_fixed_condition_columns({ $c => $_ }, 'consider nulls');  # always fish nulls out on recursion
-        next unless exists $subval->{$c};  # didn't find anything
-        $vals->{
-          ! defined $subval->{$c}                                        ? 'UNDEF'
-        : ( ! length ref $subval->{$c} or is_plain_value $subval->{$c} ) ? "VAL_$subval->{$c}"
-        : ( 'SER_' . serialize $subval->{$c} )
-        } = $subval->{$c};
-      }
-    }
-
-    if (keys %$vals == 1) {
-      ($res->{$c}) = (values %$vals)
-        unless !$consider_nulls and exists $vals->{UNDEF};
-    }
-    elsif (keys %$vals > 1) {
-      $res->{$c} = UNRESOLVABLE_CONDITION;
-    }
-  }
+sub _extract_fixed_condition_columns :DBIC_method_is_indirect_sugar {
+  DBIx::Class::_ENV_::ASSERT_NO_INTERNAL_INDIRECT_CALLS and fail_on_internal_call;
+  carp_unique("_extract_fixed_condition_columns() is deprecated, ask on IRC for a better alternative");
 
-  $res;
+  shift;
+  extract_equality_conditions(@_);
 }
 
 1;
index 2230957..b0a7cdb 100644 (file)
@@ -80,6 +80,7 @@ my $skip_idx = { map { $_ => 1 } (
   # utility classes, not part of the inheritance chain
   'DBIx::Class::Optional::Dependencies',
   'DBIx::Class::ResultSource::RowParser::Util',
+  'DBIx::Class::SQLMaker::Util',
   'DBIx::Class::_Util',
 ) };
 
similarity index 64%
rename from t/sqlmaker/dbihacks_internals.t
rename to xt/extra/internals/sqla_condition_parsers.t
index 4e34f13..5c94edc 100644 (file)
@@ -9,6 +9,7 @@ use Test::Exception;
 
 use DBICTest ':DiffSQL';
 use DBIx::Class::_Util qw( UNRESOLVABLE_CONDITION dump_value );
+use DBIx::Class::SQLMaker::Util qw( normalize_sqla_condition extract_equality_conditions );
 
 BEGIN {
   if ( eval { require Test::Differences } ) {
@@ -17,8 +18,7 @@ BEGIN {
   }
 }
 
-my $schema = DBICTest->init_schema( no_deploy => 1);
-my $sm = $schema->storage->sql_maker;
+my $sm = DBICTest->init_schema( no_deploy => 1)->storage->sql_maker;
 
 {
   package # hideee
@@ -37,95 +37,95 @@ is("$num", 69, 'test overloaded object is "sane"');
 my @tests = (
   {
     where => { artistid => 1, charfield => undef },
-    cc_result => { artistid => 1, charfield => undef },
+    normalized => { artistid => 1, charfield => undef },
     sql => 'WHERE artistid = ? AND charfield IS NULL',
-    efcc_result => { artistid => 1 },
-    efcc_n_result => { artistid => 1, charfield => undef },
+    equality_extract => { artistid => 1 },
+    equality_considering_nulls_extract => { artistid => 1, charfield => undef },
   },
   {
     where => { -and => [ artistid => 1, charfield => undef, { rank => 13 } ] },
-    cc_result => { artistid => 1, charfield => undef, rank => 13 },
+    normalized => { artistid => 1, charfield => undef, rank => 13 },
     sql => 'WHERE artistid = ?  AND charfield IS NULL AND rank = ?',
-    efcc_result => { artistid => 1, rank => 13 },
-    efcc_n_result => { artistid => 1, charfield => undef, rank => 13 },
+    equality_extract => { artistid => 1, rank => 13 },
+    equality_considering_nulls_extract => { artistid => 1, charfield => undef, rank => 13 },
   },
   {
     where => { -and => [ { artistid => 1, charfield => undef}, { rank => 13 } ] },
-    cc_result => { artistid => 1, charfield => undef, rank => 13 },
+    normalized => { artistid => 1, charfield => undef, rank => 13 },
     sql => 'WHERE artistid = ?  AND charfield IS NULL AND rank = ?',
-    efcc_result => { artistid => 1, rank => 13 },
-    efcc_n_result => { artistid => 1, charfield => undef, rank => 13 },
+    equality_extract => { artistid => 1, rank => 13 },
+    equality_considering_nulls_extract => { artistid => 1, charfield => undef, rank => 13 },
   },
   {
     where => { -and => [ -or => { name => 'Caterwauler McCrae' }, 'rank' ] },
-    cc_result => { name => 'Caterwauler McCrae', rank => undef },
+    normalized => { name => 'Caterwauler McCrae', rank => undef },
     sql => 'WHERE name = ? AND rank IS NULL',
-    efcc_result => { name => 'Caterwauler McCrae' },
-    efcc_n_result => { name => 'Caterwauler McCrae', rank => undef },
+    equality_extract => { name => 'Caterwauler McCrae' },
+    equality_considering_nulls_extract => { name => 'Caterwauler McCrae', rank => undef },
   },
   {
     where => { -and => [ [ [ artist => {'=' => \'foo' } ] ], { name => \[ '= ?', 'bar' ] } ] },
-    cc_result => { artist => {'=' => \'foo' }, name => \[ '= ?', 'bar' ] },
+    normalized => { artist => {'=' => \'foo' }, name => \[ '= ?', 'bar' ] },
     sql => 'WHERE artist = foo AND name = ?',
-    efcc_result => { artist => \'foo' },
+    equality_extract => { artist => \'foo' },
   },
   {
     where => { -and => [ -or => { name => 'Caterwauler McCrae', artistid => 2 } ] },
-    cc_result => { -or => [ artistid => 2, name => 'Caterwauler McCrae' ] },
+    normalized => { -or => [ artistid => 2, name => 'Caterwauler McCrae' ] },
     sql => 'WHERE artistid = ? OR name = ?',
-    efcc_result => {},
+    equality_extract => {},
   },
   {
     where => { -or => { name => 'Caterwauler McCrae', artistid => 2 } },
-    cc_result => { -or => [ artistid => 2, name => 'Caterwauler McCrae' ] },
+    normalized => { -or => [ artistid => 2, name => 'Caterwauler McCrae' ] },
     sql => 'WHERE artistid = ? OR name = ?',
-    efcc_result => {},
+    equality_extract => {},
   },
   {
     where => { -and => [ \'foo=bar',  [ { artistid => { '=', $num } } ], { name => 'Caterwauler McCrae'} ] },
-    cc_result => { -and => [ \'foo=bar' ], name => 'Caterwauler McCrae', artistid => $num },
+    normalized => { -and => [ \'foo=bar' ], name => 'Caterwauler McCrae', artistid => $num },
     sql => 'WHERE foo=bar AND artistid = ? AND name = ?',
-    efcc_result => { name => 'Caterwauler McCrae', artistid => $num },
+    equality_extract => { 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 },
+    normalized => { -and => [ \'foo=bar', \'buzz=bozz' ], name => 'Caterwauler McCrae', artistid => $num },
+    sql =>            'WHERE foo=bar AND artistid = ? AND name = ? AND buzz=bozz',
+    normalized_sql => 'WHERE foo=bar AND buzz=bozz AND artistid = ? AND name = ?',
+    equality_extract => { name => 'Caterwauler McCrae', artistid => $num },
   },
   {
     where => { artistid => [ $num ], rank => [ 13, 2, 3 ], charfield => [ undef ] },
-    cc_result => { artistid => $num, charfield => undef, rank => [13, 2, 3] },
+    normalized => { artistid => $num, charfield => undef, rank => [13, 2, 3] },
     sql => 'WHERE artistid = ? AND charfield IS NULL AND ( rank = ? OR rank = ? OR rank = ? )',
-    efcc_result => { artistid => $num },
-    efcc_n_result => { artistid => $num, charfield => undef },
+    equality_extract => { artistid => $num },
+    equality_considering_nulls_extract => { artistid => $num, charfield => undef },
   },
   {
     where => { artistid => { '=' => 1 }, rank => { '>' => 12 }, charfield => { '=' => undef } },
-    cc_result => { artistid => 1, charfield => undef, rank => { '>' => 12 } },
+    normalized => { artistid => 1, charfield => undef, rank => { '>' => 12 } },
     sql => 'WHERE artistid = ? AND charfield IS NULL AND rank > ?',
-    efcc_result => { artistid => 1 },
-    efcc_n_result => { artistid => 1, charfield => undef },
+    equality_extract => { artistid => 1 },
+    equality_considering_nulls_extract => { artistid => 1, charfield => undef },
   },
   {
     where => { artistid => { '=' => [ 1 ], }, charfield => { '=' => [ -AND => \'1', \['?',2] ] }, rank => { '=' => [ -OR => $num, $num ] } },
-    cc_result => { artistid => 1, charfield => [-and => { '=' => \['?',2] }, { '=' => \'1' } ], rank => { '=' => [$num, $num] } },
-    sql => 'WHERE artistid = ? AND charfield = 1 AND charfield = ? AND ( rank = ? OR rank = ? )',
-    collapsed_sql => 'WHERE artistid = ? AND charfield = ? AND charfield = 1 AND ( rank = ? OR rank = ? )',
-    efcc_result => { artistid => 1, charfield => UNRESOLVABLE_CONDITION },
+    normalized => { artistid => 1, charfield => [-and => { '=' => \['?',2] }, { '=' => \'1' } ], rank => { '=' => [$num, $num] } },
+    sql =>            'WHERE artistid = ? AND charfield = 1 AND charfield = ? AND ( rank = ? OR rank = ? )',
+    normalized_sql => 'WHERE artistid = ? AND charfield = ? AND charfield = 1 AND ( rank = ? OR rank = ? )',
+    equality_extract => { artistid => 1, charfield => UNRESOLVABLE_CONDITION },
   },
   {
     where => { -and => [ artistid => 1, artistid => 2 ], name => [ -and => { '!=', 1 }, 2 ], charfield => [ -or => { '=', 2 } ], rank => [-and => undef, { '=', undef }, { '!=', 2 } ] },
-    cc_result => { artistid => [ -and => 1, 2 ], name => [ -and => { '!=', 1 }, 2 ], charfield => 2, rank => [ -and => { '!=', 2 }, undef ] },
-    sql => 'WHERE artistid = ? AND artistid = ? AND charfield = ? AND name != ? AND name = ? AND rank IS NULL AND rank IS NULL AND rank != ?',
-    collapsed_sql => 'WHERE artistid = ? AND artistid = ? AND charfield = ? AND name != ? AND name = ? AND rank != ? AND rank IS NULL',
-    efcc_result => {
+    normalized => { artistid => [ -and => 1, 2 ], name => [ -and => { '!=', 1 }, 2 ], charfield => 2, rank => [ -and => { '!=', 2 }, undef ] },
+    sql =>            'WHERE artistid = ? AND artistid = ? AND charfield = ? AND name != ? AND name = ? AND rank IS NULL AND rank IS NULL AND rank != ?',
+    normalized_sql => 'WHERE artistid = ? AND artistid = ? AND charfield = ? AND name != ? AND name = ? AND rank != ? AND rank IS NULL',
+    equality_extract => {
       artistid => UNRESOLVABLE_CONDITION,
       name => 2,
       charfield => 2,
     },
-    efcc_n_result => {
+    equality_considering_nulls_extract => {
       artistid => UNRESOLVABLE_CONDITION,
       name => 2,
       charfield => 2,
@@ -134,14 +134,14 @@ my @tests = (
   },
   (map { {
     where => $_,
-    sql => 'WHERE (rank = 13 OR charfield IS NULL OR artistid = ?) AND (artistid = ? OR charfield IS NULL OR rank != 42)',
-    collapsed_sql => 'WHERE (artistid = ? OR charfield IS NULL OR rank = 13) AND (artistid = ? OR charfield IS NULL OR rank != 42)',
-    cc_result => { -and => [
+    sql =>            'WHERE (rank = 13 OR charfield IS NULL OR artistid = ?) AND (artistid = ? OR charfield IS NULL OR rank != 42)',
+    normalized_sql => 'WHERE (artistid = ? OR charfield IS NULL OR rank = 13) AND (artistid = ? OR charfield IS NULL OR rank != 42)',
+    normalized => { -and => [
       { -or => [ artistid => 1, charfield => undef, rank => { '=' => \13 } ] },
       { -or => [ artistid => 1, charfield => undef, rank => { '!=' => \42 } ] },
     ] },
-    efcc_result => {},
-    efcc_n_result => {},
+    equality_extract => {},
+    equality_considering_nulls_extract => {},
   } } (
 
     { -and => [
@@ -162,37 +162,37 @@ my @tests = (
       baz => { '!=' => { -ident => 'bozz' } },
       baz => { -ident => 'buzz' },
     ] },
-    sql => 'WHERE ( foo IS NOT NULL AND bar IN ( ?, ? ) ) OR foo IS NULL OR baz != bozz OR baz = buzz',
-    collapsed_sql => 'WHERE baz != bozz OR baz = buzz OR foo IS NULL OR ( bar IN ( ?, ? ) AND foo IS NOT NULL )',
-    cc_result => { -or => [
+    sql =>            'WHERE ( foo IS NOT NULL AND bar IN ( ?, ? ) ) OR foo IS NULL OR baz != bozz OR baz = buzz',
+    normalized_sql => 'WHERE baz != bozz OR baz = buzz OR foo IS NULL OR ( bar IN ( ?, ? ) AND foo IS NOT NULL )',
+    normalized => { -or => [
       baz => { '!=' => { -ident => 'bozz' } },
       baz => { '=' => { -ident => 'buzz' } },
       foo => undef,
       { bar => { -in => [ 69, 42 ] }, foo => { '!=', undef } }
     ] },
-    efcc_result => {},
+    equality_extract => {},
   },
   {
     where => { -or => [ rank => { '=' => \13 }, charfield => { '=' => undef }, artistid => { '=' => 1 }, genreid => { '=' => \['?', 2] } ] },
-    sql => 'WHERE rank = 13 OR charfield IS NULL OR artistid = ? OR genreid = ?',
-    collapsed_sql => 'WHERE artistid = ? OR charfield IS NULL OR genreid = ? OR rank = 13',
-    cc_result => { -or => [ artistid => 1, charfield => undef, genreid => { '=' => \['?', 2] }, rank => { '=' => \13 } ] },
-    efcc_result => {},
-    efcc_n_result => {},
+    sql =>            'WHERE rank = 13 OR charfield IS NULL OR artistid = ? OR genreid = ?',
+    normalized_sql => 'WHERE artistid = ? OR charfield IS NULL OR genreid = ? OR rank = 13',
+    normalized => { -or => [ artistid => 1, charfield => undef, genreid => { '=' => \['?', 2] }, rank => { '=' => \13 } ] },
+    equality_extract => {},
+    equality_considering_nulls_extract => {},
   },
   {
     where => { -and => [
       -or => [ rank => { '=' => \13 }, charfield => { '=' => undef }, artistid => 1 ],
       -or => { artistid => { '=' => 1 }, charfield => undef, rank => { '=' => \13 } },
     ] },
-    cc_result => { -and => [
+    normalized => { -and => [
       { -or => [ artistid => 1, charfield => undef, rank => { '=' => \13 } ] },
       { -or => [ artistid => 1, charfield => undef, rank => { '=' => \13 } ] },
     ] },
-    sql => 'WHERE (rank = 13 OR charfield IS NULL OR artistid = ?) AND (artistid = ? OR charfield IS NULL OR rank = 13)',
-    collapsed_sql => 'WHERE (artistid = ? OR charfield IS NULL OR rank = 13) AND (artistid = ? OR charfield IS NULL OR rank = 13)',
-    efcc_result => {},
-    efcc_n_result => {},
+    sql =>            'WHERE (rank = 13 OR charfield IS NULL OR artistid = ?) AND (artistid = ? OR charfield IS NULL OR rank = 13)',
+    normalized_sql => 'WHERE (artistid = ? OR charfield IS NULL OR rank = 13) AND (artistid = ? OR charfield IS NULL OR rank = 13)',
+    equality_extract => {},
+    equality_considering_nulls_extract => {},
   },
   {
     where => { -and => [
@@ -217,7 +217,7 @@ my @tests = (
       AND NOT foo = ?
       AND NOT foo = ?
     ',
-    collapsed_sql => 'WHERE
+    normalized_sql => 'WHERE
           ( artistid = ? OR charfield IS NULL OR rank = 13 )
       AND ( artistid = ? OR charfield IS NULL OR rank != 42 )
       AND (EXISTS (SELECT 1))
@@ -229,7 +229,7 @@ my @tests = (
       AND foo = 1
       AND foo = ?
     ',
-    cc_result => {
+    normalized => {
       -and => [
         { -or => [ artistid => 1, charfield => undef, rank => { '=' => \13 } ] },
         { -or => [ artistid => 1, charfield => undef, rank => { '!=' => \42 } ] },
@@ -241,11 +241,11 @@ my @tests = (
       foo => [ -and => { '=' => \1 }, 3 ],
       bar => [ -and => { '=' => \4 }, 2 ],
     },
-    efcc_result => {
+    equality_extract => {
       foo => UNRESOLVABLE_CONDITION,
       bar => UNRESOLVABLE_CONDITION,
     },
-    efcc_n_result => {
+    equality_considering_nulls_extract => {
       foo => UNRESOLVABLE_CONDITION,
       bar => UNRESOLVABLE_CONDITION,
     },
@@ -255,7 +255,7 @@ my @tests = (
       [ '_macro.to' => { -like => '%correct%' }, '_wc_macros.to' => { -like => '%correct%' } ],
       { -and => [ { 'group.is_active' => 1 }, { 'me.is_active' => 1 } ] }
     ] },
-    cc_result => {
+    normalized => {
       'group.is_active' => 1,
       'me.is_active' => 1,
       -or => [
@@ -264,7 +264,7 @@ my @tests = (
       ],
     },
     sql => 'WHERE ( _macro.to LIKE ? OR _wc_macros.to LIKE ? ) AND group.is_active = ? AND me.is_active = ?',
-    efcc_result => { 'group.is_active' => 1, 'me.is_active' => 1 },
+    equality_extract => { 'group.is_active' => 1, 'me.is_active' => 1 },
   },
 
   {
@@ -275,18 +275,18 @@ my @tests = (
       rank => { '=' => { -ident => 'bar' } },
     ] },
     sql => 'WHERE artistid = ? AND charfield = foo AND name IS NULL AND rank = bar',
-    cc_result => {
+    normalized => {
       artistid => { -value => [1] },
       name => undef,
       charfield => { '=', { -ident => 'foo' } },
       rank => { '=' => { -ident => 'bar' } },
     },
-    efcc_result => {
+    equality_extract => {
       artistid => [1],
       charfield => { -ident => 'foo' },
       rank => { -ident => 'bar' },
     },
-    efcc_n_result => {
+    equality_considering_nulls_extract => {
       artistid => [1],
       name => undef,
       charfield => { -ident => 'foo' },
@@ -296,40 +296,40 @@ my @tests = (
 
   {
     where => { artistid => [] },
-    cc_result => { artistid => [] },
-    efcc_result => {},
+    normalized => { artistid => [] },
+    equality_extract => {},
   },
   (map {
     {
       where => { -and => $_ },
-      cc_result => undef,
-      efcc_result => {},
+      normalized => undef,
+      equality_extract => {},
       sql => '',
     },
     {
       where => { -or => $_ },
-      cc_result => undef,
-      efcc_result => {},
+      normalized => undef,
+      equality_extract => {},
       sql => '',
     },
     {
       where => { -or => [ foo => 1, $_ ] },
-      cc_result => { foo => 1 },
-      efcc_result => { foo => 1 },
+      normalized => { foo => 1 },
+      equality_extract => { foo => 1 },
       sql => 'WHERE foo = ?',
     },
     {
       where => { -or => [ $_, foo => 1 ] },
-      cc_result => { foo => 1 },
-      efcc_result => { foo => 1 },
+      normalized => { foo => 1 },
+      equality_extract => { foo => 1 },
       sql => 'WHERE foo = ?',
     },
     {
       where => { -and => [ fuu => 2, $_, foo => 1 ] },
-      sql => 'WHERE fuu = ? AND foo = ?',
-      collapsed_sql => 'WHERE foo = ? AND fuu = ?',
-      cc_result => { foo => 1, fuu => 2 },
-      efcc_result => { foo => 1, fuu => 2 },
+      sql =>            'WHERE fuu = ? AND foo = ?',
+      normalized_sql => 'WHERE foo = ? AND fuu = ?',
+      normalized => { foo => 1, fuu => 2 },
+      equality_extract => { foo => 1, fuu => 2 },
     },
   } (
     # bare
@@ -343,16 +343,16 @@ my @tests = (
   )),
 
   # FIXME legacy compat crap, possibly worth undef/dieing in SQLMaker
-  { where => { artistid => {} }, sql => '', cc_result => undef, efcc_result => {}, efcc_n_result => {} },
+  { where => { artistid => {} }, sql => '', normalized => undef, equality_extract => {}, equality_considering_nulls_extract => {} },
 
   # batshit insanity, just to be thorough
   {
     where => { -and => [ [ 'artistid' ], [ -and => [ artistid => { '!=', 69 }, artistid => undef, artistid => { '=' => 200 } ]], artistid => [], { -or => [] }, { -and => [] }, [ 'charfield' ], { name => [] }, 'rank' ] },
-    cc_result => { artistid => [ -and => [], { '!=', 69 }, undef, 200  ], charfield => undef, name => [], rank => undef },
-    sql => 'WHERE artistid IS NULL AND artistid != ? AND artistid IS NULL AND artistid = ? AND 0=1 AND charfield IS NULL AND 0=1 AND rank IS NULL',
-    collapsed_sql => 'WHERE 0=1 AND artistid != ? AND artistid IS NULL AND artistid = ? AND charfield IS NULL AND 0=1 AND rank IS NULL',
-    efcc_result => { artistid => UNRESOLVABLE_CONDITION },
-    efcc_n_result => { artistid => UNRESOLVABLE_CONDITION, charfield => undef, rank => undef },
+    normalized => { artistid => [ -and => [], { '!=', 69 }, undef, 200  ], charfield => undef, name => [], rank => undef },
+    sql =>            'WHERE artistid IS NULL AND artistid != ? AND artistid IS NULL AND artistid = ? AND 0=1 AND charfield IS NULL AND 0=1 AND rank IS NULL',
+    normalized_sql => 'WHERE 0=1 AND artistid != ? AND artistid IS NULL AND artistid = ? AND charfield IS NULL AND 0=1 AND rank IS NULL',
+    equality_extract => { artistid => UNRESOLVABLE_CONDITION },
+    equality_considering_nulls_extract => { artistid => UNRESOLVABLE_CONDITION, charfield => undef, rank => undef },
   },
 
   # original test from RT#93244
@@ -365,7 +365,7 @@ my @tests = (
         ],
         [ { 'me.title' => 'Spoonful of bees' } ],
     ]},
-    cc_result => {
+    normalized => {
       -and => [ \[
         "LOWER(me.title) LIKE ?",
         '%spoon%',
@@ -373,7 +373,7 @@ my @tests = (
       'me.title' => 'Spoonful of bees',
     },
     sql => 'WHERE LOWER(me.title) LIKE ? AND me.title = ?',
-    efcc_result => { 'me.title' => 'Spoonful of bees' },
+    equality_extract => { 'me.title' => 'Spoonful of bees' },
   },
 
   # crazy literals
@@ -384,12 +384,12 @@ my @tests = (
       ],
     },
     sql => 'WHERE foo = bar',
-    cc_result => {
+    normalized => {
       -and => [
         \'foo = bar',
       ],
     },
-    efcc_result => {},
+    equality_extract => {},
   },
   {
     where => {
@@ -398,15 +398,15 @@ my @tests = (
         \'baz = ber',
       ],
     },
-    sql => 'WHERE foo = bar OR baz = ber',
-    collapsed_sql => 'WHERE baz = ber OR foo = bar',
-    cc_result => {
+    sql =>            'WHERE foo = bar OR baz = ber',
+    normalized_sql => 'WHERE baz = ber OR foo = bar',
+    normalized => {
       -or => [
         \'baz = ber',
         \'foo = bar',
       ],
     },
-    efcc_result => {},
+    equality_extract => {},
   },
   {
     where => {
@@ -416,13 +416,13 @@ my @tests = (
       ],
     },
     sql => 'WHERE foo = bar AND baz = ber',
-    cc_result => {
+    normalized => {
       -and => [
         \'foo = bar',
         \'baz = ber',
       ],
     },
-    efcc_result => {},
+    equality_extract => {},
   },
   {
     where => {
@@ -433,14 +433,14 @@ my @tests = (
       ],
     },
     sql => 'WHERE foo = bar AND baz = ber AND x = y',
-    cc_result => {
+    normalized => {
       -and => [
         \'foo = bar',
         \'baz = ber',
       ],
       x => { '=' => { -ident => 'y' } }
     },
-    efcc_result => { x => { -ident => 'y' } },
+    equality_extract => { x => { -ident => 'y' } },
   },
 );
 
@@ -495,8 +495,8 @@ for my $lhs (undef, '') {
 
     push @tests, {
       where => { $lhs => $rhs },
-      cc_result => { -and => [ $rhs ] },
-      efcc_result => {},
+      normalized => { -and => [ $rhs ] },
+      equality_extract => {},
       sql => 'WHERE baz',
       warn => $expected_warning,
     };
@@ -507,12 +507,12 @@ for my $lhs (undef, '') {
     ) {
       push @tests, {
         where => $w,
-        cc_result => {
+        normalized => {
           -and => [ $rhs ],
           bizz => "buzz",
           foo => "bar",
         },
-        efcc_result => {
+        equality_extract => {
           foo => "bar",
           bizz => "buzz",
         },
@@ -539,12 +539,12 @@ for my $eq (
   ) {
     push @tests, {
       where => $where,
-      cc_result => {
+      normalized => {
         0 => $eq,
         foo => 'bar',
         bizz => 'buzz',
       },
-      efcc_result => {
+      equality_extract => {
         foo => 'bar',
         bizz => 'buzz',
         ( ref $eq eq 'HASH' ? ( 0 => $eq->{'='} ) : () ),
@@ -554,12 +554,12 @@ for my $eq (
 
     push @tests, {
       where => { -or => $where },
-      cc_result => { -or => [
+      normalized => { -or => [
         "0" => $eq,
         bizz => 'buzz',
         foo => 'bar',
       ]},
-      efcc_result => {},
+      equality_extract => {},
       sql => 'WHERE 0 = baz OR bizz = ? OR foo = ?',
     }
 
@@ -574,14 +574,14 @@ for my $eq (
   ) {
     push @tests, {
       where => { -or => $where },
-      cc_result => { -or => [
+      normalized => { -or => [
         "0" => $eq,
         bizz => 'buzz',
         foo => 'bar',
       ]},
-      efcc_result => {},
-      sql => 'WHERE foo = ? OR 0 = baz OR bizz = ?',
-      collapsed_sql => 'WHERE 0 = baz OR bizz = ? OR foo = ?',
+      equality_extract => {},
+      sql =>            'WHERE foo = ? OR 0 = baz OR bizz = ?',
+      normalized_sql => 'WHERE 0 = baz OR bizz = ? OR foo = ?',
     }
   }
 
@@ -591,14 +591,14 @@ for my $eq (
   ) {
     push @tests, {
       where => { -or => $where },
-      cc_result => { -or => [
+      normalized => { -or => [
         "0" => 'baz',
         bizz => 'buzz',
         foo => 'bar',
       ]},
-      efcc_result => {},
-      sql => 'WHERE foo = ? OR 0 = ? OR bizz = ?',
-      collapsed_sql => 'WHERE 0 = ? OR bizz = ? OR foo = ?',
+      equality_extract => {},
+      sql =>            'WHERE foo = ? OR 0 = ? OR bizz = ?',
+      normalized_sql => 'WHERE 0 = ? OR bizz = ? OR foo = ?',
     };
   }
 
@@ -627,25 +627,24 @@ for my $t (@tests) {
 
     my $name = do { local $Data::Dumper::Indent = 0; dump_value $w };
 
-    my ($collapsed_cond, $collapsed_cond_as_sql);
+    my ($normalized_cond, $normalized_cond_as_sql);
 
     if ($t->{throw}) {
       throws_ok {
-        $collapsed_cond = $schema->storage->_collapse_cond($w);
-        ($collapsed_cond_as_sql) = $sm->where($collapsed_cond);
+        $sm->where( normalize_sqla_condition($w) );
       } $t->{throw}, "Exception on attempted collapse/render of $name"
         and
       next;
     }
 
     warnings_exist {
-      $collapsed_cond = $schema->storage->_collapse_cond($w);
-      ($collapsed_cond_as_sql) = $sm->where($collapsed_cond);
+      $normalized_cond = normalize_sqla_condition($w);
+      ($normalized_cond_as_sql) = $sm->where($normalized_cond);
     } $t->{warn} || [], "Expected warning when collapsing/rendering $name";
 
     is_deeply(
-      $collapsed_cond,
-      $t->{cc_result},
+      $normalized_cond,
+      $t->{normalized},
       "Expected collapsed condition produced on $name",
     );
 
@@ -658,27 +657,27 @@ for my $t (@tests) {
       if exists $t->{sql};
 
     is_same_sql(
-      $collapsed_cond_as_sql,
-      ( $t->{collapsed_sql} || $t->{sql} || $original_sql ),
-      "Collapse did not alter *the semantics* of the final SQL based on $name",
+      $normalized_cond_as_sql,
+      ( $t->{normalized_sql} || $t->{sql} || $original_sql ),
+      "Normalization did not alter *the semantics* of the final SQL based on $name",
     );
 
     is_deeply(
-      $schema->storage->_extract_fixed_condition_columns($collapsed_cond),
-      $t->{efcc_result},
-      "Expected fixed_condition produced on $name",
+      extract_equality_conditions($normalized_cond),
+      $t->{equality_extract},
+      "Expected equality_conditions produced on $name",
     );
 
     is_deeply(
-      $schema->storage->_extract_fixed_condition_columns($collapsed_cond, 'consider_nulls'),
-      $t->{efcc_n_result},
-      "Expected fixed_condition including NULLs produced on $name",
-    ) if $t->{efcc_n_result};
+      extract_equality_conditions($normalized_cond, 'consider_nulls'),
+      ( $t->{equality_considering_nulls_extract} || $t->{equality_extract} ),
+      "Expected equality_conditions including NULLs produced on $name",
+    );
 
     is_deeply(
-      $collapsed_cond,
-      $t->{cc_result},
-      "Collapsed condition result unaltered by fixed condition extractor",
+      $normalized_cond,
+      $t->{normalized},
+      "Collapsed condition result unaltered by equality conditions extractor",
     );
   }
 }