cockroachdb-laravel
cockroachdb-laravel copied to clipboard
[Bug]: Unknown database type anyelement requested exception caused by a renaming migration
What happened?
We have a Laravel project that has the following migration steps:
- Create column
Y_temp - Migrate data from column
XtoY_temp - Drop column
X - Rename column
Y_temptoX
These migrations worked without any problem on CockroachDB version 22.2.8. After updating to version 23.1.16 the renaming step from a new column to one that has been deleted previously throws the following exception:
In AbstractPlatform.php line 452:
Unknown database type anyelement requested, Doctrine\DBAL\Platforms\PostgreSQL100Platform may not support it.
How to reproduce the bug
Set up a new Laravel project with CockroachDB version 23.X.X. Create migrations with the following steps:
- Create a new table
- Create column
X - Create column
Y - Drop column
X - Rename column
Yto columnX
The last step should fail with the exception from above.
Package Version
1.3.0
PHP Version
8.2.0
Laravel Version
10.47.0
CockroachDB Version
23.1.6
Which operating systems does with happen with?
No response
Notes
I have already done some digging around. At first, I stumbled across this issue: https://github.com/doctrine/dbal/issues/6248 with an associated PR that never got merged. The changes in the PR worked on my machine, so it seemed to be a general bug in the Postgres implementation of doctrine. But after it tried to run the tests in the PR / write my own tests for it, I found out that it is actually not a problem with a plain PostgreSQL Database. It throws this exception only on a CRDB version above 23 and not on a plain PostgreSQL.
The SQL-Statement in the PostgreSQLSchemaManager from the selecteTableColumns-Function actually returns a different Result between CRDB Version 22, 23 and PostgreSQL 16.
In CRDB v23, it also returns the dropped fields named like this: ........pg.dropped.15...... with the type anyelement.
Doctrine now tries to cast the type anyelement which does not exist in the PostgreSQL implementation, causing the exception.
The changes in the PostgreSQLSchemaManager from the PR above does fix this issue. But i think it should not be implemented in the Doctrine DBAL 'cause it is not a PostgreSQL bug. I think we should find a way to somehow implement the fix in this package.
@Vision42 thanks for the bug report. This might be a difficult one as DBAL is it's own thing to begin with. I don't know if there is any project covering Crdb.
Also, Laravel 11 is moving away from using DBAL but I realistically haven't had time to make the upgrades to see how much time would be involved.
Is there any chance you can make a start on a test showing the problem at all?
Thanks for your reply :) I've now created additional tests that show the basic problem and created a PR for it.
@peterfox I think I've found a solution for this issue. I've implemented a fix and updated the PR. The tests I built in the morning are now green with the new changes.
@Vision42 it looks good for the latest versions of Laravel 10 but using the lowest compatible packages and older versions of Laravel the tests fail. Hard to get a good idea of what is the problem.
I wouldn't mind if the code fails for Laravel 9/8. 8 will be dropped soon as you mentioned this seems to only be a problem with Crdb v23. It would be good to investigate how all versions of Laravel 10 can work with your changes.
@peterfox Laravel 10 seems to require doctrine/dbal ^3.5.1 which implements the introspectTable function. With the lowest compatible packages, doctrien/dbal ^3.2.0 gets installed, which does not implement the introspectTable function called by the renameColumn function. As far as I can see, the renameColumn function does currently not work with the lowest compatible packages for Laravel 10.
Can we just bump up the composer dependency to doctrine/dbal ^3.5.1 or will that break compatibility with older versions of Laravel?
@Vision42 It would likely break older versions although it's always hard to tell. It might be a case that your fix lives in the package but has to be switched on.
e.g.
protected function getDoctrineDriver()
{
if (\Composer\InstalledVersions::satisfies(new VersionParser, 'doctrine/dbal', '^3.5.1')) {
return new CockroachDBDriver();
} else {
return new PostgresDriver();
}
}
That might save all but the Laravel 10 Lowest where Crdb is v23
@peterfox I have just tried this. But my renaming tests fail even when I use the default PostgresDriver. It's still the introspectTable exception. It actually seems to be a problem with the rename function and the lowest stable dependency definition. It is independent of my changes as far as I can see.
To get the rename function to work in Laravel 10 with the lowest stable dependencies, we need to bump up the doctrine/dbal version.
See their comment in Laravel's composer.json (I've just found it now): https://github.com/laravel/framework/blob/ce7f5b15305734ddda21b1b2be5b89204d3827e1/composer.json#L171
The other option is to skip the renaming tests altogether if the doctrine/dbal version is below 3.5.1. I think this would be the most appropriate solution, as we are basically following Laravel's approach here.
I would update the test cases if you are fine with that approach.