Skip to content

Commit

Permalink
MDL-63020 ddl: fix nullable unique indexes in OCI and MS SQL
Browse files Browse the repository at this point in the history 
This works-around the default non-standard behaviour of these DB engines.
  • Loading branch information
@timhunt
timhunt committed Sep 20, 2018
1 parent 54b2b1d commit 5abc431
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 0 deletions.
30 changes: 30 additions & 0 deletions lib/ddl/mssql_sql_generator.php
View file
Expand Up @@ -140,6 +140,36 @@ public function getTableName(xmldb_table $xmldb_table, $quoted=true) {
return $tablename;
}

public function getCreateIndexSQL($xmldb_table, $xmldb_index) {
list($indexsql) = parent::getCreateIndexSQL($xmldb_table, $xmldb_index);

// Unique indexes need to work-around non-standard SQL server behaviour.
if ($xmldb_index->getUnique()) {
// Find any nullable columns. We need to add a
// WHERE field IS NOT NULL to the index definition for each one.
//
// For example if you have a unique index on the three columns
// (required, option1, option2) where the first one is non-null,
// and the others nullable, then the SQL will end up as
//
// CREATE UNIQUE INDEX index_name ON table_name (required, option1, option2)
// WHERE option1 IS NOT NULL AND option2 IS NOT NULL
//
// The first line comes from parent calls above. The WHERE is added below.
$extraconditions = [];
foreach ($this->get_nullable_fields_in_index($xmldb_table, $xmldb_index) as $fieldname) {
$extraconditions[] = $this->getEncQuoted($fieldname) .
' IS NOT NULL';
}

if ($extraconditions) {
$indexsql .= ' WHERE ' . implode(' AND ', $extraconditions);
}
}

return [$indexsql];
}

/**
* Given one correct xmldb_table, returns the SQL statements
* to create temporary table (inside one array).
Expand Down
55 changes: 55 additions & 0 deletions lib/ddl/oracle_sql_generator.php
View file
Expand Up @@ -131,6 +131,61 @@ public function getTableName(xmldb_table $xmldb_table, $quoted=true) {
return $tablename;
}

public function getCreateIndexSQL($xmldb_table, $xmldb_index) {
if ($error = $xmldb_index->validateDefinition($xmldb_table)) {
throw new coding_exception($error);
}

$indexfields = $this->getEncQuoted($xmldb_index->getFields());

$unique = '';
$suffix = 'ix';
if ($xmldb_index->getUnique()) {
$unique = ' UNIQUE';
$suffix = 'uix';

$nullablefields = $this->get_nullable_fields_in_index($xmldb_table, $xmldb_index);
if ($nullablefields) {
// If this is a unique index with nullable fields, then we have to
// apply the work-around from https://community.oracle.com/message/9518046#9518046.
//
// For example if you have a unique index on the three columns
// (required, option1, option2) where the first one is non-null,
// and the others nullable, then the SQL will end up as
//
// CREATE UNIQUE INDEX index_name ON table_name (
// CASE WHEN option1 IS NOT NULL AND option2 IS NOT NULL THEN required ELSE NULL END,
// CASE WHEN option1 IS NOT NULL AND option2 IS NOT NULL THEN option1 ELSE NULL END,
// CASE WHEN option1 IS NOT NULL AND option2 IS NOT NULL THEN option2 ELSE NULL END)
//
// Basically Oracle behaves according to the standard if either
// none of the columns are NULL or all columns contain NULL. Therefore,
// if any column is NULL, we treat them all as NULL for the index.
$conditions = [];
foreach ($nullablefields as $fieldname) {
$conditions[] = $this->getEncQuoted($fieldname) .
' IS NOT NULL';
}
$condition = implode(' AND ', $conditions);

$updatedindexfields = [];
foreach ($indexfields as $fieldname) {
$updatedindexfields[] = 'CASE WHEN ' . $condition . ' THEN ' .
$fieldname . ' ELSE NULL END';
}
$indexfields = $updatedindexfields;
}

}

$index = 'CREATE' . $unique . ' INDEX ';
$index .= $this->getNameForObject($xmldb_table->getName(), implode(', ', $xmldb_index->getFields()), $suffix);
$index .= ' ON ' . $this->getTableName($xmldb_table);
$index .= ' (' . implode(', ', $indexfields) . ')';

return array($index);
}

/**
* Given one correct xmldb_table, returns the SQL statements
* to create temporary table (inside one array).
Expand Down
40 changes: 40 additions & 0 deletions lib/ddl/sql_generator.php
View file
Expand Up @@ -1408,4 +1408,44 @@ public function addslashes($s) {
$s = str_replace("'", "\\'", $s);
return $s;
}

/**
* Get the fields from an index definition that might be null.
* @param xmldb_table $xmldb_table the table
* @param xmldb_index $xmldb_index the index
* @return array list of fields in the index definition that might be null.
*/
public function get_nullable_fields_in_index($xmldb_table, $xmldb_index) {
global $DB;

// If we don't have the field info passed in, we need to query it from the DB.
$fieldsfromdb = null;

$nullablefields = [];
foreach ($xmldb_index->getFields() as $fieldname) {
if ($field = $xmldb_table->getField($fieldname)) {
// We have the field details in the table definition.
if ($field->getNotNull() !== XMLDB_NOTNULL) {
$nullablefields[] = $fieldname;
}

} else {
// We don't have the table definition loaded. Need to
// inspect the database.
if ($fieldsfromdb === null) {
$fieldsfromdb = $DB->get_columns($xmldb_table->getName(), false);
}
if (!isset($fieldsfromdb[$fieldname])) {
throw new coding_exception('Unknown field ' . $fieldname .
' in index ' . $xmldb_index->getName());
}

if (!$fieldsfromdb[$fieldname]->not_null) {
$nullablefields[] = $fieldname;
}
}
}

return $nullablefields;
}
}
22 changes: 22 additions & 0 deletions lib/ddl/tests/ddl_test.php
View file
Expand Up @@ -2324,6 +2324,28 @@ public function test_sql_generator_get_rename_field_sql($reserved, $oldcolumnnam
}
}

