Moodle
  1. Moodle
  2. MDL-34211

mysql_sql_generator method isNameInUse() always returns false

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.3.1, 2.4
    • Fix Version/s: 2.3.2
    • Component/s: Database SQL/XMLDB
    • Labels:
      None
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      This is very tricky because there is a local static cache for object names that prevents us writing proper phpunit tests. SO code review only.

      Show
      This is very tricky because there is a local static cache for object names that prevents us writing proper phpunit tests. SO code review only.
    • Workaround:
      Hide

      Change indexed column names so first four characters unique

      Show
      Change indexed column names so first four characters unique
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Rank:
      42550

      Description

      The method uses the wrong form of the table name when checking the $metatables array for the presence of a particular key.

      The $metatables array is keyed on the bare table name, e.g. `cohort` or `cohort_members`, without the table prefix (usually mdl_). But the variable used to check for the existence of the table in that array, $tname, is assigned the value from the getTableName() method which returns the name of the table WITH the prefix.

      The table name is never found, and so any check done for an exsiting table, index, or key name fails, which will lead to a DDL error due to name collisions becasue the getNameForObject() method in the parent sql_generator class will not append an integer to the name when needed.

      The proper form of the table name is passed in as an argument. Using that value would correct the problem.

      This problem will not usually manifest unless there are two or more column names in a table with the same first four characters, e.g. userid_sctid, userid_logon, userid_moodle. When trying to create indexes of the same type (resulting in the same suffix name, _uix) on more than one of those columns will fail due to a name collision.

        Activity

        Hide
        Fred Woolard added a comment -

        — a/lib/ddl/mysql_sql_generator.php
        +++ b/lib/ddl/mysql_sql_generator.php
        @@ -333,18 +333,14 @@ class mysql_sql_generator extends sql_generator {
        */
        public function isNameInUse($object_name, $type, $table_name) {

        • // Calculate the real table name
        • $xmldb_table = new xmldb_table($table_name);
        • $tname = $this->getTableName($xmldb_table);
          -
          switch($type) {
          case 'ix':
          case 'uix':
          // First of all, check table exists
          $metatables = $this->mdb->get_tables();
        • if (isset($metatables[$tname])) {
          + if (isset($metatables[$table_name])) {
          // Fetch all the indexes in the table
        • if ($indexes = $this->mdb->get_indexes($tname)) {
          + if ($indexes = $this->mdb->get_indexes($table_name)) {
          // Look for existing index in array
          if (isset($indexes[$object_name])) {
          return true;
        Show
        Fred Woolard added a comment - — a/lib/ddl/mysql_sql_generator.php +++ b/lib/ddl/mysql_sql_generator.php @@ -333,18 +333,14 @@ class mysql_sql_generator extends sql_generator { */ public function isNameInUse($object_name, $type, $table_name) { // Calculate the real table name $xmldb_table = new xmldb_table($table_name); $tname = $this->getTableName($xmldb_table); - switch($type) { case 'ix': case 'uix': // First of all, check table exists $metatables = $this->mdb->get_tables(); if (isset($metatables [$tname] )) { + if (isset($metatables [$table_name] )) { // Fetch all the indexes in the table if ($indexes = $this->mdb->get_indexes($tname)) { + if ($indexes = $this->mdb->get_indexes($table_name)) { // Look for existing index in array if (isset($indexes [$object_name] )) { return true;
        Hide
        Petr Škoda added a comment -

        Thanks a lot for the report!

        This problem appears only if you change existing table that already had one of these indexes created.

        Show
        Petr Škoda added a comment - Thanks a lot for the report! This problem appears only if you change existing table that already had one of these indexes created.
        Hide
        Dan Poltawski added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Hide
        Dan Poltawski added a comment -

        Integrated this, thanks Fred!

        Show
        Dan Poltawski added a comment - Integrated this, thanks Fred!
        Hide
        Dan Poltawski added a comment -

        Code looks good to me.

        Show
        Dan Poltawski added a comment - Code looks good to me.
        Hide
        Dan Poltawski added a comment -

        *Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

        Congratulations

        {tracker.user.name}

        !

        You've made into Moodle

        {tracker.fixversion-1}

        +

        I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

        cheers!

        {tracker.friendlyintegrator}
        Show
        Dan Poltawski added a comment - * Notice *: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26 Congratulations {tracker.user.name} ! You've made into Moodle {tracker.fixversion-1} + I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world. cheers! {tracker.friendlyintegrator}

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: