[Rt-commit] rt branch, 4.4-trunk, updated. rt-4.4.4-376-g2deffa7590
Jim Brandt
jbrandt at bestpractical.com
Fri Apr 23 17:21:04 EDT 2021
The branch, 4.4-trunk has been updated
via 2deffa759040895294cb26b24bc0b175efe5d9ef (commit)
via ab20fc32315a53f8f8e569ee16a687c3da7fffb3 (commit)
via 2745b9ff057b2a940c1c460432fe71cec18e3864 (commit)
via 71b9b929938bc17ea43a6def275d5fea1c5702ce (commit)
via 4b5e8129a80f2d0a9a4d9e78eb6f382ccd6f4dba (commit)
from aa3a7ed6daf222ff5ad251342a647a5cdd1f66aa (commit)
Summary of changes:
lib/RT/Principal.pm | 22 +++++-----
lib/RT/Ticket.pm | 3 +-
lib/RT/Tickets.pm | 116 +++++++++++++++++++++++++++++++++++++++++++++++-----
lib/RT/Users.pm | 36 ++++++++++------
t/api/sql.t | 37 +++++++++++++++++
5 files changed, 178 insertions(+), 36 deletions(-)
create mode 100644 t/api/sql.t
- Log -----------------------------------------------------------------
commit 4b5e8129a80f2d0a9a4d9e78eb6f382ccd6f4dba
Author: sunnavy <sunnavy at bestpractical.com>
Date: Thu Feb 6 22:37:47 2020 +0800
Remove search string including numbers in ticket autocomplete search on select
Previously we only removed non-integers, which is not enough as the
serch string could be something like "RT 5 alpha", in which case "5"
wouldn't be removed.
Search string starts with non-integers and lasts till the end, so we can
remove all the items after the first non-integer.
Note that integer stringification("var str = term + '';") was only for
the new value. Since now we add the new value a bit later, there is no
need to do it any more.
diff --git a/share/static/js/autocomplete.js b/share/static/js/autocomplete.js
index cd8ab2b0d4..8040bef144 100644
--- a/share/static/js/autocomplete.js
+++ b/share/static/js/autocomplete.js
@@ -61,14 +61,18 @@ window.RT.Autocomplete.bind = function(from) {
options.select = function(event, ui) {
var terms = this.value.split(what == 'Tickets' ? /\s+/ : /,\s*/);
terms.pop(); // remove current input
- terms.push( ui.item.value ); // add selected item
if ( what == 'Tickets' ) {
// remove non-integers in case subject search with spaces in (like "foo bar")
- terms = jQuery.grep(terms, function(term) {
- var str = term + ''; // stringify integers to call .match
- return str.match(/^\d+$/);
- } );
+ var new_terms = [];
+ for ( var i = 0; i < terms.length; i++ ) {
+ if ( terms[i].match(/\D/) ) {
+ break; // Items after the first non-integers are all parts of search string
+ }
+ new_terms.push(terms[i]);
+ }
+ terms = new_terms;
}
+ terms.push( ui.item.value ); // add selected item
terms.push(''); // add trailing delimeter so user can input another value directly
this.value = terms.join(what == 'Tickets' ? ' ' : ", ");
jQuery(this).change();
commit 71b9b929938bc17ea43a6def275d5fea1c5702ce
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 aec96f7a2e..8b62c2a5a9 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 81224831cc..2005fd6a5b 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 17d5f6df35..487a57ce64 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 2745b9ff057b2a940c1c460432fe71cec18e3864
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 benefit 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 e094c63ef6..aac127c349 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 {
commit ab20fc32315a53f8f8e569ee16a687c3da7fffb3
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Feb 26 04:57:42 2020 +0800
Test SQL conversion of OR => IN
It's mainly for ticket SQL, which involves tricky SQL tree manipulation.
diff --git a/t/api/sql.t b/t/api/sql.t
new file mode 100644
index 0000000000..7241e6fb3c
--- /dev/null
+++ b/t/api/sql.t
@@ -0,0 +1,37 @@
+use strict;
+use warnings;
+
+use RT::Test tests => undef;
+
+# The IN version of this SQL is 4x faster in a real RT instance.
+my $users = RT::Users->new( RT->SystemUser );
+$users->WhoHaveGroupRight( Right => 'OwnTicket', Object => RT->System, IncludeSuperusers => 1 );
+like(
+ $users->BuildSelectQuery,
+ qr{RightName IN \('SuperUser', 'OwnTicket'\)},
+ 'RightName check in WhoHaveGroupRight uses IN'
+);
+
+my $root_id = RT::Test->load_or_create_user( Name => 'root' )->id;
+my $alice_id = RT::Test->load_or_create_user( Name => 'alice' )->id;
+my $general_id = RT::Test->load_or_create_queue( Name => 'General' )->id;
+my $support_id = RT::Test->load_or_create_queue( Name => 'Support' )->id;
+
+my %ticketsql = (
+ q{Status = 'new' OR Status = 'open'} => qr{Status IN \('new', 'open'\)},
+ q{Status = '__Active__'} => qr{Status IN \('new', 'open', 'stalled'\)},
+ q{id = 2 OR id = 3} => qr{id IN \('2', '3'\)},
+ q{Creator = 'root' OR Creator = 'alice'} => qr{Creator IN \('$alice_id', '$root_id'\)},
+ q{Queue = 'General' OR Queue = 'Support'} => qr{Queue IN \('$general_id', '$support_id'\)},
+ q{Lifecycle = 'default' or Lifecycle = 'approvals'} => qr{Lifecycle IN \('approvals', 'default'\)},
+ q{(Queue = 'General' OR Queue = 'Support') AND (Status = 'new' OR Status = 'open')} =>
+ qr{Queue IN \('$general_id', '$support_id'\).+Status IN \('new', 'open'\)},
+);
+
+my $tickets = RT::Tickets->new( RT->SystemUser );
+for my $query ( sort keys %ticketsql ) {
+ $tickets->FromSQL($query);
+ like( $tickets->BuildSelectQuery, $ticketsql{$query}, qq{TicketSQL "$query" uses IN} );
+}
+
+done_testing;
commit 2deffa759040895294cb26b24bc0b175efe5d9ef
Merge: aa3a7ed6da ab20fc3231
Author: Jim Brandt <jbrandt at bestpractical.com>
Date: Fri Apr 23 17:09:15 2021 -0400
Merge branch '4.4/sql-convert-or-to-in' into 4.4-trunk
-----------------------------------------------------------------------
More information about the rt-commit
mailing list