Skip to content

Commit

Permalink
MDL-33018 add general index type hints and use PostgreSQL varchar_pat…
Browse files Browse the repository at this point in the history
…tern_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.
  • Loading branch information
skodak committed Jul 6, 2012
1 parent 2cbdaa7 commit bd991d0
Show file tree
Hide file tree
Showing 15 changed files with 191 additions and 23 deletions.
3 changes: 3 additions & 0 deletions admin/tool/xmldb/actions/edit_index/edit_index.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ function invoke() {
// xmldb_index Fields
$o.= ' <tr valign="top"><td><label for="fields" accesskey="f">Fields:</label></td>';
$o.= ' <td colspan="2"><input name="fields" type="text" size="40" maxlength="80" id="fields" value="' . s(implode(', ', $index->getFields())) . '" /></td></tr>';
// xmldb_index hints
$o.= ' <tr valign="top"><td><label for="hints" accesskey="h">Hints:</label></td>';
$o.= ' <td colspan="2"><input name="hints" type="text" size="40" maxlength="80" id="hints" value="' . s(implode(', ', $index->getHints())) . '" /></td></tr>';
// Change button
$o.= ' <tr valign="top"><td>&nbsp;</td><td colspan="2"><input type="submit" value="' .$this->str['change'] . '" /></td></tr>';
$o.= ' </table>';
Expand Down
12 changes: 12 additions & 0 deletions admin/tool/xmldb/actions/edit_index_save/edit_index_save.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ function invoke() {
$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();
Expand Down Expand Up @@ -160,11 +162,20 @@ function invoke() {
}
}
}
$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 = '<p>' .implode(', ', $errors) . '</p>
<p>' . $tempindex->readableInfo() . '</p>';
Expand Down Expand Up @@ -197,6 +208,7 @@ function invoke() {
// 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
Expand Down
4 changes: 2 additions & 2 deletions lib/db/install.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1037,7 +1037,7 @@
<INDEXES>
<INDEX NAME="contextlevel-instanceid" UNIQUE="true" FIELDS="contextlevel, instanceid" NEXT="instanceid"/>
<INDEX NAME="instanceid" UNIQUE="false" FIELDS="instanceid" PREVIOUS="contextlevel-instanceid" NEXT="path"/>
<INDEX NAME="path" UNIQUE="false" FIELDS="path" PREVIOUS="instanceid"/>
<INDEX NAME="path" UNIQUE="false" FIELDS="path" HINTS="varchar_pattern_ops" PREVIOUS="instanceid"/>
</INDEXES>
</TABLE>
<TABLE NAME="context_temp" COMMENT="Used by build_context_path() in upgrade and cron to keep context depths and paths in sync." PREVIOUS="context" NEXT="capabilities">
Expand Down Expand Up @@ -2847,4 +2847,4 @@
</KEYS>
</TABLE>
</TABLES>
</XMLDB>
</XMLDB>
18 changes: 18 additions & 0 deletions lib/db/upgrade.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
17 changes: 14 additions & 3 deletions lib/ddl/database_manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,11 @@ public function field_exists($table, $field) {
*
* @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();

Expand All @@ -183,17 +184,27 @@ public function find_index_name(xmldb_table $xmldb_table, xmldb_index $xmldb_ind
// 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'];
// Check if index matches queried index
$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;
}
Expand Down
36 changes: 34 additions & 2 deletions lib/ddl/postgres_sql_generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,37 @@ public function getDropTableSQL($xmldb_table) {
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.
*
Expand Down Expand Up @@ -306,7 +337,7 @@ public function getAlterFieldSQL($xmldb_table, $xmldb_field, $skip_type_clause =
$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)) {
Expand Down Expand Up @@ -417,7 +448,7 @@ function getSequenceFromDB($xmldb_table) {
if (!$this->mdb->get_record_sql("SELECT *
FROM pg_class
WHERE relname = ? AND relkind = 'S'",
array($sequencename))) {
array($sequencename))) {
$sequencename = false;
}

Expand Down Expand Up @@ -475,6 +506,7 @@ public function isNameInUse($object_name, $type, $table_name) {
*/
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',
Expand Down
13 changes: 8 additions & 5 deletions lib/ddl/sql_generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -1013,13 +1013,16 @@ public function getDropIndexSQL($xmldb_table, $xmldb_index) {
$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;
}
Expand Down
29 changes: 29 additions & 0 deletions lib/ddl/tests/ddl_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -1649,6 +1649,35 @@ public function test_reserved_words() {
$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();
Expand Down
7 changes: 6 additions & 1 deletion lib/ddl/tests/fixtures/xmldb_table.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@
<FIELD NAME="secondname" TYPE="char" LENGTH="30" NOTNULL="true" SEQUENCE="false" PREVIOUS="name" NEXT="intro"/>
<FIELD NAME="intro" TYPE="text" LENGTH="medium" NOTNULL="true" SEQUENCE="false" PREVIOUS="secondname" NEXT="avatar"/>
<FIELD NAME="avatar" TYPE="binary" LENGTH="medium" NOTNULL="false" UNSIGNED="false" SEQUENCE="false" PREVIOUS="intro" NEXT="grade"/>
<FIELD NAME="grade" TYPE="number" LENGTH="20" DECIMALS="10" NOTNULL="false" SEQUENCE="false" PREVIOUS="avatar"/>
<FIELD NAME="grade" TYPE="number" LENGTH="20" DECIMALS="10" NOTNULL="false" SEQUENCE="false" PREVIOUS="avatar" NEXT="path"/>
<FIELD NAME="path" TYPE="char" LENGTH="255" NOTNULL="true" PREVIOUS="grade"/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id" />
</KEYS>
<INDEXES>
<INDEX NAME="course" UNIQUE="false" FIELDS="course" NEXT="path"/>
<INDEX NAME="path" UNIQUE="false" FIELDS="path" HINTS="varchar_pattern_ops,unknownxyz" PREVIOUS="course"/>
</INDEXES>
</TABLE>
</TABLES>
</XMLDB>
9 changes: 8 additions & 1 deletion lib/dml/pgsql_native_moodle_database.php
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,14 @@ public function get_indexes($table) {
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);
}
Expand Down
1 change: 1 addition & 0 deletions lib/xmldb/xmldb.dtd
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

<!ELEMENT INDEX EMPTY >
<!ATTLIST INDEX COMMENT CDATA #IMPLIED >
<!ATTLIST INDEX HINTS CDATA #IMPLIED >
<!ATTLIST INDEX FIELDS CDATA #REQUIRED >
<!ATTLIST INDEX NAME NMTOKEN #REQUIRED >
<!ATTLIST INDEX NEXT NMTOKEN #IMPLIED >
Expand Down
1 change: 1 addition & 0 deletions lib/xmldb/xmldb.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
<xs:attribute name="NAME" type="xs:NMTOKEN" use="required" />
<xs:attribute name="UNIQUE" type="trueFalse" use="required" />
<xs:attribute name="FIELDS" type="fieldsList" use="required" />
<xs:attribute name="HINTS" type="xs:string" use="optional" />
<xs:attribute name="COMMENT" type="xs:string" use="optional" />
<xs:attribute name="PREVIOUS" type="xs:NMTOKEN" use="optional" />
<xs:attribute name="NEXT" type="xs:NMTOKEN" use="optional" />
Expand Down
Loading

0 comments on commit bd991d0

Please sign in to comment.