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.

Proposed resolution

Remaining tasks

User interface changes

API changes

Comments

Xano’s picture

Hmmz, 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'?

fago’s picture

What's the benefit of having this hook compared to just using hook_schema_alter()?

tstoeckler’s picture

Re #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.

fago’s picture

oh, no I forgot that this is not exposed to hook_schema(), sry. So yeah, then hook_entity_schema_alter() seems reasonable.

yched’s picture

FWIW, I've always been reluctant to add a hook_field_schema_alter() :-)

Dave Reid’s picture

I think this completely blocks a port of File entity in D8 now, if I'm not mistaken.

tstoeckler’s picture

Status:Postponed» Active
catch’s picture

hook_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?

tstoeckler’s picture

I 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:

<?php
function foo_entity_base_field_info($entity_type) {
  if (
$entity_type->id() == 'bar') {
   
$fields['foobar'] = ...;
    return
$fields;
  }
}

function
foo_entity_schema_alter($entity_type, $schema) {
  if (
$entity_type->id() == 'bar') {
   
$schema['bar']['indexes']['bar_id_foobar'] = array('id', 'foobar');
  }
}
?>

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.)

catch’s picture

Per #8 this doesn't actually add an index, because hook_schema_alter() is not run on the schema that modules are installed with.

<?php
function foo_entity_schema_alter($entity_type, $schema) {
  if (
$entity_type->id() == 'bar') {
   
$schema['bar']['indexes']['bar_id_foobar'] = array('id', 'foobar');
  }
}
?>

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.

yched’s picture

what @catch says :-)

tstoeckler’s picture

So #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?

plach’s picture

Just 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.

plach’s picture

Priority:Normal» Major
Issue tags:-Contributed project blocker+API addition

This is probably major, although it's no longer blocking the file_entity port.

plach’s picture

Issue tags:+Entity Field API

Another tag

plach’s picture

Issue tags:+entity storage