-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
[Bug]: Unknown database type anyelement requested exception caused by a renaming migration #35
Comments
@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 :) |
…ach schema manager to fix renaming issue ylsideas#35
Add a.attisdropped = false flag to compile columns sql to fix the output of Schema::getColumns(). Add custom cockroach db driver for loading the custom cockroach schema manager to fix renaming issue ylsideas#35
@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 Can we just bump up the composer dependency to |
@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 To get the rename function to work in Laravel 10 with the lowest stable dependencies, we need to bump up the The other option is to skip the renaming tests altogether if the |
What happened?
We have a Laravel project that has the following migration steps:
Y_temp
X
toY_temp
X
Y_temp
toX
These migrations worked without any problem on CockroachDB version
22.2.8
. After updating to version23.1.16
the renaming step from a new column to one that has been deleted previously throws the following exception:How to reproduce the bug
Set up a new Laravel project with CockroachDB version
23.X.X
. Create migrations with the following steps:X
Y
X
Y
to 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: doctrine/dbal#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 typeanyelement
.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.The text was updated successfully, but these errors were encountered: