New index for flagging performance in views relationships

Thank you for both patches! Right now I'm not sure if the config entity patch will get in, so I'm inclined to get the first patch in and add the indexing to the patch in the config entity issue.

I am not an expert in this area,

so I want to talk this through ... I suspect different parts of this will be obvious to each reader and if we put our heads together then we can make progress.. I will do any legs work regarding research and patch making.

Drupal\Tests\flag\FunctionalJavascript\LinkTypeAjaxTest::testAjaxLink
Drupal\Core\Entity\EntityStorageException: Exception thrown while performing a schema update. MySQL needs the 'uid, flag_id, entity_id' field specification in order to normalize the 'flagging_field__uid__flag_id__entity_id' index

Do I read that correctly ....

In addition to our definition of the "flag config entity" in flag.schema we also need to add definitions in flag.install flag_schema()?

I will keep this at needs review but as a note to myself when I come back to make the next patch we currently use array() when [] is preferred.

     $schema['flagging']['indexes'] += array(
       'entity_id__uid' => array('entity_id', 'uid'),
+      'flagging_field__uid__flag_id__entity_id' => array('uid, flag_id, entity_id'),
     );

I'm curious about the above as well as have been unable to workaround this.

I needed a quick workaround as was unsure how to solve this via /src/Entity/Storage/FlaggingStorageSchema.php. I just leveraged hook_install + hook_update_n.

