getAlterTableName doesn't correctly quote the column name on MySQL

[FSMaxB]

trafficstars

I'm not sure how to reproduce this with DBAL directly, but I stumbled upon this problem via the Laravel framework.

Original issue reported with Laravel: https://github.com/laravel/framework/issues/22402

I didn't quite find the code in Laravel that causes this, but I am quite confident that it is caused by a bug in getAlterTableSQL.

DBAL seems to create the following queries:

The query is:

ALTER TABLE bug CHANGE weird-name good_name VARCHAR(255) NOT NULL;

But it should be:

ALTER TABLE bug CHANGE `weird-name` good_name VARCHAR(255) NOT NULL;

I guess that the following line needs backticks ``` around the quoted column name: https://github.com/doctrine/dbal/blob/9f843bca49a223195921e589173ebaad8980bf0f/lib/Doctrine/DBAL/Platforms/MySqlPlatform.php#L619

[Ocramius]

Is this reproducible with DBAL 2.6?

[FSMaxB]

Yes. DBAL 2.6.3, I just checked.

[FSMaxB]

Ping @Ocramius

[Ocramius]

@FSMaxB what about this?

[FSMaxB]

Has anyone looked into this yet?

[Ocramius]

@FSMaxB nope - don't think anybody will actively get to it for now.

[derekrprice]

Still happening in DBAL v2.10.0. The solution that @FSMaxB suggested is close but might be slightly naive. There is already a _quoted attribute on the $column objects being passed in, which were generated by DBAL's column diff. When set, that attribute causes $diff->getName($this)->getQuotedName($this) & $column->getQuotedName($this) to return a correctly quoted identifier. So, the trick might be getting diff to set those attributes correctly. Alternately, getQuotedName() appears to automatically quote when it spots an identifier, regardless of the value of _quoted. If it was updated to also quote column names with spaces in them, that would work for me.

Ran into this with Laravel 6.9.0, attempting to alter columns with spaces in the names. Strangely, the same code works just fine to create the columns. It's just altering them that is an issue.

[derekrprice]

Interesting. It looks like _quoted is automatically set when the input identifier is quoted, but I already tried quoting the column name when calling Eloquent's $table->$type(). When I do that, it tries to create the column (with quotes actually as part of the name, so too many quotes) instead of altering it, so maybe the problem is in the diff algorithm not detecting that quoted column names match the originals.

[derekrprice]

I know that automatically quoting identifiers is frowned upon, for the reasons described in #146, but AbstractAsset::getquotedName already makes an exceptions for keywords, since not quoting those would break the generated SQL anyhow. In the same way, spaces in column names and other identifiers break the generated SQL, so I think that the pull request that I just attached is justified.