[Rt-commit] rt branch, 4.2/who-have-right-optimization, created. rt-4.2.11-5-g9f96a81

[Alex Vandiver]

The branch, 4.2/who-have-right-optimization has been created
        at  9f96a81abd067c8630cc4c887abfb03696bf69fe (commit)

- Log -----------------------------------------------------------------
commit 8cb2dc576e6798512dae9414045b9707c21659d8
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Oct 28 18:48:49 2014 -0400

    Only add limits for types that explicitly are rights targets
    
    This skips adding an ObjectType = 'RT::Ticket' limit, which will never
    be necessary.
    
    XXX: This causes tests to fail, but the tests are horrible and old and wrong.

diff --git a/lib/RT/Users.pm b/lib/RT/Users.pm
index a8e81eb..1d6c689 100644
--- a/lib/RT/Users.pm
+++ b/lib/RT/Users.pm
@@ -512,6 +512,7 @@ sub WhoHaveGroupRight
             my $id = 0;
             $id = $obj->id if ref($obj) && UNIVERSAL::can($obj, 'id') && $obj->id;
             next if $seen{"$type-$id"}++;
+            next unless $type->DOES("RT::Record::Role::Rights");
 
             my $object_clause = "$acl.ObjectType = '$type'";
             $object_clause   .= " AND $acl.ObjectId   = $id" if $id;
@@ -524,6 +525,7 @@ sub WhoHaveGroupRight
             $check_objects = "($acl.ObjectType != 'RT::System')";
         }
     }
+    $check_objects ||= "$acl.id = 0";
     $self->_AddSubClause( "WhichObject", "($check_objects)" );
     
     my $group_members = $self->_JoinGroupMembersForGroupRights( %args, ACLAlias => $acl );

commit 9f96a81abd067c8630cc4c887abfb03696bf69fe
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Oct 28 18:50:58 2014 -0400

    Simplify WhoHaveRoleRight and WhoHaveGroupRight queries before unioning
    
    Perform a true union in SQL, via a separate 'in' query.  This
    effectively prevents duplicates in the resultset (which
    DBIx::SearchBuilder::Union makes no attempt to do), orders the set as a
    whole, and allows the individual queries to be simpler.  In particular,
    the lack of "DISTINCT" and "ORDER BY" on the WhoHaveGroupRight query on
    MySQL causes it to generate a query plan which no longer includes "with
    temporary table" (due to the DISTINCT) or "with filesort" (due to the
    ORDER BY), and makes better use of indexes (due to the limited number of
    columns returned).
    
    This does come at the assumption that duplicates within each of the two
    queries are not common -- that is, that users will not have the relevant
    right by way of a large number of membership paths.

diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index 4d06528..aeb80e6 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -339,19 +339,34 @@ sub WithRight {
                  EquivObjects           => [ ],
                  @_ );
 
+    my @ids = (0);
+
     my $from_role = $self->Clone;
     $from_role->WithRoleRight( %args );
+    $from_role->Columns( 'id' );
+    $from_role->OrderBy();
+    $from_role->{'joins_are_distinct'} = 1;
+    while (my $row = $from_role->Next) {
+        push @ids, $row->id;
+    }
 
     my $from_group = $self->Clone;
     $from_group->WithGroupRight( %args );
+    $from_group->Columns( 'id' );
+    $from_group->OrderBy();
+    $from_group->{'joins_are_distinct'} = 1;
+    while (my $row = $from_group->Next) {
+        push @ids, $row->id;
+    }
 
-    #XXX: DIRTY HACK
-    use DBIx::SearchBuilder::Union;
-    my $union = DBIx::SearchBuilder::Union->new();
-    $union->add($from_role);
-    $union->add($from_group);
+    my $union = RT::Groups->new( $self->CurrentUser );
+    while ( @ids > 1000 ) {
+        my @batch = splice( @ids, 0, 1000 );
+        $union->Limit( FIELD => 'id', OPERATOR => 'IN', VALUE => \@batch );
+    }
+    $union->Limit( FIELD => 'id', OPERATOR => 'IN', VALUE => \@ids );
+    $union->{'handled_disabled_column'} = 1;
     %$self = %$union;
-    bless $self, ref($union);
 
     return;
 }
diff --git a/lib/RT/Users.pm b/lib/RT/Users.pm
index 1d6c689..86cda82 100644
--- a/lib/RT/Users.pm
+++ b/lib/RT/Users.pm
@@ -386,19 +386,34 @@ sub WhoHaveRight {
         return (undef);
     }
 
+    my @ids = (0);
+
     my $from_role = $self->Clone;
     $from_role->WhoHaveRoleRight( %args );
+    $from_role->Columns( 'id' );
+    $from_role->OrderBy();
+    $from_role->{'joins_are_distinct'} = 1;
+    while (my $row = $from_role->Next) {
+        push @ids, $row->id;
+    }
 
     my $from_group = $self->Clone;
     $from_group->WhoHaveGroupRight( %args );
+    $from_group->Columns( 'id' );
+    $from_group->OrderBy();
+    $from_group->{'joins_are_distinct'} = 1;
+    while (my $row = $from_group->Next) {
+        push @ids, $row->id;
+    }
 
-    #XXX: DIRTY HACK
-    use DBIx::SearchBuilder::Union;
-    my $union = DBIx::SearchBuilder::Union->new();
-    $union->add( $from_group );
-    $union->add( $from_role );
+    my $union = RT::Users->new( $self->CurrentUser );
+    while ( @ids > 1000 ) {
+        my @batch = splice( @ids, 0, 1000 );
+        $union->Limit( FIELD => 'id', OPERATOR => 'IN', VALUE => \@batch );
+    }
+    $union->Limit( FIELD => 'id', OPERATOR => 'IN', VALUE => \@ids );
+    $union->{'handled_disabled_column'} = 1;
     %$self = %$union;
-    bless $self, ref($union);
 
     return;
 }

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