(I have a site running on Drupal 7 that had Views involving flags get extremely slow. I added an index to uid and a query that was taking 60 seconds to complete now takes less than .1 seconds. I believe it may be related to this issue in Drupal 8's Flags module. Glad to see it being addressed before I upgrade.)

I can confirm this. We have a website with views counting flag count using aggregation and the performance of these pages is very bad.

I can confirm this too. We have a flagging table with +500.000 records and it's super slow 2-3 minutes query execution time.

I checked duplicate indexes using "pt-duplicate-key-checker", and I found this:

# ########################################################################
# drupal8.flagging
# ########################################################################

# flagging_field__flag_id__target_id is a left-prefix of flag_id__uid__session_id
# Key definitions:
#   KEY `flagging_field__flag_id__target_id` (`flag_id`),
#   KEY `flag_id__uid__session_id` (`flag_id`,`uid`,`session_id`(191))
# Column types:
#         `flag_id` varchar(32) character set ascii not null comment 'the id of the target entity.'
#         `uid` int(10) unsigned not null comment 'the id of the target entity.'
#         `session_id` varchar(255) default null
# To remove this duplicate index, execute:
ALTER TABLE `drupal8`.`flagging` DROP INDEX `flagging_field__flag_id__target_id`;

Removing this index does not help.

I can confirm too. Loading page about 2-3 minutes. With 5000 node + . Why anybody cant find a way to repair it? flag not working for drupal 8 correctly =(

re-roll

Did not help.
I have cleared my flagging table. And its start working faster (because table just cleaned) . But loading page with flags takes about 6 seconds. And this page without flags takes about 2 sec.
How to make it work faster?

This is a reroll of #8.

Re:10: It looks like $schema->addIndex($table, $name, $fields, $spec) expects the full table specification as the fourth argument (see https://www.drupal.org/project/drupal/issues/2537928). Attached patch adds the full spec, we'll see what testbot says.

Adding this index allowed me to sort a node view on flagging count (43,617 nodes, 145,824 flaggings in my db), as long as the view uses flaggings by the current user in the relationship:

Query build time: 17.38 ms
Query execute time: 35.86 ms
View render time: 74.69 ms

If I configure the flagging relationship to show flaggings by any user (which is what I need), the preview in Views never even gets rendered, and the query just seems to hang (same behaviour as without the patch).

After playing around with the view some more, it seems that it's the 'Sort by flagging count' that's causing this to become very slow.

Even views without 'Sort by flagging count' just showing the flagging count as a field can be very slow with large amount of nodes and flags on them.

The main problem is the relations of the flag. If you remove this, then everything loads quickly. In node specific page, but without relationship. it works fast there. But on the pages of terms it takes a very long time to load. Loads quickly only if the person is logged in
And i have not sort by flag, and still have performance problem for anonymous users

Please help with this problem( Flag module normally works only for authed users.

Fixed the FlaggingStorageSchema index array definition.

It did not help. If there are a lot of nodes, the site slows down a lot. Or the database crashes altogether

I have same problem too if I use Flags as Relations in my views. Site works very slow. If delete Flags from Relations - site works very fast. But I need this relation for 'Sort by flagging count'

Patch #26 works good for new install module Flag (patched #26).
But if I using patch for existing flagging entities. I have error. I can't update flagging entities. I use module https://www.drupal.org/project/entity_update but it don't work.

I'm having issues with views relationships and the query using the wrong index as well. The query that views generates is taking > 6 seconds to execute. Adding FORCE INDEX (PRIMARY) after the first INNER JOIN of flagging table fixes this. Adding the index in the patch above does not.

I guess I'm going to try https://www.drupal.org/project/views_index_hint, but would like to hear what others do to resolve this!

SELECT 
  users_field_data.uid AS uid, 
  flagging_users_field_data.id AS flagging_users_field_data_id, 
  users_field_data.login AS users_field_data_login_minute \
FROM users_field_data users_field_data 
INNER JOIN 
  flagging user__applicant_status ON users_field_data.uid = user__applicant_status.entity_id 
INNER JOIN 
  flagging flagging_users_field_data ON users_field_data.uid = flagging_users_field_data.entity_id AND flagging_users_field_data.flag_id = 'approved_audit' 
INNER JOIN 
  user__roles user__roles ON users_field_data.uid = user__roles.entity_id AND user__roles.deleted = '0' 
WHERE 
  user__roles.roles_target_id = 'teacher' AND 
  user__applicant_status.entity_type = 'user' AND 
  user__applicant_status.flag_id = 'applicant_active' AND 
  users_field_data.status = '1' AND
  users_field_data.applicant_stage = 'approved'
LIMIT 25 OFFSET 25; 

Associated EXPLAIN

+------+-------------+---------------------------+------+---------------------------------------------------+------------------------------------+---------+--------------------------------------------------+--------+------------------------------------+
| id   | select_type | table                     | type | possible_keys                                     | key                                | key_len | ref                                              | rows   | Extra                              |
+------+-------------+---------------------------+------+---------------------------------------------------+------------------------------------+---------+--------------------------------------------------+--------+------------------------------------+
|    1 | SIMPLE      | flagging_users_field_data | ref  | flagging_field__flag_id__target_id,entity_id__uid | flagging_field__flag_id__target_id | 34      | const                                            | 566    | Using index condition; Using where |
|    1 | SIMPLE      | users_field_data          | ref  | PRIMARY,user__id__default_langcode__langcode      | PRIMARY                            | 4       | epi8_migrate.flagging_users_field_data.entity_id | 1      | Using where                        |
|    1 | SIMPLE      | user__roles               | ref  | PRIMARY,roles_target_id                           | PRIMARY                            | 5       | epi8_migrate.users_field_data.uid,const          | 1      | Using where                        |
|    1 | SIMPLE      | user__applicant_status    | ref  | flagging_field__flag_id__target_id,entity_id__uid | flagging_field__flag_id__target_id | 34      | const                                            | 103978 | Using index condition; Using where |
+------+-------------+---------------------------+------+---------------------------------------------------+------------------------------------+---------+--------------------------------------------------+--------+------------------------------------+

As a follow up to my last comment, we've ended up converting the schema for `entity_id` in the flagging table to an integer column. We also added an index for flag_id and entity_id.

I realize this may cause issues for others including those wanting to flag config entities that have non-numeric ids. For us, it dropped our query time from 35 seconds to 0.035 ms.

That is very drastic. Maybe this module should add option for core entities that are known to have numeric ids to offer numeric id in flagging table too, some UI parameter maybe?

@andyg5000
Woah thx for this catch. I also changed the type for 'entity_id' to int and query execution time dropped from about 5 seconds to ~0.0xx seconds.

Could somebody create a new patch with the latest suggestions and or fixes? The last one is from about a year ago and at least one person reported that is only works on fresh installs.

Where using the flag module on many places and I'd be happy if i could apply a patch that works on existing installs :)

I created a patch, which should be working for new installs and has an update hook (drush updb) that fixes existing installs.

Be aware that changing the column type could have drawbacks if something expects this column to be a string but it is now an integer. I tested it for my projects and it works fine. I think it will be ok for most cases because the entity_id in drupal is always an integer.

@mvogel , thanks for taking your time to create a patch. I applied it and it seems that everthing still works as expected. Still getting dupe results, but thats a core bug :)

This is an updated version of the patch from #26 that correctly updates the entity definition so there is no error in the site status report about outdated field definitions.

(To be clear: this patch merely adds the missing index; it does not change the column type.)

Adds a missing import from #38.

I tested the patch from #36 and can confirm that it's working.

An unspoken rule in Drupal is that entity_id columns have always been numeric. Adding more indexes will not help much if those indexes are not getting hit from the datatypes being different. I tested patch #36 and it works fine. Here is an updated patch #36 with the proper patch context.

#41 worked for me, thanks.

This is night and day with my Views with flag attached. Is this production ready?