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

Alex Vandiver alexmv at bestpractical.com
Tue Jun 4 13:43:56 EDT 2013


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

- Log -----------------------------------------------------------------
commit 06f0bfd0d2e487a53c38a236fd326bdfba17bb25
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/etc/upgrade/4.0.1/content b/etc/upgrade/4.0.1/content
index 14da329..6a4e99c 100644
--- a/etc/upgrade/4.0.1/content
+++ b/etc/upgrade/4.0.1/content
@@ -6,12 +6,12 @@ our @Initial = (
         RT->Logger->debug('Removing all delegated rights');
 
         my $acl = RT::ACL->new(RT->SystemUser);
-        my $groupjoin = $acl->NewAlias('Groups');
-        $acl->Join( ALIAS1 => 'main',
-                    FIELD1 => 'PrincipalId',
-                    ALIAS2 => $groupjoin,
-                    FIELD2 => 'id'
-                  );
+        my $groupjoin = $acl->Join(
+            ALIAS1 => 'main',
+            FIELD1 => 'PrincipalId',
+            TABLE2 => 'Groups',
+            FIELD2 => 'id',
+        );
         $acl->Limit( ALIAS           => $groupjoin,
                      FIELD           => 'Domain',
                      OPERATOR        => '=',
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 4646c5ae9f12b003b00d06e50f200f06f33d5b5a
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 84c48f36741e6ee8592fab934e095e8c8386f16d
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 20ccebd32ac630a1a503820ab58078771e378099
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