Sort hash keys so that the SQL produced is stable

diff --git a/lib/SQL/Abstract/Compat.pm b/lib/SQL/Abstract/Compat.pm
index 3928a78..5158b32 100644 (file)
--- a/lib/SQL/Abstract/Compat.pm
+++ b/lib/SQL/Abstract/Compat.pm
@@ -121,7 +121,9 @@ class SQL::Abstract::Compat {
       args => \@args
     };
 
-    while (my ($key,$value) = each %$ast) {
+    for my $key ( sort keys %$ast ) {
+      my $value = $ast->{$key};
+
       if ($key =~ /^-(or|and)$/) {
         my $val = $self->recurse_where($value, uc $1);
         if ($val->{op} eq $ret->{op}) {

diff --git a/t/compat/ast/01.t b/t/compat/ast/01.t
index 55e49c7..8a376c3 100644 (file)
--- a/t/compat/ast/01.t
+++ b/t/compat/ast/01.t
@@ -1,6 +1,10 @@
 use strict;
 use warnings;
 
+use FindBin;
+use lib "$FindBin::Bin/../../lib";
+use SQLADumperSort;
+
 use SQL::Abstract::Compat;
 
 use Test::More tests => 12;
@@ -134,6 +138,7 @@ my $worker_eq = sub {
     ],
   }
 };
+
 eq_or_diff
   $visitor->recurse_where( {
     requestor => 'inna',
@@ -143,6 +148,7 @@ eq_or_diff
   { -type => 'expr',
     op => 'and',
     args => [
+      field_op_value(qw/requestor == inna/),
       field_op_value(qw/status != completed/), 
       { -type => 'expr',
         op => 'or',
@@ -152,12 +158,65 @@ eq_or_diff
           field_op_value(qw/worker == sfz/), 
         ]
       },
-      field_op_value(qw/requestor == inna/),
     ]
   },
-  "complex expr #1";
+  "complex expr 1";
 
 
+=for comment
+$visitor->convert('UPPER');
+
+eq_or_diff
+  $visitor->select_ast(
+    'test', '*', [ { ticket => [11, 12, 13] } ]
+  ),
+  { -type => 'select',
+    columns => [ { -type => 'name', args => ['*'] } ],
+    tablespec => { -type => 'name', args => ['test'] },
+    where =>
+      { -type => 'expr', op => 'or', args => [
+        field_op_value( upper(mk_name('ticket')), '==', upper(mk_value(11))),
+        field_op_value( upper(mk_name('ticket')), '==', upper(mk_value(12))),
+        field_op_value( upper(mk_name('ticket')), '==', upper(mk_value(13))),
+      ] }
+  },
+  "Complex AST with convert('UPPER')";
+
+eq_or_diff
+  $visitor->select_ast(
+    'test', '*', [ { ticket => [11, 12, 13], 
+                     hostname => { in => ['ntf', 'avd', 'bvd', '123'] } },
+                  #{ tack => { between => [qw/tick tock/] } },
+                  #{ a => [qw/b c d/], 
+                  #  e => { '!=', [qw(f g)] }, 
+                  #  q => { 'not in', [14..20] } 
+                  #}
+                 ]
+  ),
+  { -type => 'select',
+    columns => [ { -type => 'name', args => ['*'] } ],
+    tablespec => { -type => 'name', args => ['test'] },
+    where =>
+      { -type => 'expr', op => 'or', args => [
+        { -type => 'expr', op => 'and', args => [
+          field_op_value( upper(mk_name('hostname')), 
+                          in => [ 
+                            upper(mk_value('nft')),
+                            upper(mk_value('avd')),
+                            upper(mk_value('bvd')),
+                            upper(mk_value('123')),
+                          ]
+                        ),
+          { -type => 'expr', op => 'or', args => [
+            field_op_value( upper(mk_name('ticket')), '==', upper(mk_value(11))),
+            field_op_value( upper(mk_name('ticket')), '==', upper(mk_value(12))),
+            field_op_value( upper(mk_name('ticket')), '==', upper(mk_value(13))),
+          ] }
+        ] }
+    ] }
+  },
+  "Complex AST with convert('UPPER')";
+=cut
 
 sub field_op_value {
   my ($field, $op, $value) = @_;
@@ -168,16 +227,44 @@ sub field_op_value {
          ? { -type => 'name', args => $field } 
          : { -type => 'name', args => [$field] };
 
-  $value = ref $value eq 'HASH'
-         ? $value
-         : { -type => 'value', value => $value };
+  my @value = ref $value eq 'HASH'
+            ? $value
+            : ref $value eq 'ARRAY'
+            ? @$value
+            : { -type => 'value', value => $value };
 
   return {
     -type => 'expr',
     op => $op,
     args => [
       $field,
-      $value
+      @value
     ]
   };
 }
+
+sub upper { expr(UPPER => @_) }
+
+sub expr {
+  my ($op, @args) = @_;
+
+  return {
+    -type => 'expr',
+    op => $op,
+    args => [@args]
+  };
+}
+
+sub mk_name {
+  my ($field) = @_;
+  $field = ref $field eq 'HASH'
+         ? $field
+         : ref $field eq 'ARRAY' 
+         ? { -type => 'name', args => $field } 
+         : { -type => 'name', args => [$field] };
+  return $field;
+}
+
+sub mk_value {
+  return { -type => 'value', value => $_[0] }
+}

diff --git a/t/lib/SQLADumperSort.pm b/t/lib/SQLADumperSort.pm
new file mode 100644 (file)
index 0000000..52e6e3f
--- /dev/null
+++ b/t/lib/SQLADumperSort.pm
@@ -0,0 +1,27 @@
+BEGIN {
+  require Data::Dumper;
+  my $Dump = Data::Dumper->can('Dump');
+
+  no warnings 'redefine';
+
+  *Data::Dumper::Dump = sub {
+    local $Data::Dumper::Sortkeys = sub {
+      my $hash = @_[0];
+      my @keys = sort { 
+        my $a_minus = substr($a,0,1) eq '-';
+        my $b_minus = substr($b,0,1) eq '-';
+
+        return $a cmp $b if $a_minus || $b_minus;
+
+        return -1 if $a eq 'op';
+        return  1 if $b eq 'op';
+        return $a cmp $b;
+      } keys %$hash;
+
+      return \@keys;
+    };
+    return $Dump->(@_);
+  };
+}
+
+1;