Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-34211

mysql_sql_generator method isNameInUse() always returns false

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Activity

          Hide
          woolardfa@appstate.edu 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
          woolardfa@appstate.edu 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
          skodak Petr Skoda 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
          skodak Petr Skoda 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
          poltawski 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
          poltawski 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
          poltawski Dan Poltawski added a comment -

          Integrated this, thanks Fred!

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

          Code looks good to me.

          Show
          poltawski Dan Poltawski added a comment - Code looks good to me.
          Hide
          poltawski 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
          poltawski 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:
                Fix Release Date:
                10/Sep/12