[Rt-commit] rt branch, 4.2/charting-distinct, created. rt-4.2.6-36-g13ce5b0

Alex Vandiver alexmv at bestpractical.com
Tue Aug 12 17:32:08 EDT 2014


The branch, 4.2/charting-distinct has been created
        at  13ce5b0215674cff120a1ea320a07cc374123d18 (commit)

- Log -----------------------------------------------------------------
commit 13ce5b0215674cff120a1ea320a07cc374123d18
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Aug 12 16:17:26 2014 -0400

    When charting, do two searches if the query isn't distinct
    
    If a search has joins which result in a non-distinct ticket result, the
    SUM() aggregator is incorrect.  Notably, if UseSQLForACLChecks is
    enabled, and a global role right exists, it causes a LEFT JOIN to ticket
    role groups -- which cause all maching tickets to show up four times,
    multiplying SUM(TimeWorked) (for instance) by four.
    
    As we can determine if the query will be non-distinct before making it,
    split it into a SELECT DISTINCT search that returns ticket ids, followed
    by a search over those ticket ids, grouped appropriately.  This comes at
    a small speed penalty for such searches, but provides the only clear
    route for correctness in SUM() metrics.

diff --git a/lib/RT/Report/Tickets.pm b/lib/RT/Report/Tickets.pm
index 6a61960..dba2264 100644
--- a/lib/RT/Report/Tickets.pm
+++ b/lib/RT/Report/Tickets.pm
@@ -283,10 +283,8 @@ our %STATISTICS_META = (
             my $self = shift;
             my $field = shift || 'id';
 
-            # UseSQLForACLChecks may add late joins
-            my $joined = ($self->_isJoined || RT->Config->Get('UseSQLForACLChecks')) ? 1 : 0;
             return (
-                FUNCTION => ($joined ? 'DISTINCT COUNT' : 'COUNT'),
+                FUNCTION => 'COUNT',
                 FIELD    => 'id'
             );
         },
@@ -464,6 +462,29 @@ sub SetupGroupings {
 
     $self->FromSQL( $args{'Query'} ) if $args{'Query'};
 
+    # Apply ACL checks
+    $self->CurrentUserCanSee if RT->Config->Get('UseSQLForACLChecks');
+
+    # See if our query is distinct
+    if (not $self->{'joins_are_distinct'} and $self->_isJoined) {
+        # If it isn't, we need to do this in two stages -- first, find
+        # the distinct matching tickets (with no group by), then search
+        # within the matching tickets grouped by what is wanted.
+        my @match;
+        $self->Columns( 'id' );
+        while (my $row = $self->Next) {
+            push @match, $row->id;
+        }
+
+        # Replace the query with one that matches precisely those
+        # tickets, with no joins.  We then mark it as having been ACL'd,
+        # since it was by dint of being in the search results above
+        $self->CleanSlate;
+        $self->Limit( FIELD => 'Id', OPERATOR => 'IN', VALUE => \@match );
+        $self->{'_sql_current_user_can_see_applied'} = 1
+    }
+
+
     %GROUPINGS = @GROUPINGS unless keys %GROUPINGS;
 
     my $i = 0;

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


More information about the rt-commit mailing list