Problem/Motivation

Finding the group_content entity for a given entity (node,user,..) is extremely common and is often executed many times per page.

Proposed resolution

Add an index on that to improve query performance through a StorageSchema class like \Drupal\node\NodeStorageSchema::getEntitySchema().

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

Berdir created an issue. See original summary.

kristiaanvandeneynde’s picture

I would definitely be up for committing any patch that adds said index to the database. Currently a bit busy with the query access handler work, so a head-start would be appreciated :)

berdir’s picture

Will see if I can provide something, not high on my priority list either as we don't have enough data (yet) where this results in a measurable performance improvement. Also applying patches with update functions is always tricky.

lobsterr’s picture

Status: Active » Needs review
StatusFileSize
new2.14 KB

I have added the storage_schema, which provides the required index. I added an update hook, which add storage schema class to group content, but I couldn't find the way how to run update of the entity in order to apply the schema changes.
I do know how to update the field, when we add an index (addSharedTableFieldIndex) for specific field ()

$entity_definition_update_manager = \Drupal::entityDefinitionUpdateManager();
$entity_definition_update_manager->updateFieldStorageDefinition($entity_definition_update_manager->getFieldStorageDefinition('field_name', 'fast_node'));

In our case it is a composite index.
If somebody knows, how to do it, please let me know I will update the patch

berdir’s picture

Try to use updateFieldableEntityType() instead, maybe that will work better.

If that doesn't help, \Drupal\Core\Entity\Sql\SqlContentEntityStorageSchema::onEntityTypeUpdate() is the method you need to debug.

Basically, requiresEntityStorageSchemaChanges() needs to return TRUE and hasSharedTableStructureChange() needs to return FALSE, so it goes into the "sweet spot" of just re-creating the indexes.

lobsterr’s picture

StatusFileSize
new442 bytes
new2.46 KB

@Berdir, thank you for all your help. Debug helped me to understand, that I just missed use statement for GroupContentStorageSchema class and it was never used, but you are right no manual adding of the index is required.

kristiaanvandeneynde’s picture

StatusFileSize
new3.38 KB
+++ b/src/GroupContentStorageSchema.php
@@ -0,0 +1,28 @@
+    $schema = parent::getEntitySchema($entity_type, TRUE);

Why TRUE instead of passing on $reset?

I've changed this to $reset for now, rerolled the patch and moved a few files/comments around.

Ideally, this would have a test proving that the update works. But if anyone could point me to a core test showing that the used functionality indeed updates just the indexes, then I'm fine committing this without an update hook test.

lobsterr’s picture

@kristiaanvandeneynde. Yes, I have forgotten to remove TRUE, it was done for a test purpose.
I have checked other cases in the core and I didn't any tests for indexes
Some examples:
https://www.drupal.org/project/drupal/issues/2183231
https://www.drupal.org/project/drupal/issues/3007669

berdir’s picture

Status: Needs review » Reviewed & tested by the community

The update looks good and I just tested it on our site.

> But if anyone could point me to a core test showing that the used functionality indeed updates just the indexes, then I'm fine committing this without an update hook test.

It's a bit more complex than this, the update function does apply the non-destructive changes based on the changed definition and code. So right now, it does only make that change. In case we'd add more code to the storage schema class, it would attempt to add that too.

But there is no other way, that's how it is done in core as well.

kristiaanvandeneynde’s picture

Status: Reviewed & tested by the community » Fixed

Fixed, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.