Entity_id fields should be created as integer data type

Created on 8 November 2017, about 7 years ago
Updated 7 February 2023, almost 2 years ago

When we install flag module, it creates 2 tables.

flag_counts and flagging

Both have entity_id field but those are created as varchar datatype. This makes performance poor if we have more entries. Indexing is not working properly.

When we switched it to integer datatype, performance is good.

πŸ› Bug report
Status

Needs review

Version

4.0

Component

Flag core

Created by

πŸ‡ΊπŸ‡ΈUnited States dsim Irving, Texas

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡­πŸ‡ΊHungary huzooka Hungary πŸ‡­πŸ‡ΊπŸ‡ͺπŸ‡Ί

    Drupal core does allows non-integer IDs, so we used this approach:

    1. We added an integer column to Flaggings and this is filled up with the (target) entity ID if the ID is numeric.
    2. 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 the flag_counts table column. Attached patch fixes those issues.

  • πŸ‡ΊπŸ‡ΈUnited States robphillips

    Updated patch changes hook_update_N sequence and installs the flagging entity_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.

Production build 0.71.5 2024