db-migration icon indicating copy to clipboard operation
db-migration copied to clipboard

Not a bug, but a proposal for an upgrade/feature on migrations

Open awesomeguy3 opened this issue 8 years ago • 17 comments

As I said in the title, this is not a bug, but a proposal (didn't know where to write this that would be seen.

Now, on my project I have a big database (not in terms of data but in terms of tables and columns in those tables), and I also have a lot of indexes. I noticed today that I need to write in a migration class $this->createIndex('name', 'table', ['columns']) after creating a new table, what I'm thinking should be a good idea is to add a possibility for method chaining on "createTable" function where I wouldn't need to enter table name for every index. Or perhaps another parameter in createTable function that would easily let me add indexes in one place.

awesomeguy3 avatar Aug 09 '17 14:08 awesomeguy3

How would the syntax look like?

samdark avatar Aug 09 '17 15:08 samdark

Maybe something like

$this->createTable('tableName', [
    'id' => $this->primaryKey(),
    'column' => $this->string()
], 'some options', [
    'index1' => ['column1', 'column2'],
    'index2' => ['column3']
]);

OR

$this->createTable('tableName', [
    'id' => $this->primaryKey(),
    'column' => $this->string()
], 'some options')->index('index1', ['column1', 'column2'])->index('index2', 'column3');

And forgive me for not knowing how to format code better in Markdown.

awesomeguy3 avatar Aug 09 '17 15:08 awesomeguy3

Formatting is easy: https://guides.github.com/features/mastering-markdown/

samdark avatar Aug 09 '17 15:08 samdark

Second options looks interesting...

samdark avatar Aug 09 '17 15:08 samdark

I personally like first option better, but whatever you make is okay with me.

awesomeguy3 avatar Aug 09 '17 15:08 awesomeguy3

$this->createTable('tableName', 
        [
            'id' => $this->primaryKey(),
            'column' => $this->string()
        ]
)->createIndex(
        ['index_one', 'column1'],
        ['index_two', ['column1', 'column2']],
        ['index_three', ['column1', 'column2']],
)->createForeignKey(
        ['FK_name_one', 'column1', 'table1', 'column'],
        ['FK_name_two', 'column2', 'table2', 'column']
);

kjusupov avatar Aug 11 '17 08:08 kjusupov

How about something like this?

$this->createTable('tableName', [
    'id' => $this->primaryKey(),
    'column' => $this->string()->index(),
]);

Not sure if that would work with ColumnSchemaBuilder, and if not maybe something like this?

$this->createTable('tableName', [
    'id' => $this->primaryKey(),
    'column' => [$this->string(), 'index' => true],
]);

If it's just true then create an index for that one column with that name (I think this is the most common). otherwise a string with the name of the index.

A solution like @kjusupov would be required for more complex indices, but sounds like it would require something like a TableSchemaBuilder perhaps?

Sarke avatar Aug 13 '17 01:08 Sarke

I'm for

$this->createTable('tableName', [
    'id' => $this->primaryKey(),
    'name' => $this->string()->index('indexName'),
    'topic_id' => $this->integer()->foreignKey('indexName', 'referencedTable', 'referencedColumn'),
]);

rob006 avatar Aug 13 '17 10:08 rob006

Great ideas, but what about complex indexes?

awesomeguy3 avatar Aug 13 '17 10:08 awesomeguy3

We already have createIndex() for complex indexes.

rob006 avatar Aug 13 '17 10:08 rob006

I'm in favor for chaining like @kjusupov exampled. This fits with the current way of constructing the migrations.

michaelarnauts avatar Aug 18 '17 11:08 michaelarnauts

I'm in favor for chaining like @kjusupov exampled. This fits with the current way of constructing the migrations.

This syntax does not give you any real benefits, it creates only another way to do the same thig (which usually decrease readability).

rob006 avatar Aug 18 '17 11:08 rob006

I'm with @rob006 on this one, his example is quite good.

awesomeguy3 avatar Aug 18 '17 14:08 awesomeguy3

The syntax proposed by @rob006 allows sqlite3 to create foreign keys at the same time that the table, which is the only way to add a foreign key in sqlite3 so far, although I am working on adding that feature to yii2/sqlite3: #15007

santilin avatar Oct 24 '17 02:10 santilin

@kjusupov

$this->createTable('tableName', 
        [
            'id' => $this->primaryKey(),
            'column' => $this->string()
        ]
)->createIndex(
        ['index_one', 'column1'],
        ['index_two', ['column1', 'column2']],
        ['index_three', ['column1', 'column2']],
)->createForeignKey(
        ['FK_name_one', 'column1', 'table1', 'column'],
        ['FK_name_two', 'column2', 'table2', 'column']
);

The only issue I have with this approach is that you'd need to remember the order of parameters for createForeignKey() etc (unless you use an IDE that can tell you the parameters).

A better approach I think would be to make it mandatory to include the keys in the array, e.g:

$this->createForeignKey([
    'column' => 'user_id',
    'references' => 'id',
    'on' => 'user',
    'onDelete' => 'cascade',
]);

Note in this example, I didn't include the FK name - this should be optional and auto-generated (based on column and table name) if it isn't included.

omzy avatar Oct 19 '18 10:10 omzy

A better approach I think would be to make it mandatory to include the keys in the array, e.g:

Now you need to remember keys names, and IDE will not help you here...

rob006 avatar Oct 19 '18 10:10 rob006

You can already add plain string lines like this to add an index:

$this->createTable('tableName', [
    'id' => $this->primaryKey(),
    'name' => $this->string(),
    'topic_id' => $this->integer(),
    'KEY topic_id(topic_id)',
    'FOREIGN KEY ... REFERENCES ...',
]);

So we can have a schemabuilder class that would build those strings in DBMS specific syntax.

cebe avatar Oct 25 '18 23:10 cebe