Use events in Database Schema operations

Created on 22 December 2023, 12 months ago

Problem/Motivation

We could leverage more on the event system to manage database schema operations.

For example:

1. #3410312-12: Flood database backend ::isAllowed() should call ::ensureTableExists() →

@mondrake

[...] I find on-the-fly table creation a rather bad approach. If we had the table created at installation, we wouldn’t have to care about this. On-the-fly table creation breaks transactions for DBs that do not support transactional DDL - not just the transaction where ensureTable is called, also any concurrent transaction from other sessions. [...]

@catch

[...] we've got a few more options now - for example services that need to create tables could subscribe to a module installed event and create the storage then [...]

2. #3410312-14: Flood database backend ::isAllowed() should call ::ensureTableExists() →

@mondrake

[...] events would help in 📌 Adding GIN and GIST indexes to PostgreSQL databases RTBC , to allow database drivers to decorate db objects specifications with db specific features [...]

Steps to reproduce

Proposed resolution

Remaining tasks

Discuss

User interface changes

API changes

Data model changes

Release notes snippet

✨ Feature request
Status

Active

Version

11.0 🔥

Component
Database  →

Last updated about 3 hours ago

  • Maintained by
  • 🇳🇱Netherlands @daffie
Created by

🇮🇹Italy mondrake 🇮🇹

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mondrake
  • Assigned to mondrake
  • 🇮🇹Italy mondrake 🇮🇹

    I am working on a PoC for the second use case from the IS.

  • Merge request !5984PoC → (Open) created by mondrake
  • Pipeline finished with Failed
    11 months ago
    Total: 206s
    #70029
  • Assigned to mondrake
  • 🇮🇹Italy mondrake 🇮🇹

    Here's the idea of the PoC.

    We have the event service available since the very first DDL statement executed during an installation, so we can build around it. (Exception, currently - Drush does not register services defined by the database driver's module, so an event subscriber defined into it won't execute - see https://github.com/drush-ops/drush/issues/5846).

    Call the following 'hook_schema_alter strikes back' if you're a fan of the Force.

    1. We replace the current way of defining a table's structure via a nested array, with a structure of value objects defined in the Drupal\Core\Database\SchemaDefinition namespace. This structure is immutable and database abstract. For example, the db-specific field type array key 'mysql_type', or the 'mysql_engine' will no longer be defined here.

    A table currently represented like

        $schema = [
          'description' => 'The base table for configuration data.',
          'fields' => [
            'collection' => [
              'description' => 'Primary Key: Config object collection.',
              'type' => 'varchar_ascii',
              'length' => 255,
              'not null' => TRUE,
              'default' => '',
            ],
            'name' => [
              'description' => 'Primary Key: Config object name.',
              'type' => 'varchar_ascii',
              'length' => 255,
              'not null' => TRUE,
              'default' => '',
            ],
            'data' => [
              'description' => 'A serialized configuration object data.',
              'type' => 'blob',
              'not null' => FALSE,
              'size' => 'big',
            ],
          ],
          'primary key' => ['collection', 'name'],
        ];
    

    Will then become sth like

        new Table(
          name: 'config',
          description: 'The base table for configuration data.',
          columns: [
            new Column(
              name: 'collection',
              description: 'Primary Key: Config object collection.',
              type: 'varchar_ascii',
              length: 255,
              notNull: TRUE,
              default: '',
            ),
            new Column(
              name: 'name',
              description: 'Primary Key: Config object name.',
              type: 'varchar_ascii',
              length: 255,
              notNull: TRUE,
              default: '',
            ),
            new Column(
              name: 'data',
              description: 'A serialized configuration object data.',
              type: 'blob',
              notNull: FALSE,
              size: 'big',
            ),
          ],
          primaryKey: new PrimaryKey(['collection', 'name']),
        );
    

    2. This strawman structure is then used to build a concrete specification of Drupal\Core\Database\Schema namespace objects or an override of any of them in the database driver module itself - via an event subscriber. Using the event system allows ANY subscriber to alter the CONCRETE specification, keeping the abstract definition untouched. This will allow defining any db-specifc property (for example, the type of index for 📌 Adding GIN and GIST indexes to PostgreSQL databases RTBC in the overriding classes, and we will be able to have full static analysis of the code with defined properties.

    3. The CONCRETE specification is then used by the drivers' Schema class to actually perform the DDL statements necessary. Note how, most likely, a large part of what is in those classes today will be moved to the subscriber instead.

    The MR is a PoC, it runs OK a number of tests locally, but fails PHPCS and PHPStan. I am not going to fix that though, I'd rather appreciate feedback on the concept.

    Turning this issue into a meta. My next step will be to start working on point 1 above, which is all by itself a large endeavour. Will open a separate issue for that.

  • Status changed to Needs review 11 months ago
  • 🇮🇹Italy mondrake 🇮🇹
  • Issue was unassigned.
  • 🇮🇹Italy mondrake 🇮🇹
  • Status changed to Needs work 11 months ago
  • The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Status changed to Needs review 11 months ago
  • 🇮🇹Italy mondrake 🇮🇹
  • First commit to issue fork.
  • Pipeline finished with Failed
    9 months ago
    Total: 178s
    #116160
  • 🇳🇱Netherlands daffie

    @mondrake: Could you explain to me how with this change, it would help with 📌 Adding GIN and GIST indexes to PostgreSQL databases RTBC . Also with DB's that do not support transactional DDL. Why does this change make it better for those DB's? I missing why with this change it will be better.

  • 🇬🇧United Kingdom catch

    Moving this to a plan.

    We discussed the general problem (on-the-fly table creation and transactions) at Drupal dev days but I don't think that discussion made it back into an issue, this is probably the right one.

    Generally, I still think it's a good idea to make database-implementations-of-services (cache, lock, flood etc.) responsible for creating their database schema, so that if you have a redis backend installed, you don't have 10 empty cache tables in your database for no reason.

    However, we could use an interface + tagged services to allow those services to create their schema during module install at an explicit point, so that the fallback is only necessary if you change services.yml to switch an implementation (e.g. from redis to sql) on runtime. And we could maybe do the same tagged services check on a cache clear all so that sites can trigger table creation without having to wait for an entity save or something to do it for them.

    This way we'd still have on-demand table creation but we'd be able to... demand it.

  • Status changed to Active about 2 months ago
  • 🇮🇹Italy mondrake 🇮🇹

    This issue being now a plan, IMHO the MR should be moved to a separate issue (if needed).

Production build 0.71.5 2024