diff --git a/lib/ddl/sql_generator.php b/lib/ddl/sql_generator.php index 356fb719ddc..721fce7ea37 100644 --- a/lib/ddl/sql_generator.php +++ b/lib/ddl/sql_generator.php @@ -1074,8 +1074,11 @@ abstract class sql_generator { // along all the request life, but never to return cached results // We need this because sql statements are created before executing // them, hence names doesn't exist "physically" yet in DB, so we need - // to known which ones have been used - static $used_names = array(); + // to known which ones have been used. + // We track all the keys used, and the previous counters to make subsequent creates faster. + // This may happen a lot with things like bulk backups or restores. + static $usednames = array(); + static $previouscounters = array(); // Use standard naming. See http://docs.moodle.org/en/XMLDB_key_and_index_naming $tablearr = explode ('_', $tablename); @@ -1094,17 +1097,26 @@ abstract class sql_generator { $maxlengthwithoutsuffix = $this->names_max_length - strlen($suffix) - ($suffix ? 1 : 0); $namewithsuffix = substr($name, 0, $maxlengthwithoutsuffix) . ($suffix ? ('_' . $suffix) : ''); - // If the calculated name is in the cache, or if we detect it by introspecting the DB let's modify if - $counter = 1; - while (in_array($namewithsuffix, $used_names) || $this->isNameInUse($namewithsuffix, $suffix, $tablename)) { + if (isset($previouscounters[$name])) { + // If we have a counter stored, we will need to modify the key to the next counter location. + $counter = $previouscounters[$name] + 1; + $namewithsuffix = substr($name, 0, $maxlengthwithoutsuffix - strlen($counter)) . + $counter . ($suffix ? ('_' . $suffix) : ''); + } else { + $counter = 1; + } + + // If the calculated name is in the cache, or if we detect it by introspecting the DB let's modify it. + while (isset($usednames[$namewithsuffix]) || $this->isNameInUse($namewithsuffix, $suffix, $tablename)) { // Now iterate until not used name is found, incrementing the counter $counter++; $namewithsuffix = substr($name, 0, $maxlengthwithoutsuffix - strlen($counter)) . $counter . ($suffix ? ('_' . $suffix) : ''); } - // Add the name to the cache - $used_names[] = $namewithsuffix; + // Add the name to the cache. Using key look with isset because it is much faster than in_array. + $usednames[$namewithsuffix] = true; + $previouscounters[$name] = $counter; // Quote it if necessary (reserved words) $namewithsuffix = $this->getEncQuoted($namewithsuffix); diff --git a/lib/ddl/tests/ddl_test.php b/lib/ddl/tests/ddl_test.php index a0d78fdec98..edf62587b50 100644 --- a/lib/ddl/tests/ddl_test.php +++ b/lib/ddl/tests/ddl_test.php @@ -2080,6 +2080,38 @@ class core_ddl_testcase extends database_driver_testcase { strlen($gen->getNameForObject($table, $fields, $suffix)), 'Generated object name is too long. $i = '.$i); } + + // Now test to confirm that a duplicate name isn't issued, even if they come from different root names. + // Move to a new field. + $fields = "fl"; + + // Insert twice, moving is to a key with fl2. + $this->assertEquals($gen->names_max_length - 1, strlen($gen->getNameForObject($table, $fields, $suffix))); + $result1 = $gen->getNameForObject($table, $fields, $suffix); + + // Make sure we end up with _fl2_ in the result. + $this->assertRegExp('/_fl2_/', $result1); + + // Now, use a field that would result in the same key if it wasn't already taken. + $fields = "fl2"; + // Because we are now at the max key length, it will try: + // - _fl2_ (the natural name) + // - _fl2_ (removing the original 2, and adding a counter 2) + // - then settle on _fl3_. + $result2 = $gen->getNameForObject($table, $fields, $suffix); + $this->assertRegExp('/_fl3_/', $result2); + + // Make sure they don't match. + $this->assertNotEquals($result1, $result2); + // But are only different in the way we expect. This confirms the test is working properly. + $this->assertEquals(str_replace('_fl2_', '', $result1), str_replace('_fl3_', '', $result2)); + + // Now go back. We would expect the next result to be fl3 again, but it is taken, so it should move to fl4. + $fields = "fl"; + $result3 = $gen->getNameForObject($table, $fields, $suffix); + + $this->assertNotEquals($result2, $result3); + $this->assertRegExp('/_fl4_/', $result3); } }