Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Allowed array as schema property for indexes, foreign keys and unique constraints #17

Open
wants to merge 1 commit into
base: 1.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/lib/Importer/SchemaImporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ private function importSchemaTable(
}

if (isset($tableConfiguration['foreignKeys'])) {
foreach ($tableConfiguration['foreignKeys'] as $foreignKeyName => $foreignKey) {
foreach ($tableConfiguration['foreignKeys'] as $key => $foreignKey) {
$foreignKeyName = is_string($key) ? $key : null;
$table->addForeignKeyConstraint(
$foreignKey['foreignTable'],
$foreignKey['fields'],
Expand All @@ -92,15 +93,17 @@ private function importSchemaTable(
}

if (isset($tableConfiguration['indexes'])) {
foreach ($tableConfiguration['indexes'] as $indexName => $index) {
foreach ($tableConfiguration['indexes'] as $key => $index) {
$indexName = is_string($key) ? $key : null;
$table->addIndex(
$index['fields'], $indexName, [], $index['options'] ?? []
);
}
}

if (isset($tableConfiguration['uniqueConstraints'])) {
foreach ($tableConfiguration['uniqueConstraints'] as $indexName => $index) {
foreach ($tableConfiguration['uniqueConstraints'] as $key => $index) {
$indexName = is_string($key) ? $key : null;
$table->addUniqueIndex(
$index['fields'], $indexName, $index['options'] ?? []
);
Expand Down
45 changes: 44 additions & 1 deletion tests/lib/Importer/SchemaImporterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,49 @@ public function providerForTestImportFromFile(): array
]
),
],
8 => [
'08-autogenerated_index_names.yaml',
new Schema(
[
new Table(
'my_table',
[
(new Column('id', Type::getType('integer')))
->setAutoincrement(true),
new Column('data1', Type::getType('integer')),
new Column('data2', Type::getType('integer')),
new Column('name', Type::getType('string')),
],
[
new Index('IDX_9AEF3D8257CA2CA6', ['data1'], false, false),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this really deterministic, not some variation of UUID?

Copy link
Contributor Author

@Steveb-p Steveb-p Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alongosz Doctrine will always generate the same name, as long as columns included are not changed.

See the function that generates this in Doctrine:

/**
 * Generates an identifier from a list of column names obeying a certain string length.
 *
 * This is especially important for Oracle, since it does not allow identifiers larger than 30 chars,
 * however building idents automatically for foreign keys, composite keys or such can easily create
 * very long names.
 *
 * @param string[] $columnNames
 * @param string   $prefix
 * @param int      $maxSize
 *
 * @return string
 */
protected function _generateIdentifierName($columnNames, $prefix = '', $maxSize = 30)
{
    $hash = implode('', array_map(static function ($column) {
        return dechex(crc32($column));
    }, $columnNames));

    return strtoupper(substr($prefix . '_' . $hash, 0, $maxSize));
}

And the calling function:

public function addIndex(array $columnNames, $indexName = null, array $flags = [], array $options = [])
{
    if ($indexName === null) {
        $indexName = $this->_generateIdentifierName(
            array_merge([$this->getName()], $columnNames),
            'idx',
            $this->_getMaxIdentifierLength()
        );
    }

    return $this->_addIndex($this->_createIndex($columnNames, $indexName, false, false, $flags, $options));
}

new Index('IDX_9AEF3D8257CA2CA6CEC37D1C', ['data1', 'data2'], false, false),
new Index('primary', ['id'], false, true),
new Index('UNIQ_9AEF3D825E237E06', ['name'], true, false),
]
),
new Table(
'my_secondary_table',
[
(new Column('id', Type::getType('integer')))
->setAutoincrement(true),
new Column('main_id', Type::getType('integer')),
],
[
new Index('primary', ['id'], false, true),
],
[
new ForeignKeyConstraint(
['main_id'],
'my_main_table',
['id'],
'FK_D8A74C1627EA78A',
['onDelete' => 'CASCADE', 'onUpdate' => 'CASCADE']
),
]
),
]
),
],
];

return $data;
Expand All @@ -205,7 +248,7 @@ public function testImportFromFile(
) {
$yamlSchemaDefinitionFilePath = realpath(__DIR__ . "/_fixtures/{$yamlSchemaDefinitionFile}");
if (false === $yamlSchemaDefinitionFilePath) {
self::markTestIncomplete("Missing output fixture {$yamlSchemaDefinitionFilePath}");
self::fail("Missing fixture $yamlSchemaDefinitionFilePath");
}

$importer = new SchemaImporter();
Expand Down
23 changes: 23 additions & 0 deletions tests/lib/Importer/_fixtures/08-autogenerated_index_names.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
tables:
my_table:
id:
id: { type: integer, nullable: false, options: { autoincrement: true } }
uniqueConstraints:
- { fields: [name] }
indexes:
- { fields: [data1] }
- { fields: [data1, data2] }
fields:
data1: { type: integer, nullable: false }
data2: { type: integer, nullable: false }
name: { type: string, nullable: false }
my_secondary_table:
id:
id: { type: integer, nullable: false, options: { autoincrement: true } }
fields:
main_id: { type: integer, nullable: false }
foreignKeys:
- fields: [ main_id ]
foreignTable: my_main_table
foreignFields: [ id ]
options: { onDelete: CASCADE, onUpdate: CASCADE }