[Rt-commit] rt branch, 4.4/sql-convert-or-to-in, created. rt-4.4.4-87-gd16ab984d
? sunnavy
sunnavy at bestpractical.com
Mon Feb 24 17:58:53 EST 2020
The branch, 4.4/sql-convert-or-to-in has been created
at d16ab984de54033b445ff225ae0e65ea11b187f0 (commit)
- Log -----------------------------------------------------------------
commit 160b55c11cd1068f089d0974e0ae87bd273a240c
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Feb 25 04:07:01 2020 +0800
Convert more OR'd clauses to use "IN" for performance
The search results should be the same, but the performance is better.
E.g. we found that "IN" version of user right check could be 4x faster
in some RT instances.
This is mainly to cover ticket display page. Another major OR thing left
on that page is Status clauses in TicketSQL, which will be handled in
another commit since the implimentation will be quite different.
See also b14df0ee4f
diff --git a/lib/RT/Principal.pm b/lib/RT/Principal.pm
index aec96f7a2..8b62c2a5a 100644
--- a/lib/RT/Principal.pm
+++ b/lib/RT/Principal.pm
@@ -539,9 +539,12 @@ sub _HasGroupRightQuery {
}
if ( my $right = $args{'Right'} ) {
# Only find superuser or rights with the name $right
- $query .= " AND (ACL.RightName = 'SuperUser' "
- . ( $right ne 'SuperUser' ? "OR ACL.RightName = '$right'" : '' )
- . ") ";
+ if ( $right eq 'SuperUser' ) {
+ $query .= " AND ACL.RightName = 'SuperUser' "
+ }
+ else {
+ $query .= " AND ACL.RightName IN ('SuperUser', '$right')";
+ }
}
return $query;
}
@@ -657,13 +660,12 @@ sub _RolesWithRightQuery {
. " PrincipalType != 'Group'";
if ( my $right = $args{'Right'} ) {
- $query .=
- # Only find superuser or rights with the requested right
- " AND ( RightName = '$right' "
-
- # Check SuperUser if we were asked to
- . ( $args{'IncludeSuperusers'} ? "OR RightName = 'SuperUser' " : '' )
- . ")";
+ if ( $args{'IncludeSuperusers'} && $right ne 'SuperUser' ) {
+ $query .= " AND RightName IN ('SuperUser', '$right')";
+ }
+ else {
+ $query .= " AND RightName = '$right'";
+ }
}
# skip rights granted on system level if we were asked to
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 81224831c..2005fd6a5 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -2612,8 +2612,7 @@ sub SeenUpTo {
return if $attr && $attr->Content gt $self->LastUpdated;
my $txns = $self->Transactions;
- $txns->Limit( FIELD => 'Type', VALUE => 'Comment' );
- $txns->Limit( FIELD => 'Type', VALUE => 'Correspond' );
+ $txns->Limit( FIELD => 'Type', VALUE => [ 'Comment', 'Correspond' ], OPERATOR => 'IN' );
$txns->Limit( FIELD => 'Creator', OPERATOR => '!=', VALUE => $uid );
$txns->Limit(
FIELD => 'Created',
diff --git a/lib/RT/Users.pm b/lib/RT/Users.pm
index 17d5f6df3..487a57ce6 100644
--- a/lib/RT/Users.pm
+++ b/lib/RT/Users.pm
@@ -337,20 +337,30 @@ sub _JoinACL
}
my $acl = $self->NewAlias('ACL');
- $self->Limit(
- ALIAS => $acl,
- FIELD => 'RightName',
- OPERATOR => ( $args{Right} ? '=' : 'IS NOT' ),
- VALUE => $args{Right} || 'NULL',
- ENTRYAGGREGATOR => 'OR'
- );
- if ( $args{'IncludeSuperusers'} and $args{'Right'} ) {
+ if ( $args{Right} ) {
+ if ( $args{'IncludeSuperusers'} && $args{Right} ne 'SuperUser' ) {
+ $self->Limit(
+ ALIAS => $acl,
+ FIELD => 'RightName',
+ OPERATOR => 'IN',
+ VALUE => [ 'SuperUser', $args{Right} ],
+ );
+ }
+ else {
+ $self->Limit(
+ ALIAS => $acl,
+ FIELD => 'RightName',
+ OPERATOR => '=',
+ VALUE => $args{Right},
+ );
+ }
+ }
+ else {
$self->Limit(
- ALIAS => $acl,
- FIELD => 'RightName',
- OPERATOR => '=',
- VALUE => 'SuperUser',
- ENTRYAGGREGATOR => 'OR'
+ ALIAS => $acl,
+ FIELD => 'RightName',
+ OPERATOR => 'IS NOT',
+ VALUE => 'NULL',
);
}
return $acl;
commit d16ab984de54033b445ff225ae0e65ea11b187f0
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Feb 25 05:06:45 2020 +0800
Convert simple OR'd statements in TicketSQL to use IN for better performance
Here are some examples that could get benifets from this change:
Status = 'new' OR Status = 'open'
Status = '__Active__'
id = 2 OR id = 3
Creator = 'root' OR Creator = 'alice'
Queue = 'General' OR Queue = 'Support'
Lifecycle = 'default' or Lifecycle = 'approvals'
Of course also combined ones like:
( Queue = 'General' OR Queue = 'Support' ) AND ( Status = 'new' OR Status = 'open' )
See also 160b55c11c
diff --git a/lib/RT/Tickets.pm b/lib/RT/Tickets.pm
index e094c63ef..aac127c34 100644
--- a/lib/RT/Tickets.pm
+++ b/lib/RT/Tickets.pm
@@ -375,16 +375,31 @@ sub _EnumLimit {
# SQL::Statement changes != to <>. (Can we remove this now?)
$op = "!=" if $op eq "<>";
- die "Invalid Operation: $op for $field"
- unless $op eq "="
- or $op eq "!=";
+ die "Invalid Operation: $op for $field" unless $op =~ /^(?:=|!=|IN|NOT IN)$/i;
my $meta = $FIELD_METADATA{$field};
- if ( defined $meta->[1] && defined $value && $value !~ /^\d+$/ ) {
+ if ( defined $meta->[1] && defined $value ) {
my $class = "RT::" . $meta->[1];
- my $o = $class->new( $sb->CurrentUser );
- $o->Load($value);
- $value = $o->Id || 0;
+
+ if ( ref $value eq 'ARRAY' ) {
+ my @values;
+ for my $i (@$value) {
+ if ( $i !~ /^\d+$/ ) {
+ my $o = $class->new( $sb->CurrentUser );
+ $o->Load($i);
+ push @values, $o->Id || 0;
+ }
+ else {
+ push @values, $i;
+ }
+ }
+ $value = \@values;
+ }
+ elsif ( $value !~ /^\d+$/ ) {
+ my $o = $class->new( $sb->CurrentUser );
+ $o->Load($value);
+ $value = $o->Id || 0;
+ }
} elsif ( $field eq "Type" ) {
$value = lc $value if $value =~ /^(ticket|approval|reminder)$/i;
}
@@ -704,7 +719,12 @@ sub _StringLimit {
}
if ($field eq "Status") {
- $value = lc $value;
+ if ( ref $value eq 'ARRAY' ) {
+ $value = [ map lc, @$value ];
+ }
+ else {
+ $value = lc $value;
+ }
}
$sb->Limit(
@@ -746,9 +766,21 @@ sub _QueueLimit {
}
- my $o = RT::Queue->new( $sb->CurrentUser );
- $o->Load($value);
- $value = $o->Id || 0;
+ if ( ref $value eq 'ARRAY' ) {
+ my @values;
+ for my $v ( @$value ) {
+ my $o = RT::Queue->new( $sb->CurrentUser );
+ $o->Load($v);
+ push @values, $o->Id || 0;
+ }
+ $value = \@values;
+ }
+ else {
+ my $o = RT::Queue->new( $sb->CurrentUser );
+ $o->Load($value);
+ $value = $o->Id || 0;
+ }
+
$sb->Limit(
FIELD => $field,
OPERATOR => $op,
@@ -3154,6 +3186,68 @@ sub _parser {
}
);
+ # Convert simple OR'd clauses to IN for better performance, e.g.
+ # (Status = 'new' OR Status = 'open' OR Status = 'stalled')
+ # to
+ # Status IN ('new', 'open', 'stalled')
+
+ $tree->traverse(
+ sub {
+ my $node = shift;
+ my $parent = $node->getParent;
+ return if $parent eq 'root'; # Skip root's root
+
+ # For simple searches like "Status = 'new' OR Status = 'open'",
+ # the OR node is also the root node, go up one level.
+ $node = $parent if $node->isLeaf && $parent->isRoot;
+
+ return if $node->isLeaf;
+
+ if ( ( $node->getNodeValue // '' ) =~ /^or$/i && $node->getChildCount > 1 ) {
+ my @children = $node->getAllChildren;
+ my %info;
+ for my $child (@children) {
+
+ # Only handle innermost ORs
+ return unless $child->isLeaf;
+ my $entry = $child->getNodeValue;
+ return unless $entry->{Op} =~ /^!?=$/;
+
+ # Handle String/Int/Id/Enum/Queue/Lifecycle only for
+ # now. Others have more complicated logic inside, which
+ # can't be easily converted.
+
+ return unless ( $entry->{Meta}[0] // '' ) =~ /^(?:STRING|INT|ID|ENUM|QUEUE|LIFECYCLE)$/;
+
+ for my $field (qw/Key SubKey Op Value/) {
+ $info{$field}{ $entry->{$field} // '' } ||= 1;
+
+ if ( $field eq 'Value' ) {
+
+ # In case it's meta value like __Bookmarked__
+ return if $entry->{Meta}[0] eq 'ID' && $entry->{$field} !~ /^\d+$/;
+ }
+ elsif ( keys %{ $info{$field} } > 1 ) {
+ return; # Skip if Key/SubKey/Op are different
+ }
+ }
+ }
+
+ my $first_child = shift @children;
+ my $entry = $first_child->getNodeValue;
+ $entry->{Op} = $info{Op}{'='} ? 'IN' : 'NOT IN';
+ $entry->{Value} = [ sort keys %{ $info{Value} } ];
+ if ( $node->isRoot ) {
+ $parent->removeChild($_) for @children;
+ }
+ else {
+ $parent->removeChild($node);
+ $parent->addChild($first_child);
+ }
+ }
+ }
+ );
+
my $ea = '';
$tree->traverse(
sub {
-----------------------------------------------------------------------
More information about the rt-commit
mailing list