Projet

Général

Profil

Development #66011

SQL: common_role: index manquant, type de colonne et requête sub-optimaux

Ajouté par Pierre Ducroquet il y a presque 2 ans. Mis à jour il y a presque 2 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
07 juin 2022
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Repéré sur l'environnement de recette.

La table common_role a une colonne nommée "uuid", mais de type character varying(31). Je suggère de migrer vers une vraie colonne de type uuid : cela prend un peu moins de place, mais surtout ça va permettre que l'attribut soit stocké directement dans la table et non pas dans le stockage adjacent pour valeurs trop longues.
De plus, j'ai repéré la requête suivante, assez gourmande à l'échelle de l'environnement complet:

SELECT "auth_group"."id", "auth_group"."name", "common_role"."group_ptr_id", "common_role"."uuid", "common_role"."description", "common_role"."details", "common_role"."emails", "common_role"."emails_to_members" FROM "common_role" INNER JOIN "auth_group" ON ("common_role"."group_ptr_id" = "auth_group"."id") WHERE ("common_role"."uuid" IN ('a', 'b', 'c') AND NOT ("common_role"."group_ptr_id" IN (SELECT U0."id" FROM "auth_group" U0 INNER JOIN "auth_user_groups" U1 ON (U0."id" = U1."group_id") WHERE U1."user_id" = 42)));

Or, la colonne uuid n'est pas indexée. Donc common_role va subir un scan séquentiel systématiquement.
De plus, cette requête n'est pas optimale : elle référence deux fois auth_group, alors que ce n'est pas nécessaire. On peut la réécrire comme suit:

SELECT "auth_group"."id", "auth_group"."name", "common_role"."group_ptr_id", "common_role"."uuid", "common_role"."description", "common_role"."details", "common_role"."emails", "common_role"."emails_to_members" FROM "common_role" INNER JOIN "auth_group" ON ("common_role"."group_ptr_id" = "auth_group"."id") WHERE ("common_role"."uuid" IN ('a', 'b', 'c', 'd') AND NOT exists(select 1 from auth_user_groups where user_id = 42 and group_id = "common_role"."group_ptr_id"));

Le plus gros gain viendrait bien sûr de l'indexation de la colonne, mais je pense qu'un passage en type uuid serait également bénéfique pour les performances, et tout simplement pour la sécurité des données (mieux vaut avoir une contrainte)


Fichiers

Révisions associées

Révision 3690b428 (diff)
Ajouté par Pierre Ducroquet il y a presque 2 ans

hobo agent: index role::uuid column (#66011)

Historique

#1

Mis à jour par Pierre Ducroquet il y a presque 2 ans

  • Projet changé de Combo à Hobo
#2

Mis à jour par Pierre Ducroquet il y a presque 2 ans

  • Fichier 0001-hobo-agent-index-the-common-Role.uuid-column-and-use.patch ajouté
  • Statut changé de Nouveau à Solution proposée
  • Patch proposed changé de Non à Oui
#3

Mis à jour par Pierre Ducroquet il y a presque 2 ans

  • Fichier 0001-hobo-agent-index-the-common-Role.uuid-column-and-use.patch supprimé
#5

Mis à jour par Emmanuel Cazenave il y a presque 2 ans

Il y a des lignes commentées dans ton patch.

#6

Mis à jour par Pierre Ducroquet il y a presque 2 ans

Emmanuel Cazenave a écrit :

Il y a des lignes commentées dans ton patch.

Oui, ma faute, j'ai envoyé un patch incorrect, j'ai voulu renvoyer le bon et j'ai cafouillé dans mes dossiers.
J'en renvoie un plus complet après, mais je vois que le changement de type de colonne n'est pas si facile que ça...

#7

Mis à jour par Pierre Ducroquet il y a presque 2 ans

Voici le patch à jour et complété niveau tests unitaires.
Le changement du type dans PG a plus d'impact que ce que je pensais. Du coup si vous trouvez que ça fait trop, on peut le laisser choir et ne faire que l'ajout de l'index sur la colonne...

#8

Mis à jour par Emmanuel Cazenave il y a presque 2 ans

Pierre Ducroquet a écrit :

Le changement du type dans PG a plus d'impact que ce que je pensais. Du coup si vous trouvez que ça fait trop, on peut le laisser choir et ne faire que l'ajout de l'index sur la colonne...

De mon coté je serais frileux et ferait uniquement l'ajout de l'index, Hobo.agent.common est utilisé par toutes les applications, si la migration plante parce que quelque part un uuid n'en est pas un, toutes les briques se retrouvent par terre en même temps.

Sauf à faire d'abord un tour de toutes les bases pour vérifier que ça va être ok ? Je ne sais pas c'est toi le spécialiste.

#9

Mis à jour par Benjamin Dauvergne il y a presque 2 ans

Emmanuel Cazenave a écrit :

De mon coté je serais frileux et ferait uniquement l'ajout de l'index, Hobo.agent.common est utilisé par toutes les applications, si la migration plante parce que quelque part un uuid n'en est pas un, toutes les briques se retrouvent par terre en même temps.

C'est moins vrai que pour User.uuid sur authentic mais oui assez d'accord avec Manu, je ne ferai pas les deux opérations en même temps. On vit très bien dans w.c.s. avec des tas de colonne text qui pourraient être des integer (formdata.user_id ?), je ne ferai pas de celle-ci une priorité.

#10

Mis à jour par Pierre Ducroquet il y a presque 2 ans

Ainsi soit-il :)
Nouveau patch ci-joint, avec uniquement l'index.

#11

Mis à jour par Emmanuel Cazenave il y a presque 2 ans

  • Statut changé de Solution proposée à Solution validée
#12

Mis à jour par Pierre Ducroquet il y a presque 2 ans

  • Statut changé de Solution validée à Résolu (à déployer)

Mergé

commit 3690b42820f2d0f6ea5836c34eb1ef46ef17017e (HEAD -> main, origin/wip/66011_index_role_uuid, origin/main, origin/HEAD, wip/66011_index_role_uuid)
Author: Pierre Ducroquet <pducroquet@entrouvert.com>
Date:   Wed Jun 8 10:03:32 2022 +0200

    hobo agent: index role::uuid column (#66011)
#14

Mis à jour par Transition automatique il y a presque 2 ans

  • Statut changé de Résolu (à déployer) à Solution déployée
#15

Mis à jour par Transition automatique il y a plus d'un an

Automatic expiration

Formats disponibles : Atom PDF