[Rt-commit] rt branch, 4.0/joins-bundling, created. rt-4.0.11rc1-24-g794d3cc

Ruslan Zakirov ruz at bestpractical.com
Fri Apr 26 09:40:20 EDT 2013


The branch, 4.0/joins-bundling has been created
        at  794d3cc615035714795a1bf92bd955f0b278a7c7 (commit)

- Log -----------------------------------------------------------------
commit 38eab60e5115186a82fbec57ffdefb54d8395681
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Apr 4 20:03:48 2013 +0400

    bundle together watcher searches
    
    it's first attempt at wide bundling of watcher searches.
    So far it supports:
    
    * bundling of positive conditions (=, like, is null)
    * conditions can be mixed with conditions on other fields:
    
        (Cc.foo = a AND Subject = s) OR (Cc.foo = b AND Subject = s)
    
    The following cases are not bundled while in theory they could:
    * Cc = 'foo' OR Requestor = 'bar'
    * Cc != 'foo' AND Cc != 'bar'

diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index 3b834e0..416b4a8 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -939,13 +939,18 @@ sub _WatcherLimit {
     }
     $rest{SUBKEY} ||= 'EmailAddress';
 
-    my $groups = $self->_RoleGroupsJoin( Type => $type, Class => $class, New => !$type );
+    my ($groups, $group_members, $users);
+    if ( $rest{'BUNDLE'} ) {
+        ($groups, $group_members, $users) = @{ $rest{'BUNDLE'} };
+    } else {
+        $groups = $self->_RoleGroupsJoin( Type => $type, Class => $class, New => !$type );
+    }
 
     $self->_OpenParen;
     if ( $op =~ /^IS(?: NOT)?$/i ) {
         # is [not] empty case
 
-        my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
+        $group_members ||= $self->_GroupMembersJoin( GroupsAlias => $groups );
         # to avoid joining the table Users into the query, we just join GM
         # and make sure we don't match records where group is member of itself
         $self->SUPER::Limit(
@@ -983,7 +988,7 @@ sub _WatcherLimit {
         $users_obj->RowsPerPage(2);
         my @users = @{ $users_obj->ItemsArrayRef };
 
-        my $group_members = $self->_GroupMembersJoin( GroupsAlias => $groups );
+        $group_members ||= $self->_GroupMembersJoin( GroupsAlias => $groups );
         if ( @users <= 1 ) {
             my $uid = 0;
             $uid = $users[0]->id if @users;
@@ -1008,7 +1013,7 @@ sub _WatcherLimit {
                 VALUE      => "$group_members.MemberId",
                 QUOTEVALUE => 0,
             );
-            my $users = $self->Join(
+            $users ||= $self->Join(
                 TYPE            => 'LEFT',
                 ALIAS1          => $group_members,
                 FIELD1          => 'MemberId',
@@ -1034,10 +1039,10 @@ sub _WatcherLimit {
     } else {
         # positive condition case
 
-        my $group_members = $self->_GroupMembersJoin(
+        $group_members ||= $self->_GroupMembersJoin(
             GroupsAlias => $groups, New => 1, Left => 0
         );
-        my $users = $self->Join(
+        $users ||= $self->Join(
             TYPE            => 'LEFT',
             ALIAS1          => $group_members,
             FIELD1          => 'MemberId',
@@ -1054,6 +1059,7 @@ sub _WatcherLimit {
         );
     }
     $self->_CloseParen;
+    return ($groups, $group_members, $users);
 }
 
 sub _RoleGroupsJoin {
diff --git a/lib/RT/Tickets_SQL.pm b/lib/RT/Tickets_SQL.pm
index 608862a..ba47e46 100644
--- a/lib/RT/Tickets_SQL.pm
+++ b/lib/RT/Tickets_SQL.pm
@@ -171,19 +171,40 @@ sub _parser {
     my @bundle;
     my $ea = '';
 
+    my %sub_tree;
+    my $depth = 0;
+
     my %callback;
     $callback{'OpenParen'} = sub {
       $self->_close_bundle(@bundle); @bundle = ();
-      $self->_OpenParen
+      $self->_OpenParen;
+      $depth++;
+      push @$_, '(' foreach values %sub_tree;
     };
     $callback{'CloseParen'} = sub {
       $self->_close_bundle(@bundle); @bundle = ();
       $self->_CloseParen;
+      $depth--;
+      foreach my $list ( values %sub_tree ) {
+          if ( $list->[-1] eq '(' ) {
+              pop @$list;
+              pop @$list if $list->[-1] =~ /^(?:AND|OR)$/i;
+          }
+          else {
+              pop @$list while $list->[-2] ne '(';
+              $list->[-1] = pop @$list;
+          }
+      }
+    };
+    $callback{'EntryAggregator'} = sub {
+      $ea = $_[0] || '';
+      push @$_, $ea foreach values %sub_tree;
     };
-    $callback{'EntryAggregator'} = sub { $ea = $_[0] || '' };
     $callback{'Condition'} = sub {
         my ($key, $op, $value) = @_;
 
+        my ($negative_op, $null_op, $inv_op, $range_op)
+            = $self->ClassifySQLOperation( $op );
         # key has dot then it's compound variant and we have subkey
         my $subkey = '';
         ($key, $subkey) = ($1, $2) if $key =~ /^([^\.]+)\.(.+)$/;
@@ -225,10 +246,23 @@ sub _parser {
         }
         else {
             $self->_close_bundle(@bundle); @bundle = ();
-            $sub->( $self, $key, $op, $value,
+            my @res; my $bundle_with;
+            if ( $class eq 'WATCHERFIELD' && $key ne 'Owner' && !$negative_op && (!$null_op || $subkey) ) {
+                if ( !$sub_tree{$key} ) {
+                  $sub_tree{$key} = [ ('(')x$depth, \@res ];
+                } else {
+                  $bundle_with = $self->_check_bundling_possibility( $string, @{ $sub_tree{$key} } );
+                  if ( $sub_tree{$key}[-1] eq '(' ) {
+                        push @{ $sub_tree{$key} }, \@res;
+                  }
+                }
+            }
+            pop @$_ foreach grep @$_ && $_->[-1] =~ /^(?:AND|OR)$/i, values %sub_tree;
+            @res = $sub->( $self, $key, $op, $value,
                     SUBCLAUSE       => '',  # don't need anymore
                     ENTRYAGGREGATOR => $ea,
                     SUBKEY          => $subkey,
+                    BUNDLE          => $bundle_with,
                   );
         }
         $self->{_sql_looking_at}{lc $key} = 1;
@@ -238,6 +272,29 @@ sub _parser {
     $self->_close_bundle(@bundle); @bundle = ();
 }
 
+sub _check_bundling_possibility {
+    my $self = shift;
+    my $string = shift;
+    my @list = reverse @_;
+    while (my $e = shift @list) {
+        next if $e eq '(';
+        if ( lc($e) eq 'and' ) {
+            return undef;
+        }
+        elsif ( lc($e) eq 'or' ) {
+            return shift @list;
+        }
+        else {
+            # should not happen
+            $RT::Logger->error(
+                "Joins optimization failed when parsing '$string'. It's bug in RT, contact Best Practical"
+            );
+            die "Internal error. Contact your system administrator.";
+        }
+    }
+    return undef;
+}
+
 =head2 ClausesToSQL
 
 =cut

commit 794d3cc615035714795a1bf92bd955f0b278a7c7
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Mon Apr 8 21:10:25 2013 +0400

    uncomment heavy tests and put them under ENV variable
    
    instroduce $ENV{RT_TEST_HEAVY} that enables heavy tests.
    In this case these are 70k+ tests that test searches by
    watchers combined in different ways.

diff --git a/lib/RT/Tickets_SQL.pm b/lib/RT/Tickets_SQL.pm
index ba47e46..9202c69 100644
--- a/lib/RT/Tickets_SQL.pm
+++ b/lib/RT/Tickets_SQL.pm
@@ -198,7 +198,7 @@ sub _parser {
     };
     $callback{'EntryAggregator'} = sub {
       $ea = $_[0] || '';
-      push @$_, $ea foreach values %sub_tree;
+      push @$_, $ea foreach grep @$_ && $_->[-1] ne '(', values %sub_tree;
     };
     $callback{'Condition'} = sub {
         my ($key, $op, $value) = @_;
diff --git a/t/ticket/search_by_watcher.t b/t/ticket/search_by_watcher.t
index a6800bb..4fbc261 100644
--- a/t/ticket/search_by_watcher.t
+++ b/t/ticket/search_by_watcher.t
@@ -2,7 +2,7 @@
 use strict;
 use warnings;
 
-use RT::Test nodata => 1, tests => 2108;
+use RT::Test nodata => 1, tests => undef;
 use RT::Ticket;
 
 my $q = RT::Test->load_or_create_queue( Name => 'Regression' );
@@ -101,40 +101,41 @@ sub run_auto_tests {
             run_test( $query, %checks );
         } }
     }
-# XXX: It
-#    @queries = (
-#        '? AND ? AND ?'  => sub { $_[0] and $_[1] and $_[2] },
-#        '(? OR ?) AND ?' => sub { return (($_[0] or $_[1]) and $_[2]) },
-#        '? OR (? AND ?)' => sub { $_[0] or ($_[1] and $_[2]) },
-#        '(? AND ?) OR ?' => sub { ($_[0] and $_[1]) or $_[2] },
-#        '? AND (? OR ?)' => sub { $_[0] and ($_[1] or $_[2]) },
-#        '? OR ? OR ?'    => sub { $_[0] or $_[1] or $_[2] },
-#    );
-#    while ( my ($template, $t_cb) = splice @queries, 0, 2 ) {
-#        my @atmp = @conditions;
-#        while ( my ($a, $a_cb) = splice @atmp, 0, 2 ) {
-#        my @btmp = @conditions;
-#        while ( my ($b, $b_cb) = splice @btmp, 0, 2 ) {
-#        next if $a eq $b;
-#        my @ctmp = @conditions;
-#        while ( my ($c, $c_cb) = splice @ctmp, 0, 2 ) {
-#        next if $a eq $c;
-#        next if $b eq $c;
-#
-#            my %checks = ();
-#            foreach my $ticket ( @tickets ) {
-#                my $s = $ticket->Subject;
-#                $checks{ $s } = $t_cb->( scalar $a_cb->($s), scalar $b_cb->($s), scalar $c_cb->($s) );
-#            }
-#
-#            my $query = $template;
-#            foreach my $tmp ($a, $b, $c) {
-#                $query =~ s/\?/$tmp/;
-#            }
-#
-#            run_test( $query, %checks );
-#        } } }
-#    }
+    return unless $ENV{'RT_TEST_HEAVY'};
+
+    @queries = (
+        '? AND ? AND ?'  => sub { $_[0] and $_[1] and $_[2] },
+        '(? OR ?) AND ?' => sub { return (($_[0] or $_[1]) and $_[2]) },
+        '? OR (? AND ?)' => sub { $_[0] or ($_[1] and $_[2]) },
+        '(? AND ?) OR ?' => sub { ($_[0] and $_[1]) or $_[2] },
+        '? AND (? OR ?)' => sub { $_[0] and ($_[1] or $_[2]) },
+        '? OR ? OR ?'    => sub { $_[0] or $_[1] or $_[2] },
+    );
+    while ( my ($template, $t_cb) = splice @queries, 0, 2 ) {
+        my @atmp = @conditions;
+        while ( my ($a, $a_cb) = splice @atmp, 0, 2 ) {
+        my @btmp = @conditions;
+        while ( my ($b, $b_cb) = splice @btmp, 0, 2 ) {
+        next if $a eq $b;
+        my @ctmp = @conditions;
+        while ( my ($c, $c_cb) = splice @ctmp, 0, 2 ) {
+        next if $a eq $c;
+        next if $b eq $c;
+
+            my %checks = ();
+            foreach my $ticket ( @tickets ) {
+                my $s = $ticket->Subject;
+                $checks{ $s } = $t_cb->( scalar $a_cb->($s), scalar $b_cb->($s), scalar $c_cb->($s) );
+            }
+
+            my $query = $template;
+            foreach my $tmp ($a, $b, $c) {
+                $query =~ s/\?/$tmp/;
+            }
+
+            run_test( $query, %checks );
+        } } }
+    }
 
 }
 
@@ -266,4 +267,4 @@ my $nobody = RT::Nobody();
 }
 
 @tickets = ();
-
+done_testing();

-----------------------------------------------------------------------


More information about the Rt-commit mailing list