derrabus
commented
3 years ago Did you only upgrade ORM from 2.9 to 2.10 or did you also upgrade DBAL from 2.13 to 3.1?
Open alexander-schranz opened 3 years ago
derrabus
commented
3 years ago Did you only upgrade ORM from 2.9 to 2.10 or did you also upgrade DBAL from 2.13 to 3.1?
alexander-schranz
commented
3 years ago @derrabus with dbal 3.1.
derrabus
commented
3 years ago This is actually a DBAL issue, I'm afraid: DBAL 3 does not ship with the json_array type anymore and the ORM can only register types that DBAL actually provides.
You need to upgrade your schema before upgrading to DBAL 3.
alexander-schranz
commented
3 years ago @derrabus but should a not longer exist type just be ignored and the schema still be updated to the new schema?
derrabus
commented
3 years ago Yes maybe. But at the moment, the DBAL can only create schema diffs if it knows all types that are used in the current database.
beberlei
commented
2 years ago Can you register the json_array type manually to use the JsonType class for it? Since you only need it for the migration, registering the type manually could do the trick.
alexander-schranz
commented
2 years ago Sure there are workaround like just remove manual the json_array from the table, or first downgrade to doctrine/dbal 2 before going to dbal 3. But all things require here manual updates of a project and in case of a library which migrated from json_array to json for its entities semi nice as you don't want in that case register a type which is already not longer supported. Sadly I'm not into it how the diff works but from my point of view it would be a great feature if the diff don't care about the doctrine type just about what the diff in the schema is and what it need to be changed.
chasen
commented
2 years ago I am having the same issue. This is a breaking change introduced in doctrine/orm 2.10. When its dependencies were updated to doctrine/dbal: ^2.13.1 || ^3.1.1. Previously this packaged installed the dbal 2 version. But after upgrading it now installs the dbal 3 version by default.
Sure this can be prevented by also adding in a top level requirement to our project for "doctrine/dbal": "^2.13.1" but that shouldn't be needed with a minor version change.
YetiCGN
commented
2 years ago Expected behavior An upgrade from json_array to json should work and the doctype comment be removed by using:
ALTER TABLE se_role_settings CHANGE value value JSON NOT NULL; -- no comment
we_analytics.content from the WebsiteBundle is also affected as well as ac_activities (two JSON columns) from the ActivityBundle.
derrabus
commented
2 years ago Let's not collect more me-toos in here. We know about it, we know the cause, we know the workarounds. If you want to contribute to the solution, please feel free comment or open a PR.
To recap:
tl;dr: If you use deprecated DBAL types, don't upgrade DBAL before you have migrated away from them.
This is a community driven library. If you have an idea how to create a smoother upgrade path, please open a PR to either DBAL or ORM and we discuss it. If you want to to express that this issue affects you, feel free to 👍🏻 the issue description.
chasen
commented
2 years ago Would it make sense to release these changes as a new major version as well? It would seem that when dependent packages are upgraded to next major versions and when those versions are known to have breaking changes a new major version of the parent package would be warranted.
When introducing dbal v3 why not also release that as orm 3? This would provide the expected manual upgrade path and continue to follow the values of semver.
greg0ire
commented
2 years ago @chasen nice question, let me answer it :slightly_smiling_face:
No, it would not make sense. When using json_array in an ORM mapping, you are referring to a DBAL type, meaning the link this points to shows a class defined in the doctrine/dbal package, not in the ORM. If you do that, then you should require doctrine/dbal in your composer.json, thus making the dependency explicit, and the upgrade would then become something conscious on your end.
I think an improvement can be made by mentioning this in the docs: https://www.doctrine-project.org/projects/doctrine-orm/en/2.10/tutorials/getting-started.html#getting-started-with-doctrine , in case some of you would be willing to improve the developer experience.
chasen
commented
2 years ago Thanks @greg0ire this is what we ended up doing but it still leaves a bad taste. This feels like a poor developer experience where you had been relying on a single package previously and then a breaking change was introduced in that package without a major version change.
If the issue is indeed only with DBAL and not with ORM why not remove it as a dependency then and have developers do as you suggest and explicitly add the requirement on the dbal version of their choice?
Or Is that what you are recommending get updated in the getting started docs? That instead of only relying on doctrine/orm that developers also specifically define the doctrine/dbal package too?
greg0ire
commented
2 years ago If the issue is indeed only with DBAL and not with ORM why not remove it as a dependency then and have developers do as you suggest and explicitly add the requirement on the dbal version of their choice?
The ORM requires the DBAL (cannot work without it). What I'm saying is that the user should also require it if they mention it in the code they write.
Or Is that what you are recommending get updated in the getting started docs? That instead of only relying on doctrine/orm that developers also specifically define the doctrine/dbal package too?
Yes, as soon as you write mapping files, you depend on the DBAL, and if you use the ORM, I think you will be using mapping files. So I would expect doctrine/dbal to appear in the composer.json mentioned in "getting started".
chasen
commented
2 years ago That makes sense. Thanks for all of the explanation. If I get a few minutes later today, ill see if I can take a stab at updating the getting started docs to better call this out to help others avoid similar issues.
bordeux
commented
2 years ago I also today reach this issue, but i fixed it using this migration:
<?php declare(strict_types=1);
namespace DoctrineMigrations;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
final class Version20220529152449 extends AbstractMigration
{
public function up(Schema $schema): void
{
$query = "
SELECT c.column_name,
pgd.description,
c.table_schema,
c.table_name
FROM pg_catalog.pg_statio_all_tables as st
inner join pg_catalog.pg_description pgd on (pgd.objoid=st.relid)
inner join information_schema.columns c on (pgd.objsubid=c.ordinal_position and c.table_schema=st.schemaname and c.table_name=st.relname)
WHERE pgd.description = :desc
";
$data = $this->connection
->executeQuery($query, ["desc" => '(DC2Type:json_array)'])
->fetchAllAssociative();
foreach ($data as $column) {
$this->addSql("
COMMENT ON COLUMN {$column['table_schema']}.{$column['table_name']}.{$column['column_name']} IS NULL;
");
}
}
public function down(Schema $schema): void
{
// no way back!
}
}
nicolassing
commented
1 year ago I don't know if anyone still has this problem, but this solution seems to work perfectly : https://dunglas.dev/2022/01/json-columns-and-doctrine-dbal-3-upgrade/
Bug Report
Summary
The deprecated json_array type is not longer supported but its actually not possible to upgrade in 2.10.0 the database from json_array to json.
Current behavior
After upgrade to doctrine/orm I did replace all
json_arraywithjsonbut when now running:The the FieldRegistry fails as the database still has the comment
(DC2Type:json_array)in it and is so mapped to the json_array type.How to reproduce
Create a Entity with
json_arrayupdate doctrine/orm to2.10.0change type fromjson_arraytojsonand run:Expected behavior
An upgrade form json_array to json should work and the doctype comment be removed by using: