[Rt-commit] rt branch 5.0/use-bind-values created. rt-5.0.1-612-g2f1050014d
BPS Git Server
git at git.bestpractical.com
Wed Sep 8 22:17:34 UTC 2021
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "rt".
The branch, 5.0/use-bind-values has been created
at 2f1050014dfde8e89929b2b177276d4c1af9ef0e (commit)
- Log -----------------------------------------------------------------
commit 2f1050014dfde8e89929b2b177276d4c1af9ef0e
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Sep 8 17:35:47 2021 +0800
Give Pg a hint about the data type of the argument
As the string will be converted to a placeholder(?), Pg won't know its
type and thus complain about it, the error is like:
could not determine data type of parameter $2
diff --git a/lib/RT/Assets.pm b/lib/RT/Assets.pm
index 8cd4d4c2f8..dad7d054ca 100644
--- a/lib/RT/Assets.pm
+++ b/lib/RT/Assets.pm
@@ -983,6 +983,9 @@ sub _LinkLimit {
if ( RT->Config->Get('DatabaseType') eq 'SQLite' ) {
$join_expression = qq{'$local_prefix' || main.id};
}
+ elsif ( RT->Config->Get('DatabaseType') eq 'Pg' ) {
+ $join_expression = qq{CONCAT( '$local_prefix'::text, main.id )};;
+ }
else {
$join_expression = qq{CONCAT( '$local_prefix', main.id )};;
}
commit e4252dbfb780b76a4cfac3d452514316f4e6f299
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Sep 1 05:23:32 2021 +0800
Refactor bare select queries to use bind values
diff --git a/lib/RT/Principal.pm b/lib/RT/Principal.pm
index a49f63aa0e..953867ad70 100644
--- a/lib/RT/Principal.pm
+++ b/lib/RT/Principal.pm
@@ -418,12 +418,10 @@ sub HasRights {
my %res = ();
{
- my $query
- = "SELECT DISTINCT ACL.RightName "
- . $self->_HasGroupRightQuery(
- EquivObjects => $args{'EquivObjects'}
- );
- my $rights = $RT::Handle->dbh->selectcol_arrayref($query);
+ my ( $query, @bind_values ) = $self->_HasGroupRightQuery( EquivObjects => $args{'EquivObjects'} );
+ $query = "SELECT DISTINCT ACL.RightName $query";
+
+ my $rights = $RT::Handle->dbh->selectcol_arrayref($query, undef, @bind_values);
unless ($rights) {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
@@ -432,12 +430,9 @@ sub HasRights {
}
my $roles;
{
- my $query
- = "SELECT DISTINCT Groups.Name "
- . $self->_HasRoleRightQuery(
- EquivObjects => $args{'EquivObjects'}
- );
- $roles = $RT::Handle->dbh->selectcol_arrayref($query);
+ my ( $query, @bind_values ) = $self->_HasRoleRightQuery( EquivObjects => $args{'EquivObjects'} );
+ $query = "SELECT DISTINCT Groups.Name $query";
+ $roles = $RT::Handle->dbh->selectcol_arrayref($query, undef, @bind_values);
unless ($roles) {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
@@ -468,14 +463,11 @@ sub HasRights {
if ( @enabled_roles ) {
- my $query
- = "SELECT DISTINCT ACL.RightName "
- . $self->_RolesWithRightQuery(
- EquivObjects => $args{'EquivObjects'}
- )
- . ' AND ('. join( ' OR ', map "PrincipalType = '$_'", @enabled_roles ) .')'
- ;
- my $rights = $RT::Handle->dbh->selectcol_arrayref($query);
+ my ( $query, @bind_values ) = $self->_RolesWithRightQuery( EquivObjects => $args{'EquivObjects'} );
+ $query = "SELECT DISTINCT ACL.RightName $query";
+ $query .= ' AND ('. join( ' OR ', ("PrincipalType = ?") x @enabled_roles ) .')';
+ push @bind_values, @enabled_roles;
+ my $rights = $RT::Handle->dbh->selectcol_arrayref($query, undef, @bind_values);
unless ($rights) {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
@@ -520,12 +512,11 @@ sub _HasGroupRight {
@_
);
- my $query
- = "SELECT ACL.id, ACL.ObjectType, ACL.ObjectId "
- . $self->_HasGroupRightQuery( %args );
+ my ( $query, @bind_values ) = $self->_HasGroupRightQuery(%args);
+ $query = "SELECT ACL.id, ACL.ObjectType, ACL.ObjectId $query";
$self->_Handle->ApplyLimits( \$query, 1 );
- my ( $hit, $obj, $id ) = $self->_Handle->FetchResult($query);
+ my ( $hit, $obj, $id ) = $self->_Handle->FetchResult($query, @bind_values);
return (0) unless $hit;
$obj .= "-$id" if $id;
@@ -540,30 +531,33 @@ sub _HasGroupRightQuery {
@_
);
+ my @bind_values;
my $query
= "FROM ACL, Principals, CachedGroupMembers WHERE "
# Never find disabled groups.
. "Principals.id = ACL.PrincipalId "
- . "AND Principals.PrincipalType = 'Group' "
- . "AND Principals.Disabled = 0 "
-
+ . "AND Principals.PrincipalType = ? " # 'Group'
+ . "AND Principals.Disabled = ? " # 0
# See if the principal is a member of the group recursively or _is the rightholder_
# never find recursively disabled group members
# also, check to see if the right is being granted _directly_ to this principal,
# as is the case when we want to look up group rights
. "AND CachedGroupMembers.GroupId = ACL.PrincipalId "
. "AND CachedGroupMembers.GroupId = Principals.id "
- . "AND CachedGroupMembers.MemberId = ". $self->Id . " "
- . "AND CachedGroupMembers.Disabled = 0 ";
+ . "AND CachedGroupMembers.MemberId = ? " # $self->Id
+ . "AND CachedGroupMembers.Disabled = ? "; # 0
+ push @bind_values, 'Group', 0, $self->Id, 0;
my @clauses;
foreach my $obj ( @{ $args{'EquivObjects'} } ) {
my $type = ref($obj) || $obj;
- my $clause = "ACL.ObjectType = '$type'";
+ my $clause = "ACL.ObjectType = ?"; # $type
+ push @bind_values, $type;
if ( defined eval { $obj->id } ) { # it might be 0
- $clause .= " AND ACL.ObjectId = " . $obj->id;
+ $clause .= " AND ACL.ObjectId = ?"; # $obj->id;
+ push @bind_values, $obj->id;
}
push @clauses, "($clause)";
@@ -574,13 +568,15 @@ sub _HasGroupRightQuery {
if ( my $right = $args{'Right'} ) {
# Only find superuser or rights with the name $right
if ( $right eq 'SuperUser' ) {
- $query .= " AND ACL.RightName = 'SuperUser' "
+ $query .= " AND ACL.RightName = ? "; # 'SuperUser'
+ push @bind_values, 'SuperUser';
}
else {
- $query .= " AND ACL.RightName IN ('SuperUser', '$right')";
+ $query .= " AND ACL.RightName IN (?, ?)"; # 'SuperUser', $right
+ push @bind_values, 'SuperUser', $right;
}
}
- return $query;
+ return ( $query, @bind_values );
}
sub _HasRoleRight {
@@ -593,11 +589,11 @@ sub _HasRoleRight {
my @roles = $self->RolesWithRight(%args);
return 0 unless @roles;
- my $query = "SELECT Groups.id "
- . $self->_HasRoleRightQuery( %args, Roles => \@roles );
+ my ( $query, @bind_values ) = $self->_HasRoleRightQuery( %args, Roles => \@roles );
+ $query = "SELECT Groups.id $query";
$self->_Handle->ApplyLimits( \$query, 1 );
- my ($hit) = $self->_Handle->FetchResult($query);
+ my ($hit) = $self->_Handle->FetchResult($query, @bind_values);
return (1) if $hit;
return 0;
@@ -619,30 +615,44 @@ sub _HasRoleRightQuery {
" FROM Groups, Principals, CachedGroupMembers WHERE "
# Never find disabled things
- . "Principals.Disabled = 0 " . "AND CachedGroupMembers.Disabled = 0 "
+ . "Principals.Disabled = ? " . "AND CachedGroupMembers.Disabled = ? " # 0, 0
# We always grant rights to Groups
. "AND Principals.id = Groups.id "
- . "AND Principals.PrincipalType = 'Group' "
+ . "AND Principals.PrincipalType = ? " # 'Group'
# See if the principal is a member of the group recursively or _is the rightholder_
# never find recursively disabled group members
# also, check to see if the right is being granted _directly_ to this principal,
# as is the case when we want to look up group rights
. "AND Principals.id = CachedGroupMembers.GroupId "
- . "AND CachedGroupMembers.MemberId IN (" . ( join ',', $self->Id, map { $_->id } @{ $groups->ItemsArrayRef } ) . ") "
+ . "AND CachedGroupMembers.MemberId IN (" . ( join ',', ('?') x (1 + @{ $groups->ItemsArrayRef } ) ) . ") "
;
+ my @bind_values = ( 0, 0, 'Group', $self->Id, map { $_->id } @{ $groups->ItemsArrayRef } );
if ( $args{'Roles'} ) {
- $query .= "AND (" . join( ' OR ',
- map $RT::Handle->__MakeClauseCaseInsensitive('Groups.Name', '=', "'$_'"),
- @{ $args{'Roles'} }
- ) . ")";
+ $query .= "AND ("
+ . join( ' OR ',
+ ( $RT::Handle->__MakeClauseCaseInsensitive( 'Groups.Name', '=', '?' ) )
+ x @{ $args{'Roles'} } )
+ . ")";
+ push @bind_values, map { lc $_ } @{ $args{'Roles'} };
}
- my @object_clauses = RT::Users->_RoleClauses( Groups => @{ $args{'EquivObjects'} } );
+ my @object_clauses;
+ foreach my $obj ( @{ $args{'EquivObjects'} } ) {
+ my $type = ref($obj) ? ref($obj) : $obj;
+
+ my $role_clause = $RT::Handle->__MakeClauseCaseInsensitive( "Groups.Domain", '=', '?' );
+ push @bind_values, lc "$type-Role";
+ if ( my $id = eval { $obj->id } ) {
+ $role_clause .= " AND Groups.Instance = ?";
+ push @bind_values, $id;
+ }
+ push @object_clauses, "($role_clause)";
+ }
$query .= " AND (" . join( ' OR ', @object_clauses ) . ")";
- return $query;
+ return ( $query, @bind_values );
}
=head2 RolesWithRight
@@ -672,10 +682,10 @@ sub RolesWithRight {
return () if $args{'Right'} eq 'ExecuteCode'
and RT->Config->Get('DisallowExecuteCode');
- my $query = "SELECT DISTINCT PrincipalType "
- . $self->_RolesWithRightQuery( %args );
+ my ($query, @bind_values) = $self->_RolesWithRightQuery( %args );
+ $query = "SELECT DISTINCT PrincipalType $query";
- my $roles = $RT::Handle->dbh->selectcol_arrayref($query);
+ my $roles = $RT::Handle->dbh->selectcol_arrayref($query, undef, @bind_values);
unless ($roles) {
$RT::Logger->warning( $RT::Handle->dbh->errstr );
return ();
@@ -715,32 +725,39 @@ sub _RolesWithRightQuery {
@_
);
+ my @bind_values;
my $query = " FROM ACL WHERE"
# we need only roles
- . " PrincipalType != 'Group'";
+ . " PrincipalType != ?"; # 'Group'
+ push @bind_values, 'Group';
if ( my $right = $args{'Right'} ) {
if ( $args{'IncludeSuperusers'} && $right ne 'SuperUser' ) {
- $query .= " AND RightName IN ('SuperUser', '$right')";
+ $query .= " AND RightName IN (?, ?)"; # 'SuperUser', $right
+ push @bind_values, 'SuperUser', $right;
}
else {
- $query .= " AND RightName = '$right'";
+ $query .= " AND RightName = ?"; # $right
+ push @bind_values, $right;
}
}
# skip rights granted on system level if we were asked to
unless ( $args{'IncludeSystemRights'} ) {
- $query .= " AND ObjectType != 'RT::System'";
+ $query .= " AND ObjectType != ?"; # 'RT::System'
+ push @bind_values, 'RT::System';
}
my (@object_clauses);
foreach my $obj ( @{ $args{'EquivObjects'} } ) {
my $type = ref($obj) ? ref($obj) : $obj;
- my $object_clause = "ObjectType = '$type'";
+ my $object_clause = "ObjectType = ?"; # $type
+ push @bind_values, $type;
if ( my $id = eval { $obj->id } ) {
- $object_clause .= " AND ObjectId = $id";
+ $object_clause .= " AND ObjectId = ?"; # $id
+ push @bind_values, $id;
}
push @object_clauses, "($object_clause)";
}
@@ -749,7 +766,7 @@ sub _RolesWithRightQuery {
$query .= " AND (" . join( ' OR ', @object_clauses ) . ")"
if @object_clauses;
- return $query;
+ return ( $query, @bind_values );
}
commit 0c0465c31d148cb73f5851fc3df6f3a01a5a7e9e
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Aug 31 05:58:20 2021 +0800
Update tests to force BuildSelectQuery to not use bind values
Bind values are enabled in RT by default now.
diff --git a/t/api/sql.t b/t/api/sql.t
index 7241e6fb3c..7a39cd65f9 100644
--- a/t/api/sql.t
+++ b/t/api/sql.t
@@ -7,7 +7,7 @@ use RT::Test tests => undef;
my $users = RT::Users->new( RT->SystemUser );
$users->WhoHaveGroupRight( Right => 'OwnTicket', Object => RT->System, IncludeSuperusers => 1 );
like(
- $users->BuildSelectQuery,
+ $users->BuildSelectQuery(PreferBind => 0),
qr{RightName IN \('SuperUser', 'OwnTicket'\)},
'RightName check in WhoHaveGroupRight uses IN'
);
@@ -31,7 +31,7 @@ my %ticketsql = (
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} );
+ like( $tickets->BuildSelectQuery(PreferBind => 0), $ticketsql{$query}, qq{TicketSQL "$query" uses IN} );
}
done_testing;
diff --git a/t/ticket/priority.t b/t/ticket/priority.t
index 6a40a5b7cb..327a968e2b 100644
--- a/t/ticket/priority.t
+++ b/t/ticket/priority.t
@@ -91,11 +91,11 @@ diag "Ticket/Transaction search";
for my $field (qw/Priority InitialPriority FinalPriority/) {
my $tickets = RT::Tickets->new( RT->SystemUser );
$tickets->FromSQL("Queue = 'General' AND $field = 'Low'");
- like( $tickets->BuildSelectQuery, qr/$field = '20'/, "$field is translated properly" );
+ like( $tickets->BuildSelectQuery(PreferBind => 0), qr/$field = '20'/, "$field is translated properly" );
my $txns = RT::Transactions->new( RT->SystemUser );
$txns->FromSQL("TicketQueue = 'General' AND Ticket$field = 'Low'");
- like( $txns->BuildSelectQuery, qr/$field = '20'/, "Ticket$field is translated properly" );
+ like( $txns->BuildSelectQuery(PreferBind => 0), qr/$field = '20'/, "Ticket$field is translated properly" );
}
my $tickets = RT::Tickets->new( RT->SystemUser );
commit b415ab181e07c8f3d6223fab12e0f998c05d4efa
Author: sunnavy <sunnavy at bestpractical.com>
Date: Thu Sep 9 01:45:46 2021 +0800
Use bind values for searches mainly for better performance
This is initially for Oracle, which recommends it eagerly.
diff --git a/lib/RT/SearchBuilder.pm b/lib/RT/SearchBuilder.pm
index ee16cf42ca..caf1d035b1 100644
--- a/lib/RT/SearchBuilder.pm
+++ b/lib/RT/SearchBuilder.pm
@@ -69,6 +69,7 @@ use warnings;
use 5.010;
use base qw(DBIx::SearchBuilder RT::Base);
+$DBIx::SearchBuilder::PREFER_BIND = 1 unless defined $ENV{SB_PREFER_BIND};
use RT::Base;
use DBIx::SearchBuilder "1.40";
-----------------------------------------------------------------------
hooks/post-receive
--
rt
More information about the rt-commit
mailing list