From bd991d03cf0135dbd6d00c5ddaf1392ff6dbc405 Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Sun, 20 May 2012 10:50:14 +0200 Subject: [PATCH] MDL-33018 add general index type hints and use PostgreSQL varchar_pattern_ops index type for context.path This significantly improves performance of accesslib queries, credit for the discovery of this solution goes to Andrew Masterton from OU. --- .../actions/edit_index/edit_index.class.php | 3 + .../edit_index_save/edit_index_save.class.php | 12 ++++ lib/db/install.xml | 4 +- lib/db/upgrade.php | 18 ++++++ lib/ddl/database_manager.php | 17 +++++- lib/ddl/postgres_sql_generator.php | 36 +++++++++++- lib/ddl/sql_generator.php | 13 +++-- lib/ddl/tests/ddl_test.php | 29 ++++++++++ lib/ddl/tests/fixtures/xmldb_table.xml | 7 ++- lib/dml/pgsql_native_moodle_database.php | 9 ++- lib/xmldb/xmldb.dtd | 1 + lib/xmldb/xmldb.xsd | 1 + lib/xmldb/xmldb_index.php | 57 +++++++++++++++++-- lib/xmldb/xmldb_table.php | 5 +- version.php | 2 +- 15 files changed, 191 insertions(+), 23 deletions(-) diff --git a/admin/tool/xmldb/actions/edit_index/edit_index.class.php b/admin/tool/xmldb/actions/edit_index/edit_index.class.php index 42b020e1925..7725c9e0e38 100644 --- a/admin/tool/xmldb/actions/edit_index/edit_index.class.php +++ b/admin/tool/xmldb/actions/edit_index/edit_index.class.php @@ -125,6 +125,9 @@ class edit_index extends XMLDBAction { // xmldb_index Fields $o.= ' '; $o.= ' '; + // xmldb_index hints + $o.= ' '; + $o.= ' '; // Change button $o.= '  '; $o.= ' '; diff --git a/admin/tool/xmldb/actions/edit_index_save/edit_index_save.class.php b/admin/tool/xmldb/actions/edit_index_save/edit_index_save.class.php index 87d2804bb1f..515cbbbe7b9 100644 --- a/admin/tool/xmldb/actions/edit_index_save/edit_index_save.class.php +++ b/admin/tool/xmldb/actions/edit_index_save/edit_index_save.class.php @@ -90,6 +90,8 @@ class edit_index_save extends XMLDBAction { $unique = required_param('unique', PARAM_INT); $fields = required_param('fields', PARAM_CLEAN); $fields = str_replace(' ', '', trim(strtolower($fields))); + $hints = required_param('hints', PARAM_CLEAN); + $hints = str_replace(' ', '', trim(strtolower($hints))); $editeddir = $XMLDB->editeddirs[$dirpath]; $structure = $editeddir->xml_file->getStructure(); @@ -160,11 +162,20 @@ class edit_index_save extends XMLDBAction { } } } + $hintsarr = array(); + foreach (explode(',', $hints) as $hint) { + $hint = preg_replace('/[^a-z]/', '', $hint); + if ($hint === '') { + continue; + } + $hintsarr[] = $hint; + } if (!empty($errors)) { $tempindex = new xmldb_index($name); $tempindex->setUnique($unique); $tempindex->setFields($fieldsarr); + $tempindex->setHints($hintsarr); // Prepare the output $o = '

' .implode(', ', $errors) . '

' . $tempindex->readableInfo() . '

'; @@ -197,6 +208,7 @@ class edit_index_save extends XMLDBAction { // Set the rest of fields $index->setUnique($unique); $index->setFields($fieldsarr); + $index->setHints($hintsarr); // If the hash has changed from the old one, change the version // and mark the structure as changed diff --git a/lib/db/install.xml b/lib/db/install.xml index 1d88b4d12cc..55eb8540cd7 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1037,7 +1037,7 @@ - + @@ -2847,4 +2847,4 @@
- \ No newline at end of file + diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index f50ae6b2798..95b021aac49 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -899,6 +899,24 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2012062500.02); } + if ($oldversion < 2012062500.04) { + + // Define index path (not unique) to be added to context + $table = new xmldb_table('context'); + $index = new xmldb_index('path', XMLDB_INDEX_NOTUNIQUE, array('path'), array('varchar_pattern_ops')); + + // Recreate index with new pattern hint + if ($DB->get_dbfamily() === 'postgres') { + if ($dbman->index_exists($table, $index)) { + $dbman->drop_index($table, $index); + } + $dbman->add_index($table, $index); + } + + // Main savepoint reached + upgrade_main_savepoint(true, 2012062500.04); + } + return true; } diff --git a/lib/ddl/database_manager.php b/lib/ddl/database_manager.php index b8e0f823aeb..ea2621fb803 100644 --- a/lib/ddl/database_manager.php +++ b/lib/ddl/database_manager.php @@ -165,10 +165,11 @@ class database_manager { * * @param xmldb_table $xmldb_table table to be searched * @param xmldb_index $xmldb_index the index to be searched - * @return string|bool Index name or false if no indexes are found. + * @param bool $returnall true means return array of all indexes, false means first index only as string + * @return array|string|bool Index name, array of index names or false if no indexes are found. * @throws ddl_table_missing_exception Thrown when table is not found. */ - public function find_index_name(xmldb_table $xmldb_table, xmldb_index $xmldb_index) { + public function find_index_name(xmldb_table $xmldb_table, xmldb_index $xmldb_index, $returnall = false) { // Calculate the name of the table $tablename = $xmldb_table->getName(); @@ -183,6 +184,8 @@ class database_manager { // Get list of indexes in table $indexes = $this->mdb->get_indexes($tablename); + $return = array(); + // Iterate over them looking for columns coincidence foreach ($indexes as $indexname => $index) { $columns = $index['columns']; @@ -190,10 +193,18 @@ class database_manager { $diferences = array_merge(array_diff($columns, $indcolumns), array_diff($indcolumns, $columns)); // If no differences, we have find the index if (empty($diferences)) { - return $indexname; + if ($returnall) { + $return[] = $indexname; + } else { + return $indexname; + } } } + if ($return and $returnall) { + return $return; + } + // Arriving here, index not found return false; } diff --git a/lib/ddl/postgres_sql_generator.php b/lib/ddl/postgres_sql_generator.php index 8d410868e01..2b44baaf2f7 100644 --- a/lib/ddl/postgres_sql_generator.php +++ b/lib/ddl/postgres_sql_generator.php @@ -118,6 +118,37 @@ class postgres_sql_generator extends sql_generator { return $sqlarr; } + /** + * Given one correct xmldb_index, returns the SQL statements + * needed to create it (in array). + * + * @param xmldb_table $xmldb_table The xmldb_table instance to create the index on. + * @param xmldb_index $xmldb_index The xmldb_index to create. + * @return array An array of SQL statements to create the index. + * @throws coding_exception Thrown if the xmldb_index does not validate with the xmldb_table. + */ + public function getCreateIndexSQL($xmldb_table, $xmldb_index) { + $sqls = parent::getCreateIndexSQL($xmldb_table, $xmldb_index); + + $hints = $xmldb_index->getHints(); + $fields = $xmldb_index->getFields(); + if (in_array('varchar_pattern_ops', $hints) and count($fields) == 1) { + // Add the pattern index and keep the normal one. + foreach ($sqls as $sql) { + $field = reset($fields); + $count = 0; + $newindex = preg_replace("/^CREATE INDEX ([a-z0-9_]+) ON ([a-z0-9_]+) \($field\)$/", "CREATE INDEX \\1_pattern ON \\2 USING btree ($field varchar_pattern_ops)", $sql, -1, $count); + if ($count != 1) { + debugging('Unexpected getCreateIndexSQL() structure.'); + continue; + } + $sqls[] = $newindex; + } + } + + return $sqls; + } + /** * Given one XMLDB Type, length and decimals, returns the DB proper SQL type. * @@ -306,7 +337,7 @@ class postgres_sql_generator extends sql_generator { $results[] = 'ALTER TABLE ' . $tablename . ' ALTER COLUMN ' . $fieldname . ' DROP DEFAULT'; // Drop default clause } $alterstmt = 'ALTER TABLE ' . $tablename . ' ALTER COLUMN ' . $this->getEncQuoted($xmldb_field->getName()) . - ' TYPE' . $this->getFieldSQL($xmldb_table, $xmldb_field, null, true, true, null, false); + ' TYPE' . $this->getFieldSQL($xmldb_table, $xmldb_field, null, true, true, null, false); // Some castings must be performed explicitly (mainly from text|char to numeric|integer) if (($oldmetatype == 'C' || $oldmetatype == 'X') && ($xmldb_field->getType() == XMLDB_TYPE_NUMBER || $xmldb_field->getType() == XMLDB_TYPE_FLOAT)) { @@ -417,7 +448,7 @@ class postgres_sql_generator extends sql_generator { if (!$this->mdb->get_record_sql("SELECT * FROM pg_class WHERE relname = ? AND relkind = 'S'", - array($sequencename))) { + array($sequencename))) { $sequencename = false; } @@ -475,6 +506,7 @@ class postgres_sql_generator extends sql_generator { */ public static function getReservedWords() { // This file contains the reserved words for PostgreSQL databases + // This file contains the reserved words for PostgreSQL databases // http://www.postgresql.org/docs/current/static/sql-keywords-appendix.html $reserved_words = array ( 'all', 'analyse', 'analyze', 'and', 'any', 'array', 'as', 'asc', diff --git a/lib/ddl/sql_generator.php b/lib/ddl/sql_generator.php index 9157f1ebfeb..0555bf36988 100644 --- a/lib/ddl/sql_generator.php +++ b/lib/ddl/sql_generator.php @@ -1013,13 +1013,16 @@ abstract class sql_generator { $results = array(); // Get the real index name - $dbindexname = $this->mdb->get_manager()->find_index_name($xmldb_table, $xmldb_index); + $dbindexnames = $this->mdb->get_manager()->find_index_name($xmldb_table, $xmldb_index, true); // Replace TABLENAME and INDEXNAME as needed - $dropsql = str_replace('TABLENAME', $this->getTableName($xmldb_table), $this->drop_index_sql); - $dropsql = str_replace('INDEXNAME', $this->getEncQuoted($dbindexname), $dropsql); - - $results[] = $dropsql; + if ($dbindexnames) { + foreach ($dbindexnames as $dbindexname) { + $dropsql = str_replace('TABLENAME', $this->getTableName($xmldb_table), $this->drop_index_sql); + $dropsql = str_replace('INDEXNAME', $this->getEncQuoted($dbindexname), $dropsql); + $results[] = $dropsql; + } + } return $results; } diff --git a/lib/ddl/tests/ddl_test.php b/lib/ddl/tests/ddl_test.php index 19f30a65c60..4ad5e359dc9 100644 --- a/lib/ddl/tests/ddl_test.php +++ b/lib/ddl/tests/ddl_test.php @@ -1649,6 +1649,35 @@ class ddl_testcase extends database_driver_testcase { $this->assertTrue(count($reserved) > 1); } + public function test_index_hints() { + $DB = $this->tdb; + $dbman = $DB->get_manager(); + + $table = new xmldb_table('testtable'); + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('name', XMLDB_TYPE_CHAR, 255, null, XMLDB_NOTNULL, null); + $table->add_field('path', XMLDB_TYPE_CHAR, 255, null, XMLDB_NOTNULL, null); + $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); + $table->add_index('name', XMLDB_INDEX_NOTUNIQUE, array('name'), array('xxxx,yyyy')); + $table->add_index('path', XMLDB_INDEX_NOTUNIQUE, array('path'), array('varchar_pattern_ops')); + + // Drop if exists + if ($dbman->table_exists($table)) { + $dbman->drop_table($table); + } + $dbman->create_table($table); + $tablename = $table->getName(); + $this->tables[$tablename] = $table; + + $table = new xmldb_table('testtable'); + $index = new xmldb_index('name', XMLDB_INDEX_NOTUNIQUE, array('name'), array('xxxx,yyyy')); + $this->assertTrue($dbman->index_exists($table, $index)); + + $table = new xmldb_table('testtable'); + $index = new xmldb_index('path', XMLDB_INDEX_NOTUNIQUE, array('path'), array('varchar_pattern_ops')); + $this->assertTrue($dbman->index_exists($table, $index)); + } + public function test_index_max_bytes() { $DB = $this->tdb; $dbman = $DB->get_manager(); diff --git a/lib/ddl/tests/fixtures/xmldb_table.xml b/lib/ddl/tests/fixtures/xmldb_table.xml index 08d64c5838e..fca74e2a5f0 100644 --- a/lib/ddl/tests/fixtures/xmldb_table.xml +++ b/lib/ddl/tests/fixtures/xmldb_table.xml @@ -9,11 +9,16 @@ - + + + + + + diff --git a/lib/dml/pgsql_native_moodle_database.php b/lib/dml/pgsql_native_moodle_database.php index 0c9b18e21f8..34105ed6fb8 100644 --- a/lib/dml/pgsql_native_moodle_database.php +++ b/lib/dml/pgsql_native_moodle_database.php @@ -353,7 +353,14 @@ class pgsql_native_moodle_database extends moodle_database { continue; } $columns = explode(',', $matches[4]); - $columns = array_map(array($this, 'trim_quotes'), $columns); + foreach ($columns as $k=>$column) { + $column = trim($column); + if ($pos = strpos($column, ' ')) { + // index type is separated by space + $column = substr($column, 0, $pos); + } + $columns[$k] = $this->trim_quotes($column); + } $indexes[$row['indexname']] = array('unique'=>!empty($matches[1]), 'columns'=>$columns); } diff --git a/lib/xmldb/xmldb.dtd b/lib/xmldb/xmldb.dtd index 20bad535c7b..87d17e39322 100644 --- a/lib/xmldb/xmldb.dtd +++ b/lib/xmldb/xmldb.dtd @@ -14,6 +14,7 @@ + diff --git a/lib/xmldb/xmldb.xsd b/lib/xmldb/xmldb.xsd index d30597eae92..d144882f06c 100644 --- a/lib/xmldb/xmldb.xsd +++ b/lib/xmldb/xmldb.xsd @@ -85,6 +85,7 @@ + diff --git a/lib/xmldb/xmldb_index.php b/lib/xmldb/xmldb_index.php index c578a00d5e3..83b67bb2687 100644 --- a/lib/xmldb/xmldb_index.php +++ b/lib/xmldb/xmldb_index.php @@ -34,6 +34,9 @@ class xmldb_index extends xmldb_object { /** @var array index fields */ protected $fields; + /** @var array index hints */ + protected $hints; + /** * Note: * - MySQL: MyISAM has a limit of 1000 bytes for any key including composed, InnoDB has limit 3500 bytes. @@ -54,14 +57,16 @@ class xmldb_index extends xmldb_object { * Creates one new xmldb_index * * @param string $name - * @param string type XMLDB_INDEX_UNIQUE, XMLDB_INDEX_NOTUNIQUE - * @param array fields an array of fieldnames to build the index over + * @param string $type XMLDB_INDEX_UNIQUE, XMLDB_INDEX_NOTUNIQUE + * @param array $fields an array of fieldnames to build the index over + * @param array $hints an array of optional hints */ - public function __construct($name, $type=null, $fields=array()) { + public function __construct($name, $type=null, $fields=array(), $hints=array()) { $this->unique = false; $this->fields = array(); + $this->hints = array(); parent::__construct($name); - $this->set_attributes($type, $fields); + return $this->set_attributes($type, $fields, $hints); } /** @@ -69,10 +74,12 @@ class xmldb_index extends xmldb_object { * * @param string type XMLDB_INDEX_UNIQUE, XMLDB_INDEX_NOTUNIQUE * @param array fields an array of fieldnames to build the index over + * @param array $hints array of optional hints */ - public function set_attributes($type, $fields) { + public function set_attributes($type, $fields, $hints = array()) { $this->unique = !empty($type) ? true : false; $this->fields = $fields; + $this->hints = $hints; } /** @@ -107,6 +114,22 @@ class xmldb_index extends xmldb_object { return $this->fields; } + /** + * Set optional index hints. + * @param array $hints + */ + public function setHints($hints) { + $this->hints = $hints; + } + + /** + * Returns optional index hints. + * @return array + */ + public function getHints() { + return $this->hints; + } + /** * Load data from XML to the index * @param $xmlarr array @@ -173,6 +196,15 @@ class xmldb_index extends xmldb_object { // Finally, set the array of fields $this->fields = $fieldsarr; + if (isset($xmlarr['@']['HINTS'])) { + $this->hints = array(); + $hints = strtolower(trim($xmlarr['@']['HINTS'])); + if ($hints !== '') { + $hints = explode(',', $hints); + $this->hints = array_map('trim', $hints); + } + } + if (isset($xmlarr['@']['COMMENT'])) { $this->comment = trim($xmlarr['@']['COMMENT']); } @@ -201,7 +233,7 @@ class xmldb_index extends xmldb_object { if (!$this->loaded) { $this->hash = null; } else { - $key = $this->unique . implode (', ', $this->fields); + $key = $this->unique . implode (', ', $this->fields) . implode (', ', $this->hints); $this->hash = md5($key); } } @@ -220,6 +252,9 @@ class xmldb_index extends xmldb_object { } $o.= ' UNIQUE="' . $unique . '"'; $o.= ' FIELDS="' . implode(', ', $this->fields) . '"'; + if ($this->hints) { + $o.= ' HINTS="' . implode(', ', $this->hints) . '"'; + } if ($this->comment) { $o.= ' COMMENT="' . htmlspecialchars($this->comment) . '"'; } @@ -274,6 +309,12 @@ class xmldb_index extends xmldb_object { } else { $result .= 'null'; } + // Hints + $hints = $this->getHints(); + if (!empty($hints)) { + $result .= ', array(' . "'". implode("', '", $hints) . "')"; + } + // Return result return $result; } @@ -293,6 +334,10 @@ class xmldb_index extends xmldb_object { // fields $o .= ' (' . implode(', ', $this->fields) . ')'; + if ($this->hints) { + $o .= ' [' . implode(', ', $this->hints) . ']'; + } + return $o; } diff --git a/lib/xmldb/xmldb_table.php b/lib/xmldb/xmldb_table.php index b3e77d875e4..c69c2a1c6f2 100644 --- a/lib/xmldb/xmldb_table.php +++ b/lib/xmldb/xmldb_table.php @@ -813,9 +813,10 @@ class xmldb_table extends xmldb_object { * @param string $name name of the index * @param int $type XMLDB_INDEX_UNIQUE, XMLDB_INDEX_NOTUNIQUE * @param array $fields an array of fieldnames to build the index over + * @param array $hints optional index type hints */ - public function add_index($name, $type, $fields) { - $index = new xmldb_index($name, $type, $fields); + public function add_index($name, $type, $fields, $hints = array()) { + $index = new xmldb_index($name, $type, $fields, $hints); $this->addIndex($index); } diff --git a/version.php b/version.php index 0e59d871b3e..b281bb412c3 100644 --- a/version.php +++ b/version.php @@ -30,7 +30,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2012062500.03; // YYYYMMDD = weekly release date of this DEV branch +$version = 2012062500.04; // YYYYMMDD = weekly release date of this DEV branch // RR = release increments - 00 in DEV branches // .XX = incremental changes