2.9 BC Break on Doctrine\DBAL\Schema\Table method signatures

[ricardofiorani]

Q	A
BC Break	yes
Version	2.9.0

Summary

Sorry if this is intended, but I couldn't find anywhere mentioning these changes on these Doctrine\DBAL\Schema\Table method signatures :

• setPrimaryKey(int, string) -> setPrimaryKey(array)
• addUniqueIndex(int, string) -> addUniqueIndex(array)
• addIndex(int, string) -> addIndex(array)

I can drop a PR either reverting the method signature's back or update the docs if needed.

[Ocramius]

@ricardofiorani what version are you comparing this to? I don't see changes in those methods within the last year.

[ricardofiorani]

@Ocramius from v2.8.0 to v2.9.0

[ricardofiorani]

On my composer.json I was requiring it as "doctrine/dbal": "^2.8",

[Ocramius]

Can you check more precisely? I'm seeing:

https://github.com/doctrine/dbal/blob/v2.8.0/lib/Doctrine/DBAL/Schema/Table.php#L147

and

https://github.com/doctrine/dbal/blob/v2.9.0/lib/Doctrine/DBAL/Schema/Table.php#L108

[ricardofiorani]

@Ocramius

On the docblock:

2.9 * @param mixed[][] $columns
2.8 * @param array $columns

PHPStan don't like it :(

[Ocramius]

Yes, but this is correct: we just documented what the expected types are.

If those are not the parameters you were passing to it, then your application had a pre-existing bug in it.

[ricardofiorani]

@Ocramius but by mixed[][] you are suggesting that it is a two level array, which is not the case in any of those methods... it should be mixed[] then.

[ricardofiorani]

Anyway, I'll stick my application with 2.8 until I have some more time to dig into this.

Thank you for your responses.

[Ocramius]

@ricardofiorani send a patch to fix the types there then - running phpstan on that file (specifically) will likely reveal more issues that we haven't inspected yet 👍

[morozov]

We probably need to fix parameter names as well. In most cases, $columns is mixed[][] which is an array of column definitions, which are arrays themselves. In the case of setPrimaryKey(), it should have been $columnNames, same as in addIndex().

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.