Cannot modify a table which uses JSON type

Looks straightforward, so there are no issues with merging this.

For consistency should we add this to getFieldTypeMap() for the other drivers anyway?

So this bit in the Experience Builder module revealed this bug:

  /**
   * {@inheritdoc}
   */
  public static function schema(FieldStorageDefinitionInterface $field_definition) {
    return [
      'columns' => [
        'tree' => [
          'description' => 'The component tree structure.',
          'type' => 'json',
          'pgsql_type' => 'jsonb',
          'mysql_type' => 'json',
          'sqlite_type' => 'json',
          'not null' => FALSE,
        ],
        'props' => [
          'description' => 'The prop values for each component in the component tree.',
          'type' => 'json',
          'pgsql_type' => 'jsonb',
          'mysql_type' => 'json',
          'sqlite_type' => 'json',
          'not null' => FALSE,
        ],
      ],
      'indexes' => [],
      'foreign keys' => [
        // @todo Add the "hash" part the proposal at https://www.drupal.org/project/drupal/issues/3440578
      ],
    ];
  }

— \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::schema()

But I made this use exactly the same pattern that Metatag uses, hence this bit:

* @see https://git.drupalcode.org/project/metatag/-/blob/2.0.x/src/Plugin/Field/FieldType/MetatagFieldItem.php

— \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem.

So why is the Metatag module not running into the same problem? 🤔

I think we should add JSON storage support in #3343634: Add "json" as core data type to Schema and Database API. If that takes too long, we should at least at testing for the change and a comment about why the change is done.

NW for #9. I think this is OK as a stopgap if we can get test coverage (and also answer #7). I would definitely like to see JSON support done properly in core but I'm also concerned about how long it will take given the tight timelines we have on Experience Builder.

Lets skip the test. We can do that in #3379255: [PP-1] Replace ban_schema() implementation with ::ensureTableExists().

#11: oh, damn, mea culpa! 😬 I failed to document that correctly then 😮‍💨

Increasing priority as suggested by @catch at

The core issue hasn't had an update for six months, seems like the priority could be increased from 'normal'. But also there needs to be a tracking issue for XB to update this code once it's fixed. That will probably requires a dependency on the minor release that fixes the bug, unless we determine it can be fixed in a patch release of core.

— #3520923: [PP-1] Use `json` schema type for SQLite and remove `text` workaround

I'm a bit confused by #12, ban module doesn't use a JSON field type, and it's slated for removal to contrib anyway, is #3379255: [PP-1] Replace ban_schema() implementation with ::ensureTableExists() the right issue link?

Sounds to me like @daffie is happy for this to go in without tests - so we just need a comment explaining why we're doing a stopgap and a link to the bigger issue in a @todo? Anything else?

@catch indicated in #3520923: [PP-1] Use `json` schema type for SQLite and remove `text` workaround this could also go into a bugfix release.

This isn't limited to fields, was able to add a simple test to the generic schema test that fails without this change on sqlite (only).

Ready for review

MR looks trivial! 🥳

Can a core committer please queue the test-only CI job on SQLite to prove @larowlan's was able to add a simple test to the generic schema test that fails without this change on sqlite (only)? 🙏

sqlite pipeline running at https://git.drupalcode.org/issue/drupal-3487533/-/pipelines/519476

The test only job on https://git.drupalcode.org/issue/drupal-3487533/-/jobs/5524833 isn't showing a failure. This might be an issue with the test-only job though, not sure the last time we needed to run that job on a specific database.

0c728283617a:/data$ phpunit -c core core/modules/sqlite/tests/src/Kernel/sqlite/SchemaTest.php --filter=testSchema
⏳️ Bootstrapped tests in 0 seconds.
🐘 PHP Version 8.3.11.
💧 Drupal Version 11.3-dev.
🗄️  Database engine sqlite.
PHPUnit 10.5.46 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.11
Configuration: /data/app/core/phpunit.xml

E.....                                                              6 / 6 (100%)

Time: 00:10.267, Memory: 14.00 MB

There was 1 error:

1) Drupal\Tests\sqlite\Kernel\sqlite\SchemaTest::testSchema
Exception: Unable to parse the column type JSON

/data/app/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php:534
/data/app/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php:42
/data/app/core/modules/sqlite/src/Driver/Database/sqlite/Schema.php:567
/data/app/core/tests/Drupal/KernelTests/Core/Database/DriverSpecificSchemaTestBase.php:283

Output from running locally shows the expected error from the issue summary in the test

Wanted to confirm this really does fail on gitlab just in case there's an sqlite version issue or similar, so opened https://git.drupalcode.org/project/drupal/-/merge_requests/12359 with the fix reverted. Tests are still running so hasn't answered the question yet.

Here we go, just the test only job is for whatever reason not able to show it:

https://git.drupalcode.org/issue/drupal-3487533/-/jobs/5531634#L970

Late here but now the rc is out I think we should get this in asap, so will try to commit tomorrow if no-one beats me to it.

Thanks @catch

This looks good.

Is the only follow-up here #3343634: Add "json" as core data type to Schema and Database API or do we need an additional small follow-up to add the mapping to the other core drivers? Tagging for follow-up to determine that but it might be existing JSON issues cover it enough.

Committed/pushed to 11.x and cherry-picked to 11.2.x, thanks!

Thanks @catch, yes that's the only follow up we need as far as I'm aware - removing the tag