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

db/Schema::getTableMetadata() broken #15117

Closed
etienneq opened this issue Nov 7, 2017 · 7 comments
Closed

db/Schema::getTableMetadata() broken #15117

etienneq opened this issue Nov 7, 2017 · 7 comments
Assignees
Milestone

Comments

@etienneq
Copy link

etienneq commented Nov 7, 2017

What steps will reproduce the problem?

Run migrate/fresh more than once.

What is the expected result?

Command should run on each call.

What do you get instead?

On second call an exception is thrown because the migration table gets not created after truncate.

Additional info

Q A
Yii version 2.0.13
PHP version 7.0.18
Operating system Debian

The error was introduced in Yii 2.0.13.

The reason is the implementation of db/Schema::getTableMetadata(). The migration table does not exist but it's schema information are still present in file cache.

This is the current implementation:

if ($refresh || !isset($this->_tableMetadata[$name])) {
	...
}

if (!array_key_exists($type, $this->_tableMetadata[$name])) {
	...
}

And it should look like this:

if (!isset($this->_tableMetadata[$name])) {
	...
}

if ($refresh || !array_key_exists($type, $this->_tableMetadata[$name])) {
	...
}
@sergeymakinen
Copy link
Member

Em. yii\db\Schema::refreshTableSchema will be used in yii\console\controllers\MigrateController::truncateDatabase() and should take care of it.

The current implementation should work fine because it discards a complete table metadata if $refresh is true.

@samdark samdark added feature:db status:to be verified Needs to be reproduced and validated. labels Nov 7, 2017
@samdark samdark added this to the 2.0.13.1 milestone Nov 7, 2017
@samdark
Copy link
Member

samdark commented Nov 7, 2017

What's the database used? MySQL?

@etienneq
Copy link
Author

etienneq commented Nov 8, 2017

Yes, MySQL.

@samdark samdark added the type:bug Bug label Nov 8, 2017
@antichris
Copy link

The current implementation should work fine because it discards a complete table metadata if $refresh is true.
@sergeymakinen

No, it does not work "fine".

use yii\console\controllers\MigrateController,
    yii\db\Connection,
    yii\db\Schema,
    yii\db\TableSchema;

MigrateController::truncateDatabase() gets table names from Schema::getTableSchemas(), where the TableSchema::$name for the migrations table is plain migration (or {prefix}migration if current Connection::$tablePrefix is {prefix}). The metadata for this table name is discarded.

But MigrateController::getMigrationHistory(), which is called when migrating up, gets the metadata for MigrateController::$migrationTable, defined as {{%migration}} by default. If the metadata for this table name is cached, Schema::getTableMetadata() will return that regardless of the value of $refresh (and the actual schema state). That's what this issue is all about.

I suppose a solution would involve tuning table prefix resolution for schema caching and/or migrations.

@sergeymakinen
Copy link
Member

@U-D13 thanks for details. It's definitely not okay.
But caching/purging logic is still the same from the old Schema, so it this issue probably existed for some time before.
Anyway I will dig into this.

@sergeymakinen
Copy link
Member

I confirm this.
An issue resides in yii\db\Schema::getCacheKey it was there for a long time:
a cache key doesn't resolve table names to raw names, so {{%table}} and table entries are stored/deleted as is.
I will add tests and fix it tonight.

@boboldehampsink
Copy link
Contributor

This error is still present in the latest version. Running migrate/up once, then dropping/recreating db and running migrate/up again results in error because its still cached. The solution proposed in the original post fixes it and seems logical to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants