Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
#2183231: Make ContentEntityDatabaseStorage generate static database schemas for content entities will add automatic entity schemas but will remove the ability to dynamically alter schemas with hook_schema_alter().
Modules adding fields to entities might want to add multi-field indexes and thus need to alter the schema.
Comments
Comment #1
XanoHmmz, your use case would also link the abstract and storage-agnostic field API to the schema API much more. Didn't we want to let go of that 'dependency'?
Comment #2
fagoWhat's the benefit of having this hook compared to just using hook_schema_alter()?
Comment #3
tstoecklerRe #2: hook_schema_alter() does not work for entity schemas, that's the whole point why this was even considered.
Or do you mean we should invoke hook_schema_alter() in getSchema()? I would find that quite confusing.
Comment #4
fagooh, no I forgot that this is not exposed to hook_schema(), sry. So yeah, then hook_entity_schema_alter() seems reasonable.
Comment #5
yched CreditAttribution: yched commentedFWIW, I've always been reluctant to add a hook_field_schema_alter() :-)
Comment #6
Dave ReidI think this completely blocks a port of File entity in D8 now, if I'm not mistaken.
Comment #7
tstoecklerComment #8
catchhook_schema_alter() only affects the schema definition for functions like drupal_write_record() and drupal_schema_field_sql(), or schema module in contrib. It never affected the actual tables themselves - always have to make those changes manually anyway. So hook_entity_schema_alter() or hook_field_shema_alter() would be very different from hook_schema_alter() in terms of what it would have to support.
@Dave Reid could file entity override the class to add the bundle definition etc. rather than altering just the schema?
Comment #9
tstoecklerI just checked file_entity_schema_alter() in D7 and it adds a 'type' column to the file_managed table. In D8 it would add a field definition in hook_entity_base_field_info() and the column would be generated automatically.
Just also read #1, I had missed that. Can you clarify what you mean by the 'dependency' you speak of? Not sure what you mean.
So to clarify. The only use-case that has been mentioned so far (albeit not in the issue summary, sorry!) is the following:
- You are a Contrib module add a field to an entity type and you need/want to add an cross-column index of your field to a column that is not part of your field. I.e. a cross-column index with the 'id' columns. I.e. the following code:
Anything else is already supported:
- You want to add a field with an index: Just put the index in your schema().
- You are not contrib, but a custom module on a custom site: Just alter the storage and go crazy in getSchema() (Even in Contrib, modules such as File Entity could validly swap the storage, as it shouldn't be supported to have both File Entity and Some Better FIle Entity (or whatever) installed on a site, which both fundamentally alter the behavior of the entity. However, generally, swapping the storage class should be discouraged in contrib.)
Comment #10
catchPer #8 this doesn't actually add an index, because hook_schema_alter() is not run on the schema that modules are installed with.
You always have to do db_add_index() in an install or update function - and that's possible to do in 8.x still too.
Comment #11
yched CreditAttribution: yched commentedwhat @catch says :-)
Comment #12
tstoecklerSo #10 is correct, but it would be strange, though, to have the actual database schema not be identical to the result of $storage->getSchema(), no?
Comment #13
plachJust marked #2385327: Provide a way to add new indexes to entities as a duplicate of this. As we imagined there are real use cases for altering index definitions, we need to fix at least that.
Comment #14
plachThis is probably major, although it's no longer blocking the file_entity port.
Comment #15
plachAnother tag
Comment #16
plach