Use Native JSON type instead of LONGTEXT for MariaDB

[alexander-schranz]

Q	A
Type	bug/feature/improvement
BC Break	yes/no
Fixed issues	fixes #3202

Summary

Use JSON type instead of LONGTEXT for MariaDB. See #3202

[greg0ire]

This is a BC-break, but maybe you just wanted to validate that tests stay green with this change? If yes, please set the PR status to draft until it is finished.

As advised, I think a platform option would allow people to switch to JSON while retaining backwards-compatibility for others. What do you think?

[alexander-schranz]

platform option

Do you have an example for a platform option and how they could be used here? How are platform options configured? Are the queries params in the dsn?

[morozov]

This is a BC-break […]

In this regard, the mappings are internal implementation details of the DBAL, so whether it's a BC break, depends on whether the DBAL can migrate the schema from the old mapping to the new one seamlessly.

[alexander-schranz]

The change seems to have no effect for doctrine:schema:update or bin/console doctrine:migrations:diff. As for the diff:

`data` longtext COLLATE utf8mb4_unicode_ci NOT NULL COMMENT '(DC2Type:json)',

is the same as the new schema with:

`data` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL CHECK (json_valid(`new_data`)),

So this would be no BC Break. If somebody want to upgrade from (DC2Type:json) to native mariadb JSON with json_valid check they need to do it manually or drop the column first.

[morozov]

The change seems to have no effect for doctrine:schema:update or bin/console doctrine:migrations:diff.

@alexander-schranz could you provide some more details on how you used these commands? This way, we could reproduce your scenario and make sure that the necessary cases have been tested.

[alexander-schranz]

@morozov I don't see this behaviour as an issue.

I have an exist database with a table which looks like this (output of: SHOW CREATE TABLE app_activity):

CREATE TABLE `app_activity` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `data` longtext COLLATE utf8mb4_unicode_ci NOT NULL COMMENT '(DC2Type:json)',
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=11 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci

And entity which looks like this:

/**
 * @ORM\Entity
 * @ORM\Table(name="app_activity")
 */
class Activity
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue
     */
    private int $id;
    
    /**
     * @ORM\Column(type="json")
     *
     * @var mixed[]
     */
    private array $$data;
}

And when then use this current branch of doctrine/dbal the bin/console doctrine:schema:update --dump-sql or bin/console doctrine:migrations:diff does not change anything about this database. As from DBAL perspective the columns is already json (this is because of (DC2Type:json)).

Only when dropping the table/database and creating the schema again it will create it with the as native json type:

CREATE TABLE app_activity (
   id INT AUTO_INCREMENT NOT NULL,
   data JSON NOT NULL,
   PRIMARY KEY(id)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB;

Output of: SHOW CREATE TABLE app_activity is here correctly with the json_valid method then the JSON is still a longtext here:

CREATE TABLE `app_activity` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `data` longtext CHARACTER SET utf8mb4 COLLATE utf8mb4_bin NOT NULL CHECK (json_valid(`data`)),
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=11 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci

As written above I don't see this as an issue. It will just create future json fields with json_valid method. And so it will not be breaking change as it will not effect exist tables.

[morozov]

@alexander-schranz I may have said it wrong, I didn't imply that there is an issue.

From what you described, the current behavior looks right: as long as the new DBAL version doesn't produce an invalid diff while comparing the local schema with the one deployed by the old version, it should be fine.

I assume that the tools you used don't leverage the schema comparison improvement implemented in #4746. This way, the comparator compares only the column types and considers them equal.

I suspect that once provided a database platform, the comparator will detect a diff. The question is, will the SQL the target platform builds for this diff be valid. I can check that myself.

In the meantime, please rebase against 3.3.x since it's an improvement, not a bug fix.

[morozov]

The migration works well using both the old and the new comparison logic. But the introspection of the new column doesn't work properly (tested on MariaDB 10.3). Consider this:

$column = new Column('data', new JsonType(), []);
$localTable = new Table('test', [$column]);

$sm = $connection->createSchemaManager();
$sm->dropAndCreateTable($localTable);
$deployedTable = $sm->createSchema()
    ->getTable('test');
$comparator = $sm->createComparator();
$diff = $comparator->diffTable($deployedTable, $localTable);
$sql = $sm->getDatabasePlatform()->getAlterTableSQL($diff);
var_dump($sql);
// ALTER TABLE test CHANGE data data JSON NOT NULL

Since JSON is just an alias for LONGTEXT, the column is introspected as LONGTEXT which causes a false-positive diff.

[morozov]

Closing due to the lack of progress.