[Rt-commit] rt branch, 4.2/distinct-joins, created. rt-4.1.8-390-ga3fcaff

Ruslan Zakirov ruz at bestpractical.com
Tue Jun 4 10:24:10 EDT 2013


The branch, 4.2/distinct-joins has been created
        at  a3fcaff7f53371d6a1d8d0b5208fc26f79610f3a (commit)

- Log -----------------------------------------------------------------
commit 9b6429d142abd3975c3213505ec351701636464a
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 8 13:58:48 2013 +0400

    replace NewAlias with Join calls
    
    Join method does NewAlias for us and it makes next code
    easier to implement.
    
    Not all NewAlias calls are replaced with Join, only those
    that can benefit from explicit DISTINCT argument and
    code that auto guess join DISTINCTness.

diff --git a/lib/RT/Attachments.pm b/lib/RT/Attachments.pm
index 219dc78..4e2f195 100644
--- a/lib/RT/Attachments.pm
+++ b/lib/RT/Attachments.pm
@@ -111,14 +111,12 @@ sub TransactionAlias {
     return $self->{'_sql_transaction_alias'}
         if $self->{'_sql_transaction_alias'};
 
-    my $res = $self->NewAlias('Transactions');
-    $self->Limit(
-        ENTRYAGGREGATOR => 'AND',
-        FIELD           => 'TransactionId',
-        VALUE           => $res . '.id',
-        QUOTEVALUE      => 0,
+    return $self->{'_sql_transaction_alias'} = $self->Join(
+        ALIAS1 => 'main',
+        FIELD1 => 'TransactionId',
+        TABLE2 => 'Transactions',
+        FIELD2 => 'id',
     );
-    return $self->{'_sql_transaction_alias'} = $res;
 }
 
 =head2 ContentType (VALUE => 'text/plain', ENTRYAGGREGATOR => 'OR', OPERATOR => '=' ) 
@@ -202,13 +200,11 @@ sub LimitByTicket {
         VALUE           => 'RT::Ticket',
     );
 
-    my $tickets = $self->NewAlias('Tickets');
-    $self->Limit(
-        ENTRYAGGREGATOR => 'AND',
-        ALIAS           => $tickets,
-        FIELD           => 'id',
-        VALUE           => $transactions . '.ObjectId',
-        QUOTEVALUE      => 0,
+    my $tickets = $self->Join(
+        ALIAS1 => $transactions,
+        FIELD1 => 'ObjectId',
+        TABLE2 => 'Tickets',
+        FIELD2 => 'id',
     );
     $self->Limit(
         ENTRYAGGREGATOR => 'AND',
diff --git a/lib/RT/CachedGroupMembers.pm b/lib/RT/CachedGroupMembers.pm
index efa1273..3f8c39a 100644
--- a/lib/RT/CachedGroupMembers.pm
+++ b/lib/RT/CachedGroupMembers.pm
@@ -88,9 +88,10 @@ groups from users for display purposes
 sub LimitToUsers {
     my $self = shift;
 
-    my $principals = $self->NewAlias('Principals');
-    $self->Join( ALIAS1 => 'main', FIELD1 => 'MemberId',
-                 ALIAS2 => $principals, FIELD2 =>'id');
+    my $principals = $self->Join(
+        ALIAS1 => 'main', FIELD1 => 'MemberId',
+        TABLE2 => 'Principals', FIELD2 =>'id'
+    );
 
     $self->Limit(       ALIAS => $principals,
                          FIELD => 'PrincipalType',
@@ -113,9 +114,11 @@ groups from users for display purposes
 sub LimitToGroups {
     my $self = shift;
 
-    my $principals = $self->NewAlias('Principals');
-    $self->Join( ALIAS1 => 'main', FIELD1 => 'MemberId',
-                 ALIAS2 => $principals, FIELD2 =>'id');
+    my $principals = $self->Join(
+        ALIAS1 => 'main', FIELD1 => 'MemberId',
+        TABLE2 => 'Principals', FIELD2 =>'id'
+    );
+
 
     $self->Limit(       ALIAS => $principals,
                          FIELD => 'PrincipalType',
diff --git a/lib/RT/GroupMembers.pm b/lib/RT/GroupMembers.pm
index 0c0cf25..5a7aed2 100644
--- a/lib/RT/GroupMembers.pm
+++ b/lib/RT/GroupMembers.pm
@@ -88,9 +88,10 @@ groups from users for display purposes
 sub LimitToUsers {
     my $self = shift;
 
-    my $principals = $self->NewAlias('Principals');
-    $self->Join( ALIAS1 => 'main', FIELD1 => 'MemberId',
-                 ALIAS2 => $principals, FIELD2 =>'id');
+    my $principals = $self->Join(
+        ALIAS1 => 'main', FIELD1 => 'MemberId',
+        TABLE2 => 'Principals', FIELD2 =>'id'
+    );
 
     $self->Limit(       ALIAS => $principals,
                          FIELD => 'PrincipalType',
@@ -113,9 +114,10 @@ groups from users for display purposes
 sub LimitToGroups {
     my $self = shift;
 
-    my $principals = $self->NewAlias('Principals');
-    $self->Join( ALIAS1 => 'main', FIELD1 => 'MemberId',
-                 ALIAS2 => $principals, FIELD2 =>'id');
+    my $principals = $self->Join(
+        ALIAS1 => 'main', FIELD1 => 'MemberId',
+        TABLE2 => 'Principals', FIELD2 =>'id'
+    );
 
     $self->Limit(       ALIAS => $principals,
                          FIELD => 'PrincipalType',
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index ac6f9a5..26960c8 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -3593,12 +3593,11 @@ sub GetPrincipalsMap {
             );
 
             # Limit to UserEquiv groups
-            my $groups = $Users->NewAlias('Groups');
-            $Users->Join(
-                ALIAS1 => $groups,
-                FIELD1 => 'id',
-                ALIAS2 => $group_members,
-                FIELD2 => 'GroupId'
+            my $groups = $Users->Join(
+                ALIAS1 => $group_members,
+                FIELD1 => 'GroupId',
+                TABLE2 => 'Groups',
+                FIELD2 => 'id',
             );
             $Users->Limit( ALIAS => $groups, FIELD => 'Domain', VALUE => 'ACLEquivalence' );
             $Users->Limit( ALIAS => $groups, FIELD => 'Type', VALUE => 'UserEquiv' );
diff --git a/lib/RT/Scrips.pm b/lib/RT/Scrips.pm
index 007d7fe..2f78b5c 100644
--- a/lib/RT/Scrips.pm
+++ b/lib/RT/Scrips.pm
@@ -458,13 +458,11 @@ sub _FindScrips {
     $self->LimitToGlobal;
     $self->LimitByStage( $args{'Stage'} );
 
-    my $ConditionsAlias = $self->NewAlias('ScripConditions');
-
-    $self->Join(
+    my $ConditionsAlias = $self->Join(
         ALIAS1 => 'main',
         FIELD1 => 'ScripCondition',
-        ALIAS2 => $ConditionsAlias,
-        FIELD2 => 'id'
+        TABLE2 => 'ScripConditions',
+        FIELD2 => 'id',
     );
 
     #We only want things where the scrip applies to this sort of transaction
diff --git a/lib/RT/Transactions.pm b/lib/RT/Transactions.pm
index 54dd41e..f0e75ca 100644
--- a/lib/RT/Transactions.pm
+++ b/lib/RT/Transactions.pm
@@ -108,11 +108,10 @@ sub LimitToTicket {
     my $tid  = shift;
 
     unless ( $self->{'tickets_table'} ) {
-        $self->{'tickets_table'} ||= $self->NewAlias('Tickets');
-        $self->Join(
+        $self->{'tickets_table'} ||= $self->Join(
             ALIAS1 => 'main',
             FIELD1 => 'ObjectId',
-            ALIAS2 => $self->{'tickets_table'},
+            TABLE2 => 'Tickets',
             FIELD2 => 'id'
         );
         $self->Limit(
diff --git a/lib/RT/Users.pm b/lib/RT/Users.pm
index cf27993..65ef513 100644
--- a/lib/RT/Users.pm
+++ b/lib/RT/Users.pm
@@ -86,12 +86,11 @@ sub _Init {
                     FIELD => 'Name',
                     ORDER => 'ASC' );
 
-    $self->{'princalias'} = $self->NewAlias('Principals');
-
     # XXX: should be generalized
-    $self->Join( ALIAS1 => 'main',
+    $self->{'princalias'} = $self->Join(
+                 ALIAS1 => 'main',
                  FIELD1 => 'id',
-                 ALIAS2 => $self->{'princalias'},
+                 TABLE2 => 'Principals',
                  FIELD2 => 'id' );
     $self->Limit( ALIAS => $self->{'princalias'},
                   FIELD => 'PrincipalType',

commit f227d0ec325d45cec03433a9a023532229d2d4b3
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 8 14:01:21 2013 +0400

    put explicit DISTINCT argument in a few places

diff --git a/lib/RT/Groups.pm b/lib/RT/Groups.pm
index 8d30dde..434430f 100644
--- a/lib/RT/Groups.pm
+++ b/lib/RT/Groups.pm
@@ -255,15 +255,15 @@ sub WithMember {
     my %args = ( PrincipalId => undef,
                  Recursively => undef,
                  @_);
-    my $members;
-
-    if ($args{'Recursively'}) {
-        $members = $self->NewAlias('CachedGroupMembers');
-    } else {
-        $members = $self->NewAlias('GroupMembers');
-    }
-    $self->Join(ALIAS1 => 'main', FIELD1 => 'id',
-                ALIAS2 => $members, FIELD2 => 'GroupId');
+    my $members = $self->Join(
+        ALIAS1 => 'main', FIELD1 => 'id',
+        $args{'Recursively'}
+            ? (TABLE2 => 'CachedGroupMembers')
+            # (GroupId, MemberId) is unique in GM table
+            : (TABLE2 => 'GroupMembers', DISTINCT => 1)
+        ,
+        FIELD2 => 'GroupId',
+    );
 
     $self->Limit(ALIAS => $members, FIELD => 'MemberId', OPERATOR => '=', VALUE => $args{'PrincipalId'});
     $self->Limit(ALIAS => $members, FIELD => 'Disabled', VALUE => 0)
@@ -286,6 +286,7 @@ sub WithoutMember {
         FIELD1 => 'id',
         TABLE2 => $members,
         FIELD2 => 'GroupId',
+        DISTINCT => $members eq 'GroupMembers',
     );
     $self->Limit(
         LEFTJOIN => $members_alias,
diff --git a/lib/RT/SearchBuilder.pm b/lib/RT/SearchBuilder.pm
index 46cf96c..c539a94 100644
--- a/lib/RT/SearchBuilder.pm
+++ b/lib/RT/SearchBuilder.pm
@@ -265,6 +265,7 @@ sub _CustomFieldJoin {
             FIELD1 => 'id',
             TABLE2 => 'ObjectCustomFieldValues',
             FIELD2 => 'ObjectId',
+            $cf->SingleValue? (DISTINCT => 1) : (),
         );
         $self->Limit(
             LEFTJOIN        => $ocfvalias,
diff --git a/lib/RT/SearchBuilder/Role/Roles.pm b/lib/RT/SearchBuilder/Role/Roles.pm
index 7167b13..b571599 100644
--- a/lib/RT/SearchBuilder/Role/Roles.pm
+++ b/lib/RT/SearchBuilder/Role/Roles.pm
@@ -119,6 +119,7 @@ sub _RoleGroupsJoin {
         TABLE2          => 'Groups',
         FIELD2          => 'Instance',
         ENTRYAGGREGATOR => 'AND',
+        DISTINCT        => !!$args{'Type'},
     );
     $self->Limit(
         LEFTJOIN        => $groups,

commit 1d8f24cc0318225797b33d43b79f0ad5cd8db3d6
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 8 14:02:05 2013 +0400

    we can auto mark Join as distinct in some cases
    
    If we join to a table and in target table we join to id
    then join is distinct.

diff --git a/lib/RT/SearchBuilder.pm b/lib/RT/SearchBuilder.pm
index c539a94..eae2305 100644
--- a/lib/RT/SearchBuilder.pm
+++ b/lib/RT/SearchBuilder.pm
@@ -95,6 +95,17 @@ sub CleanSlate {
     return $self->SUPER::CleanSlate(@_);
 }
 
+sub Join {
+    my $self = shift;
+    my %args = @_;
+
+    $args{'DISTINCT'} = 1 if
+        !exists $args{'DISTINCT'}
+        && $args{'TABLE2'} && lc($args{'FIELD2'}||'') eq 'id';
+
+    return $self->SUPER::Join( %args );
+}
+
 sub JoinTransactions {
     my $self = shift;
     my %args = ( New => 0, @_ );

commit a3fcaff7f53371d6a1d8d0b5208fc26f79610f3a
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Wed May 8 14:06:57 2013 +0400

    we don't need this code
    
    this is covered by auto marking joins as distinct

diff --git a/sbin/rt-fulltext-indexer.in b/sbin/rt-fulltext-indexer.in
index 624ee8f..f2a5264 100644
--- a/sbin/rt-fulltext-indexer.in
+++ b/sbin/rt-fulltext-indexer.in
@@ -210,11 +210,6 @@ sub attachments {
         VALUE => 'deleted'
     );
 
-    # On newer DBIx::SearchBuilder's, indicate that making the query DISTINCT
-    # is unnecessary because the joins won't produce duplicates.  This
-    # drastically improves performance when fetching attachments.
-    $res->{joins_are_distinct} = 1;
-
     return goto_specific(
         suffix => $type,
         error => "Don't know how to find $type attachments",

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


More information about the Rt-commit mailing list