- Issue created by @mondrake
- Assigned to mondrake
- 🇮🇹Italy mondrake 🇮🇹
I am working on a PoC for the second use case from the IS.
- 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 3:27pm 31 December 2023 - Issue was unassigned.
- Status changed to Needs work
11 months ago 3:22pm 13 January 2024 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 3:26pm 13 January 2024 - First commit to issue fork.
- 🇳🇱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 9:45am 10 October 2024 - 🇮🇹Italy mondrake 🇮🇹
This issue being now a plan, IMHO the MR should be moved to a separate issue (if needed).