Skip to content

Commit

Permalink
Add referenced schema to foreign key constraints (#1252)
Browse files Browse the repository at this point in the history
Also refactor tests.
  • Loading branch information
jtpalmer authored Mar 12, 2020
1 parent 7c3efcf commit 35c22c2
Show file tree
Hide file tree
Showing 8 changed files with 1,015 additions and 107 deletions.
18 changes: 16 additions & 2 deletions classes/ETL/DbModel/ForeignKeyConstraint.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
use Log;
use stdClass;

class ForeignKeyConstraint extends NamedEntity implements iEntity
class ForeignKeyConstraint extends SchemaEntity implements iEntity
{
/**
* Properties required by this class. These will be merged with other
Expand All @@ -34,6 +34,7 @@ class ForeignKeyConstraint extends NamedEntity implements iEntity
*/
private $localProperties = array(
'columns' => array(),
'referenced_schema' => null,
'referenced_table' => null,
'referenced_columns' => array(),
'on_delete' => null,
Expand Down Expand Up @@ -90,6 +91,7 @@ protected function filterAndVerifyValue($property, $value)
);
}
break;
case 'referenced_schema':
case 'referenced_table':
if (!is_string($value)) {
$this->logAndThrowException(
Expand Down Expand Up @@ -182,6 +184,13 @@ public function compare(iEntity $cmp)
return 1;
}

// If the referenced schema is not set then use the schema.
if (($this->referenced_schema === null ? $this->schema : $this->referenced_schema)
!= ($cmp->referenced_schema === null ? $cmp->schema : $cmp->referenced_schema)
) {
return -4;
}

if ($this->name != $cmp->name
|| $this->columns != $cmp->columns
|| $this->referenced_table != $cmp->referenced_table
Expand Down Expand Up @@ -218,7 +227,12 @@ public function getSql($includeSchema = false)
. implode(', ', array_map(array($this, 'quote'), $this->columns))
. ')';
$parts[] = 'REFERENCES';
$parts[] = $this->quote($this->referenced_table);
if ($this->referenced_schema !== null) {
$parts[] = $this->quote($this->referenced_schema) . '.'
. $this->quote($this->referenced_table);
} else {
$parts[] = $this->quote($this->referenced_table);
}
$parts[] = '('
. implode(
', ',
Expand Down
23 changes: 19 additions & 4 deletions classes/ETL/DbModel/Table.php
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ public function discover($source)
SELECT
tc.constraint_name AS name,
GROUP_CONCAT(kcu.column_name ORDER BY position_in_unique_constraint ASC) AS columns,
kcu.referenced_table_schema AS referenced_schema,
kcu.referenced_table_name AS referenced_table,
GROUP_CONCAT(kcu.referenced_column_name ORDER BY position_in_unique_constraint ASC) AS referenced_columns,
rc.update_rule AS on_update,
Expand Down Expand Up @@ -677,6 +678,12 @@ public function getSql($includeSchema = true)

$foreignKeyConstraintCreateList = array();
foreach ( $this->foreign_key_constraints as $name => $constraint ) {
// The table schema may have been set after the table was initially
// created. If the foreign key constraint doesn't explicitly define
// a schema, default to the table's schema.
if ( null === $constraint->schema ) {
$constraint->schema = $this->schema;
}
$foreignKeyConstraintCreateList[$name] = $constraint->getSql($includeSchema);
}

Expand Down Expand Up @@ -1105,10 +1112,18 @@ public function __set($property, $value)
// Clear the array no matter what, that way NULL is handled properly.
if ( null !== $value ) {
foreach ( $value as $item ) {
$constraint = ( is_object($item) && $item instanceof ForeignKeyConstraint
? $item
: new ForeignKeyConstraint($item, $this->systemQuoteChar, $this->logger) );
$this->properties[$property][$constraint->name] = $constraint;
if ( is_object($item) && $item instanceof ForeignKeyConstraint ) {
$this->properties[$property][$item->name] = $item;
} else {
if ( $item instanceof stdClass ) {
// Default to the schema of the parent table.
if ( ! isset($item->schema) ) {
$item->schema = $this->schema;
}
}
$constraint = new ForeignKeyConstraint($item, $this->systemQuoteChar, $this->logger);
$this->properties[$property][$constraint->name] = $constraint;
}
}
}
break;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
{
"Alter ON DELETE from NO ACTION to CASCADE": [
{
"table_definition": {
"name": "test_db_model",
"engine": "InnoDB",
"charset": "latin1",
"collation": "latin1_swedish_ci",
"comment": "Events on an instance",
"columns": [
{
"name": "event_id",
"type": "bigint(20) unsigned",
"nullable": false,
"extra": "auto_increment",
"comment": "Generated during intermediate ingest"
},
{
"name": "col1",
"type": "varchar(64)",
"charset": "latin1",
"collation": "latin1_swedish_ci",
"nullable": false,
"default": "mydefault"
},
{
"name": "col2",
"type": "varchar(64)",
"charset": "utf8",
"collation": "utf8_unicode_ci",
"nullable": false,
"default": "mydefault"
},
{
"name": "event_time",
"type": "timestamp",
"nullable": false,
"default": "CURRENT_TIMESTAMP"
},
{
"name": "instance_id",
"type": "int(11) unsigned",
"nullable": true,
"default": 10
},
{
"name": "inferred",
"type": "int(1) unsigned",
"nullable": true,
"default": 0,
"comment": "Not explicitly provided by source but inferred from other data"
}
],
"indexes": [
{
"name": "fk_col",
"columns": [
"col1"
],
"type": "BTREE",
"is_unique": false
},
{
"name": "fk_instance",
"columns": [
"instance_id",
"inferred"
],
"type": "BTREE",
"is_unique": true
}
],
"foreign_key_constraints": [
{
"name": "con_col1",
"columns": [
"col1"
],
"referenced_table": "db_test_model2",
"referenced_columns": [
"col3"
],
"on_delete": "CASCADE",
"on_update": "NO ACTION"
}
]
}
},
{
"columns": [
"instance_id"
],
"referenced_table": "other_table",
"referenced_columns": [
"other_column"
],
"on_delete": "NO ACTION"
},
{
"columns": [
"instance_id"
],
"referenced_table": "other_table",
"referenced_columns": [
"other_column"
],
"on_delete": "CASCADE"
},
[
"ALTER TABLE `test_db_model`\nDROP FOREIGN KEY `fk_instance_id`;",
"ALTER TABLE `test_db_model`\nADD CONSTRAINT `fk_instance_id` FOREIGN KEY (`instance_id`) REFERENCES `other_table` (`other_column`) ON DELETE CASCADE;"
]
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
{
"Table with simplest foreign key constraint": [
{
"table_definition": {
"name": "test",
"engine": "InnoDB",
"columns": [
{
"name": "id",
"type": "int(11)",
"nullable": false,
"extra": "auto_increment"
},
{
"name": "other_id",
"type": "int(11)",
"nullable": false
}
],
"indexes": [
{
"name": "PRIMARY",
"columns": [
"id"
]
},
{
"name": "idx_other_id",
"columns": [
"other_id"
]
}
],
"foreign_key_constraints": [
{
"name": "fk_other",
"columns": [
"other_id"
],
"referenced_table": "other",
"referenced_columns": [
"id"
]
}
]
}
},
[
"CREATE TABLE IF NOT EXISTS `test` (\n `id` int(11) NOT NULL auto_increment,\n `other_id` int(11) NOT NULL,\n PRIMARY KEY (`id`),\n INDEX `idx_other_id` (`other_id`),\n CONSTRAINT `fk_other` FOREIGN KEY (`other_id`) REFERENCES `other` (`id`)\n) ENGINE = innodb;"
]
],
"Table with complex foreign key constraint": [
{
"table_definition": {
"name": "test",
"engine": "InnoDB",
"columns": [
{
"name": "id",
"type": "int(11)",
"nullable": false,
"extra": "auto_increment"
},
{
"name": "other_id",
"type": "int(11)",
"nullable": false
}
],
"indexes": [
{
"name": "PRIMARY",
"columns": [
"id"
]
},
{
"name": "idx_other_id",
"columns": [
"other_id"
]
}
],
"foreign_key_constraints": [
{
"name": "fk_other",
"columns": [
"other_id"
],
"referenced_schema": "mod_other",
"referenced_table": "other",
"referenced_columns": [
"id"
],
"on_delete": "SET NULL",
"on_update": "CASCADE"
}
]
}
},
[
"CREATE TABLE IF NOT EXISTS `test` (\n `id` int(11) NOT NULL auto_increment,\n `other_id` int(11) NOT NULL,\n PRIMARY KEY (`id`),\n INDEX `idx_other_id` (`other_id`),\n CONSTRAINT `fk_other` FOREIGN KEY (`other_id`) REFERENCES `mod_other`.`other` (`id`) ON DELETE SET NULL ON UPDATE CASCADE\n) ENGINE = innodb;"
]
]
}
Loading

0 comments on commit 35c22c2

Please sign in to comment.