- ππΊHungary huzooka Hungary ππΊπͺπΊ
Drupal core does allows non-integer IDs, so we used this approach:
- We added an integer column to Flaggings and this is filled up with the (target) entity ID if the ID is numeric.
- We modified the flag_relationship views plugin: we check whether the views entity's identifier has integer schema. IF so, we modify the column used for joining from the original entity_id column to our integer column. This makes our views queries fast, and does not break potential new features (e.g. flagging configs).
We have about 1M flaggings. Without this modification, the queries of our views weren't able to finish in 30 minutes (!).
With the above modifications, they take 1ms. - πΊπΈUnited States robphillips
OOTB entity flag type deriver does not support config entities (see link). Other modules could implement their own flag type that uses non-numeric IDs. I'm unaware of any that do, but its a possibility to consider nonetheless.
https://git.drupalcode.org/project/flag/-/blob/8.x-4.x/src/Plugin/Deriva...
For most sites converting
entity_id
to an integer is a quick and easy performance win. In my use case queries went from timing out to completing in under 100ms. However, the MR is incomplete because it does not change the flagging entity type definition and doesn't convert theflag_counts
table column. Attached patch fixes those issues. - πΊπΈUnited States robphillips
Updated patch changes
hook_update_N
sequence and installs the flaggingentity_id
definition. - π«π·France tree2009
Can we have two versions of flag module, one uses int as ID, the other uses varchar, so people can choose which version they like, and stop endless argument about int vs. varchar.
- π§πΎBelarus dewalt
Non integer IDs cause issues using PostgreSQL:
SQLSTATE[42883]: Undefined function: 7 ERROR: operator does not exist: bigint = character varying LINE 4: INNER JOIN "flagging" "flagging_taxonomy_term_field_data" ON taxonomy_term_field_data.tid = flagging_tax>
P.S. Looks like the issue has same patches to https://www.drupal.org/project/flag/issues/2888664 π New index for flagging performance in views relationships Needs work , install hook there contains a bit more operations.
- π¨πSwitzerland berdir Switzerland
Yes, there are multiple issues about this and one should be closed as a duplicate. This is a big change, we can _not_ just change to integer because that will cause problems on existing sites, especially if they happen to flag things with string ids.
#14 is the direction we have to go, similar to what entity_usage is doing on its own and what the dynamic_entity_reference project is doing.
Either way, this will be something for a 2.x version as it's a big change with a complex upgrade path and will impact views and queries and what not.
- π§πΎBelarus dewalt
@berdir, I've just like to write that this patch solved issue with view query (and possible issue with content deletion), but another issue happened - now there is error on PostgreSQL on config entities deletion, because it has string ID, and `entity_id` column is numeric now. Looks like there are 2 ways:
- Support content entities only with numeric IDs (huge change, but by the way, why flag initially supports config entities? Its look as really rare case to flag e.g. node type or view).
- Try to normalize all entity ID numbers to strings before pass to queries (may be I'll try to do it on Monday, first I would like to check Voting API, which uses "entity_reference" field type with dynamic numeric/string column type detection) - π©πͺGermany geek-merlin Freiburg, Germany
@huzooka #14:
> Drupal core does allows non-integer IDs, so we used this approach:Can you share your approach?
- π¨πSwitzerland berdir Switzerland
@dewalt: Yes, that's exactly the problem and why the fix isn't as simple as just changing it to integer. converting to string doesn't fix join/query performance. a solution is needed, but it's fairly complex and not quickfix.