Cannot delete a field which uses JSON type

ugh picked the wrong branch

mglaman → changed the visibility of the branch 3487533-cannot-delete-a to hidden.

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 ✨ Add "json" as core data type Active . 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.

@wim leers metatag uses text for all? https://git.drupalcode.org/project/metatag/-/blob/2.0.x/src/Plugin/Field.... I think I know what you're talking about, though. Is it the json_field? https://git.drupalcode.org/project/json_field/-/blob/8.x-1.x/src/Plugin/...

Note they use text for the sqlite_type! XB does not.

     'columns' => [
        'value' => [
          'type' => 'json',
          'pgsql_type' => 'json',
          'mysql_type' => 'json',
          'sqlite_type' => 'text',
          'not null' => FALSE,
        ],
      ],
    ];

re #9 I highly doubt ✨ Add "json" as core data type Active is going to make enough progress anytime soon.

we should at least at testing for the change and a comment about why the change is done.

I'll see if I can make a quick enough test. Hopefully without a field type and just manipulating a table with a JSON column.

Lets skip the test. We can do that in 📌 Replace ban_schema() implementation with ::ensureTableExists() Needs work .

Okay, so what's left? Add this line to each driver? I don't know what value the in-code comment would provide. Or would the comment be for why we only added it to SQLite and not the others?

      // @note: Only the SQLite driver has this field map to due to a fatal error caused by this driver's schema
      // @todo: Add to all drivers in XYZ
      'json:normal'     => 'JSON',

Something like this?

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