[Rt-commit] rt branch, 4.4/mysql8-quoted-tables, created. rt-4.4.4-139-g461a31886f

Aaron Trevena ast at bestpractical.com
Mon Sep 14 10:21:26 EDT 2020


The branch, 4.4/mysql8-quoted-tables has been created
        at  461a31886f6deccea1cd4bcd2ebd7979593284c3 (commit)

- Log -----------------------------------------------------------------
commit 85639e1e26684261f7e89ebd585cad5c60fcdead
Author: Aaron Trevena <ast at bestpractical.com>
Date:   Tue Sep 1 16:19:56 2020 +0100

    Update mysql schema for mysql 8 making Groups reserved word
    
    MySQL made Groups a reserved word in version 8.0.2 which causes unquoted use of
    the Groups table name in RT a syntax error.

diff --git a/etc/acl.mysql b/etc/acl.mysql
index 26e27fbfbc..2f56ca9adb 100644
--- a/etc/acl.mysql
+++ b/etc/acl.mysql
@@ -1,5 +1,7 @@
 
 sub acl {
+    my $dbh = shift;
+    my $mysql_version = $dbh->{mysql_serverversion};
     my $db_name = RT->Config->Get('DatabaseName');
     my $db_rthost = RT->Config->Get('DatabaseRTHost');
     my $db_user = RT->Config->Get('DatabaseUser');
@@ -13,12 +15,23 @@ sub acl {
         return;
     }
     $db_name =~ s/([_%\\])/\\$1/g;
-    return (
-        "GRANT SELECT,INSERT,CREATE,INDEX,UPDATE,DELETE
+
+    my @sql;
+    # run 2 part acl update for mysql or mariadb 8 or higher
+    if ($mysql_version > 8011) {
+       @sql = (
+        "CREATE USER IF NOT EXISTS '$db_user'\@'$db_rthost' IDENTIFIED BY '$db_pass';",
+        "GRANT SELECT,INSERT,CREATE,INDEX,UPDATE,DELETE ON `$db_name`.* TO '$db_user'\@'$db_rthost';",
+    );
+    }
+    else {
+       @sql = ("GRANT SELECT,INSERT,CREATE,INDEX,UPDATE,DELETE
                ON `$db_name`.*
                TO '$db_user'\@'$db_rthost'
-               IDENTIFIED BY '$db_pass';",
-    );
+               IDENTIFIED BY '$db_pass';");
+   }
+
+    return (@sql);
 }
 
 1;
diff --git a/etc/schema.mysql b/etc/schema.mysql
index eefc145ca4..f33f180be3 100644
--- a/etc/schema.mysql
+++ b/etc/schema.mysql
@@ -70,7 +70,7 @@ CREATE TABLE Principals (
 ) ENGINE=InnoDB CHARACTER SET ascii;
 
 
-CREATE TABLE Groups (
+CREATE TABLE `Groups` (
   id INTEGER NOT NULL  AUTO_INCREMENT,
   Name varchar(200) NULL  ,
   Description varchar(255) NULL  ,
@@ -83,8 +83,8 @@ CREATE TABLE Groups (
   PRIMARY KEY (id)
 ) ENGINE=InnoDB CHARACTER SET utf8;
 
-CREATE INDEX Groups1 ON Groups (Domain, Name, Instance);
-CREATE INDEX Groups2 On Groups (Instance);
+CREATE INDEX Groups1 ON `Groups` (Domain, Name, Instance);
+CREATE INDEX Groups2 On `Groups` (Instance);
 
 CREATE TABLE ScripConditions (
   id INTEGER NOT NULL  AUTO_INCREMENT,

commit 53c8cb4575367bf9fbef2ef389bfcf3a3fe9974e
Author: Aaron Trevena <ast at bestpractical.com>
Date:   Tue Sep 1 16:20:29 2020 +0100

    Update queries and user creation for Mysql 8
    
    MySQL 8.0.2 made Groups a reserved word causing unquoted use of the Groups table
    name in RT a syntax error.
    
    Mysql 8 also removed implicit user creation from the GRANT command, causing user
    creation in setup and tests to fail.
    
    Table name quoting requires an updated DBIx::SearchBuilder with tablename quoting

diff --git a/lib/RT/Handle.pm b/lib/RT/Handle.pm
index 29e929e7b7..a5963ec200 100644
--- a/lib/RT/Handle.pm
+++ b/lib/RT/Handle.pm
@@ -271,10 +271,19 @@ sub CheckCompatibility {
         return (0, "couldn't get version of the mysql server")
             unless $version;
 
+        # allow checking if mysql or mariadb
+        my $subtype = ($version =~ m/mariadb/i) ? 'mariadb' : 'mysql';
         ($version) = $version =~ /^(\d+\.\d+)/;
         return (0, "RT is unsupported on MySQL versions before 4.1.  Your version is $version.")
             if $version < 4.1;
 
+        # Installed DBIx::SearchBuilder must have table name quoting for mysql 8
+        if ($version >= 8 and $subtype eq 'mysql') {
+            unless ($self->can('QuoteName')) {
+                return (0, "RT support for MySQL 8 requires a DBIx::SearchBuilder with table quoting support, check that you have the latest version of DBIx::SearchBuilder installed");
+            }
+        }
+
         # MySQL must have InnoDB support
         local $dbh->{FetchHashKeyName} = 'NAME_lc';
         my $innodb = lc($dbh->selectall_hashref("SHOW ENGINES", "engine")->{InnoDB}{support} || "no");
diff --git a/lib/RT/Principal.pm b/lib/RT/Principal.pm
index aec96f7a2e..afddbe6bcd 100644
--- a/lib/RT/Principal.pm
+++ b/lib/RT/Principal.pm
@@ -422,8 +422,9 @@ sub HasRights {
     }
     my $roles;
     {
+        my $groups_table = $self->_MaybeQuoteName('Groups');
         my $query
-            = "SELECT DISTINCT Groups.Name "
+            = "SELECT DISTINCT $groups_table.Name "
             . $self->_HasRoleRightQuery(
                 EquivObjects => $args{'EquivObjects'}
             );
@@ -555,8 +556,8 @@ sub _HasRoleRight {
 
     my @roles = $self->RolesWithRight(%args);
     return 0 unless @roles;
-
-    my $query = "SELECT Groups.id "
+    my $groups_table = $self->_MaybeQuoteName('Groups');
+    my $query = "SELECT $groups_table.id "
         . $self->_HasRoleRightQuery( %args, Roles => \@roles );
 
     $self->_Handle->ApplyLimits( \$query, 1 );
@@ -574,14 +575,15 @@ sub _HasRoleRightQuery {
                  @_
                );
 
+    my $groups_table = $self->_MaybeQuoteName('Groups');
     my $query =
-        " FROM Groups, Principals, CachedGroupMembers WHERE "
+        " FROM $groups_table, Principals, CachedGroupMembers WHERE "
 
         # Never find disabled things
         . "Principals.Disabled = 0 " . "AND CachedGroupMembers.Disabled = 0 "
 
         # We always grant rights to Groups
-        . "AND Principals.id = Groups.id "
+        . "AND Principals.id = $groups_table.id "
         . "AND Principals.PrincipalType = 'Group' "
 
 # See if the principal is a member of the group recursively or _is the rightholder_
@@ -594,7 +596,7 @@ sub _HasRoleRightQuery {
 
     if ( $args{'Roles'} ) {
         $query .= "AND (" . join( ' OR ',
-            map $RT::Handle->__MakeClauseCaseInsensitive('Groups.Name', '=', "'$_'"),
+            map $RT::Handle->__MakeClauseCaseInsensitive("$groups_table.Name", '=', "'$_'"),
             @{ $args{'Roles'} }
         ) . ")";
     }
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 3503f67f30..261a85b16b 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -1600,6 +1600,7 @@ entire database.
 sub LockForUpdate {
     my $self = shift;
 
+    my $table = $self->_MaybeQuoteName($self->Table);
     my $pk = $self->_PrimaryKey;
     my $id = @_ ? $_[0] : $self->$pk;
     $self->_expire if $self->isa("DBIx::SearchBuilder::Record::Cachable");
@@ -1608,12 +1609,11 @@ sub LockForUpdate {
         # "RESERVED" on the first UPDATE/INSERT/DELETE.  Do a no-op
         # UPDATE to force the upgade.
         return RT->DatabaseHandle->dbh->do(
-            "UPDATE " .$self->Table.
-                " SET $pk = $pk WHERE 1 = 0");
+            "UPDATE $table SET $pk = $pk WHERE 1 = 0"
+        );
     } else {
         return $self->_LoadFromSQL(
-            "SELECT * FROM ".$self->Table
-                ." WHERE $pk = ? FOR UPDATE",
+            "SELECT * FROM $table WHERE $pk = ? FOR UPDATE",
             $id,
         );
     }
@@ -2582,6 +2582,16 @@ sub PreInflate {
     return 1;
 }
 
+sub _MaybeQuoteName {
+    my ($self, $name) = @_;
+
+    # no action unless DBIx::SearchBuilder quoting supported and enabled
+    return $name
+        unless (defined($self->_Handle->{'QuoteTableNames'}) && $self->_Handle->{'QuoteTableNames'});
+
+    return $self->QuotedTableName($name);
+}
+
 sub PostInflate {
 }
 
diff --git a/lib/RT/Users.pm b/lib/RT/Users.pm
index 17d5f6df35..36ca8f75a0 100644
--- a/lib/RT/Users.pm
+++ b/lib/RT/Users.pm
@@ -476,14 +476,16 @@ sub _RoleClauses {
     my $groups = shift;
     my @objects = @_;
 
+    my $groups_table = $self->RecordClass->_MaybeQuoteName($groups);
+
     my @groups_clauses;
     foreach my $obj ( @objects ) {
         my $type = ref($obj)? ref($obj): $obj;
 
-        my $role_clause = $RT::Handle->__MakeClauseCaseInsensitive("$groups.Domain", '=', "'$type-Role'");
+        my $role_clause = $RT::Handle->__MakeClauseCaseInsensitive("$groups_table.Domain", '=', "'$type-Role'");
 
         if ( my $id = eval { $obj->id } ) {
-            $role_clause .= " AND $groups.Instance = $id";
+            $role_clause .= " AND $groups_table.Instance = $id";
         }
         push @groups_clauses, "($role_clause)";
     }
diff --git a/sbin/rt-validator.in b/sbin/rt-validator.in
index 33767827f1..8d877d278d 100644
--- a/sbin/rt-validator.in
+++ b/sbin/rt-validator.in
@@ -97,6 +97,7 @@ END
 
 my $dbh = $RT::Handle->dbh;
 my $db_type = RT->Config->Get('DatabaseType');
+my $groups_table = ($db_type eq 'mysql') ? '`Groups`' : 'Groups' ;
 
 my %TYPE = (
     'Transactions.Field'    => 'text',
@@ -641,11 +642,11 @@ push @CHECKS, 'Tickets -> other' => sub {
     {
         my $query = <<END;
 SELECT Tickets.Id, Tickets.Owner
-FROM Groups
-JOIN Tickets ON Tickets.Id = Groups.Instance
-LEFT JOIN GroupMembers ON Groups.Id = GroupMembers.GroupId
-WHERE Groups.Name = 'Owner'
-AND Groups.Domain = 'RT::Ticket-Role'
+FROM $groups_table
+JOIN Tickets ON Tickets.Id = $groups_table.Instance
+LEFT JOIN GroupMembers ON $groups_table.Id = GroupMembers.GroupId
+WHERE $groups_table.Name = 'Owner'
+AND $groups_table.Domain = 'RT::Ticket-Role'
 AND GroupMembers.MemberId IS NULL
 END
 
@@ -678,11 +679,11 @@ END
     {
         my $query = <<END;
 SELECT Tickets.Id, Tickets.Owner, GroupMembers.MemberId
-FROM Groups
-JOIN GroupMembers ON Groups.Id = GroupMembers.GroupId
-JOIN Tickets ON Tickets.Id = Groups.Instance
-WHERE Groups.Name = 'Owner'
-AND Groups.Domain = 'RT::Ticket-Role'
+FROM $groups_table
+JOIN GroupMembers ON $groups_table.Id = GroupMembers.GroupId
+JOIN Tickets ON Tickets.Id = $groups_table.Instance
+WHERE $groups_table.Name = 'Owner'
+AND $groups_table.Domain = 'RT::Ticket-Role'
 AND Tickets.Owner != GroupMembers.MemberId
 END
 
@@ -957,6 +958,7 @@ push @CHECKS, Attributes => sub {
 
 # Fix situations when Creator or LastUpdatedBy references ACL equivalence
 # group of a user instead of user
+
 push @CHECKS, 'FIX: LastUpdatedBy and Creator' => sub {
     my $res = 1;
     my %fix = ();
@@ -967,10 +969,11 @@ push @CHECKS, 'FIX: LastUpdatedBy and Creator' => sub {
             next unless $object->_Accessible( $column, 'auto' );
 
             my $table = m2t($model);
+            $table = "`$table`" if ($db_type eq 'mysql');
             my $query = <<END;
 SELECT m.id, g.id, g.Instance
 FROM
-    Groups g JOIN $table m ON g.id = m.$column
+    $groups_table g JOIN $table m ON g.id = m.$column
 WHERE
     g.Domain = ?
     AND g.Name = ?
@@ -1191,17 +1194,18 @@ push @CHECKS, 'Links: missing object' => sub {
 
         foreach my $use ( @URI_USES ) {
             my $stable = m2t( $use->{'model'} );
+            my $stablename = ($db_type eq 'mysql') ? "`$stable`" : $stable;
             my $scolumn = $use->{'column'};
 
             foreach my $tmodel ( @models ) {
                 my $tclass = 'RT::'. $tmodel;
                 my $ttable = m2t($tmodel);
-
+                my $ttablename = ($db_type eq 'mysql') ? "`$ttable`" : $ttable;
                 my $tprefix = $prefix .'/'. ($tclass eq 'RT::Ticket'? 'ticket' : $tclass) .'/';
 
                 $tprefix = $prefix . '/article/' if $tclass eq 'RT::Article';
 
-                my $query = "SELECT s.id FROM $stable s LEFT JOIN $ttable t "
+                my $query = "SELECT s.id FROM $stablename s LEFT JOIN $ttablename t "
                   ." ON t.id = ". sql_str2int("SUBSTR(s.$scolumn, ?)")
                     ." WHERE s.$scolumn LIKE ? AND t.id IS NULL";
                 my @binds = (length($tprefix) + 1, sql_escape_like($tprefix).'%');
@@ -1223,7 +1227,6 @@ push @CHECKS, 'Links: missing object' => sub {
                         ." errors with links.\n",
                         'Link to a missing object in $ttable'
                                       );
-
                     delete_record($stable, $sid);
                 }
             }
@@ -1271,6 +1274,9 @@ sub check_integrity {
     my ($ttable, @tcols) = (shift, shift);
     my %args = @_;
 
+    my $stablename = ($db_type eq 'mysql') ? "`$stable`" : $stable;
+    my $ttablename = ($db_type eq 'mysql') ? "`$ttable`" :  $ttable;
+
     @scols = @{ $scols[0] } if ref $scols[0];
     @tcols = @{ $tcols[0] } if ref $tcols[0];
 
@@ -1278,7 +1284,7 @@ sub check_integrity {
         if $opt{'verbose'};
 
     my $query = "SELECT s.id, ". join(', ', map "s.$_", @scols)
-        ." FROM $stable s LEFT JOIN $ttable t"
+        ." FROM $stablename s LEFT JOIN $ttablename t"
         ." ON (". join(
             ' AND ', map columns_eq_cond('s', $stable, $scols[$_] => 't', $ttable, $tcols[$_]), (0..(@scols-1))
         ) .")"
@@ -1305,7 +1311,6 @@ sub check_integrity {
     my $sth = execute_query( $query, @binds );
     while ( my ($sid, @set) = $sth->fetchrow_array ) {
         $res = 0;
-
         print STDERR "Record #$sid in $stable references a nonexistent record in $ttable\n";
         for ( my $i = 0; $i < @scols; $i++ ) {
             print STDERR "\t$scols[$i] => '$set[$i]' => $tcols[$i]\n";
@@ -1348,6 +1353,7 @@ sub check_uniqueness {
     my $on = shift;
     my %args = @_;
 
+    $on = "`$on`" if ($db_type eq 'mysql');
     my @columns = @{ $args{'columns'} };
 
     print "Checking uniqueness of ( ", join(', ', map "'$_'", @columns )," ) in table '$on'\n"
@@ -1388,14 +1394,16 @@ sub check_uniqueness {
 
 sub load_record {
     my ($table, $id) = @_;
-    my $sth = execute_query( "SELECT * FROM $table WHERE id = ?", $id );
+    my $tablename = ($db_type eq 'mysql') ? "`$table`" : $table;
+    my $sth = execute_query( "SELECT * FROM $tablename WHERE id = ?", $id );
     return $sth->fetchrow_hashref('NAME_lc');
 }
 
 sub delete_record {
     my ($table, $id) = (@_);
     print "Deleting record #$id in $table\n" if $opt{'verbose'};
-    my $query = "DELETE FROM $table WHERE id = ?";
+    my $tablename = ($db_type eq 'mysql') ? "`$table`" : $table;
+    my $query = "DELETE FROM $tablename WHERE id = ?";
     $redo_check{ $_ } = 1 foreach @{ $redo_on{'Delete'}{ $table } || [] };
     return execute_query( $query, $id );
 }
@@ -1411,6 +1419,8 @@ sub update_records {
     my $where = shift;
     my $what = shift;
 
+    $table = "`$table`" if ($db_type eq 'mysql');
+
     my (@where_cols, @where_binds);
     while ( my ($k, $v) = each %$where ) { push @where_cols, $k; push @where_binds, $v; }
 

commit 461a31886f6deccea1cd4bcd2ebd7979593284c3
Author: Aaron Trevena <ast at bestpractical.com>
Date:   Tue Sep 1 16:20:41 2020 +0100

    Update tests for mysql 8
    
    Tests fail due to new Groups reserved word and other changes in mysql 8, requiring
    Groups table to be escaped, add debug output for any failing validator results.

diff --git a/t/validator/transaction.t b/t/validator/transaction.t
index 6b8a5c2980..a87533bc5b 100644
--- a/t/validator/transaction.t
+++ b/t/validator/transaction.t
@@ -24,7 +24,7 @@ my $ticket = RT::Test->create_ticket( Queue => 'General', Subject => 'test ticke
     $RT::Handle->dbh->do( "DELETE FROM ObjectCustomFieldValues where CustomField=" . $cf->id );
 
     my ( $ecode, $res ) = RT::Test->run_validator( resolve => 1 );
-    isnt( $ecode, 0, 'non-zero exit code' );
+    isnt( $ecode, 0, 'non-zero exit code' ) or diag ($res);
     like(
         $res,
         qr/Transactions references a nonexistent record in CustomFields/,
@@ -59,7 +59,7 @@ my $ticket = RT::Test->create_ticket( Queue => 'General', Subject => 'test ticke
     $RT::Handle->dbh->do( "DELETE FROM Principals where id=" . $user->PrincipalId );
 
     my ( $ecode, $res ) = RT::Test->run_validator( resolve => 1 );
-    isnt( $ecode, 0, 'non-zero exit code' );
+    isnt( $ecode, 0, 'non-zero exit code' ) or diag ($res);
     like(
         $res,
         qr/Transactions references a nonexistent record in Users/,
diff --git a/t/web/query_log.t b/t/web/query_log.t
index cfb4d81d71..17ce2ef227 100644
--- a/t/web/query_log.t
+++ b/t/web/query_log.t
@@ -15,4 +15,6 @@ $m->get_ok("/Admin/Tools/Queries.html");
 $m->text_contains("/index.html", "we include info about a page we hit while logging in");
 $m->text_contains("Stack:", "stack traces");
 $m->text_like(qr{/autohandler:\d+}, "stack trace includes mason components");
-$m->text_contains("SELECT * FROM Principals WHERE id = '".$root->id."'", "we interpolate bind params");
+
+my $id = $root->id;
+$m->text_like(qr/SELECT.\*.FROM.`?Principals`?.WHERE.id.=.'$id'/, "we interpolate bind params");

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


More information about the rt-commit mailing list