phalcon / migrations

Generate or migrate database changes via migrations.
https://docs.phalcon.io/latest/en/db-migrations
BSD 3-Clause "New" or "Revised" License
27 stars 23 forks source link

Set PRIMARY KEY in table "phalcon_migrations" on column "version" #58

Closed jacentemu closed 4 years ago

jacentemu commented 4 years ago

Introduction I'm running migrations with --log-in-db option (storing list of done migrations in database, not in local files). This option use phalcon_migrations table for storing this information about migration and date of executing.

Issue description I think, that in table phalcon_migrations, the column version should become PRIMARY KEY (or UNIQUE KEY). Two reasons about that:

Current status

At this moment, column version in table phalcon_migrations is normal, non unique key, so multiple entries can have the same value. Table with migrations is defined in https://github.com/phalcon/migrations/blob/master/src/Migrations.php#L511

new Column( 'version', [ 'type' => Column::TYPE_VARCHAR, 'size' => 255, 'notNull' => true, ]

And it looks:

mysql> describe phalcon_migrations; +------------+--------------+------+-----+-------------------+-------+ | Field | Type | Null | Key | Default | Extra | +------------+--------------+------+-----+-------------------+-------+ | version | varchar(255) | NO | MUL | NULL | | | start_time | timestamp | NO | | CURRENT_TIMESTAMP | | | end_time | timestamp | NO | | CURRENT_TIMESTAMP | | +------------+--------------+------+-----+-------------------+-------+

I tested both: setting version as PRIMARY KEY and UNIQUE KEY, and migrations worked fine. Generally, both options (primary/unique) should give the same result, because if in MySQL table there is no PRIMARY KEY, then value with UNIQUE KEY is "promoted" to be PRIMARY KEY. So perhaps PRIMARY KEY is better option to make table structure more transparent.

Expected result:

Column version on table phalcon_migrations should be PRIMARY KEY (or UNIQUE KEY).

Unnecessary additional opinion BTW, in my opinion, in future, the option --log-in-db should be default option. It is necessary when using phalcon/migrations with multiple instances of application working with single database, useful in testing and CI.. I don't see benefits of storing this information outside the DB.

Jeckerson commented 4 years ago

BTW, in my opinion, in future, the option --log-in-db should be default option.

Yes, there are plans to do that in v2 or little bit later, as many things to refactor without braking old.

Jeckerson commented 4 years ago

Implemented in #62