public function test_get_nullable_fields_in_index() {
$DB = $this->tdb;
$gen = $DB->get_manager()->generator;

$indexwithoutnulls = $this->tables['test_table0']->getIndex('type-name');
$this->assertSame([], $gen->get_nullable_fields_in_index(
$this->tables['test_table0'], $indexwithoutnulls));

$indexwithnulls = new xmldb_index('course-grade', XMLDB_INDEX_UNIQUE, ['course', 'grade']);
$this->assertSame(['grade'], $gen->get_nullable_fields_in_index(
$this->tables['test_table0'], $indexwithnulls));

$this->create_deftable('test_table0');

// Now test using a minimal xmldb_table, to ensure we get the data from the DB.
$table = new xmldb_table('test_table0');
$this->assertSame([], $gen->get_nullable_fields_in_index(
$table, $indexwithoutnulls));
$this->assertSame(['grade'], $gen->get_nullable_fields_in_index(
$table, $indexwithnulls));
}

// Following methods are not supported == Do not test.
/*
public function testRenameIndex() {
Expand Down
38 changes: 38 additions & 0 deletions lib/dml/tests/dml_test.php
View file
Expand Up @@ -2407,6 +2407,44 @@ public function test_insert_records() {
}
}

public function test_insert_record_with_nullable_unique_index() {
$DB = $this->tdb;
$dbman = $DB->get_manager();

$table = $this->get_test_table();
$tablename = $table->getName();

$table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
$table->add_field('notnull1', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0');
$table->add_field('nullable1', XMLDB_TYPE_INTEGER, '10', null, null, null, null);
$table->add_field('nullable2', XMLDB_TYPE_INTEGER, '10', null, null, null, null);
$table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
$table->add_index('notnull1-nullable1-nullable2', XMLDB_INDEX_UNIQUE,
array('notnull1', 'nullable1', 'nullable2'));
$dbman->create_table($table);

// Insert one record. Should be OK (no exception).
$DB->insert_record($tablename, (object) ['notnull1' => 1, 'nullable1' => 1, 'nullable2' => 1]);

// Inserting a duplicate should fail.
try {
$DB->insert_record($tablename, (object) ['notnull1' => 1, 'nullable1' => 1, 'nullable2' => 1]);
$this->fail('dml_write_exception expected when a record violates a unique index');
} catch (moodle_exception $e) {
$this->assertInstanceOf('dml_write_exception', $e);
}

// Inserting a record with nulls in the nullable columns should work.
$DB->insert_record($tablename, (object) ['notnull1' => 1, 'nullable1' => null, 'nullable2' => null]);

// And it should be possible to insert a duplicate.
$DB->insert_record($tablename, (object) ['notnull1' => 1, 'nullable1' => null, 'nullable2' => null]);

// Same, but with only one of the nullable columns being null.
$DB->insert_record($tablename, (object) ['notnull1' => 1, 'nullable1' => 1, 'nullable2' => null]);
$DB->insert_record($tablename, (object) ['notnull1' => 1, 'nullable1' => 1, 'nullable2' => null]);
}

public function test_import_record() {
// All the information in this test is fetched from DB by get_recordset() so we
// have such method properly tested against nulls, empties and friends...
Expand Down

0 comments on commit 5abc431

Please sign in to comment.