entrouvert
/
authentic
16
0
Fork 0
Code Issues Pull Requests 33 Releases Activity

manager: search role with unaccent lookup (#87906) #272

Merged
bdauvergne merged 1 commits from wip/87906-search-role-unaccent into main 2024-03-30 08:56:19 +01:00
Conversation 9 Commits 1 Files Changed 5 +57 -2
tnoel commented 2024-03-08 12:03:05 +01:00
Owner
Copy Link
No description provided.
tnoel added 1 commit 2024-03-08 12:03:05 +01:00
gitea/authentic/pipeline/head This commit looks good Details
cdf52169c9 manager: search role with unaccent lookup (#87906)
tnoel commented 2024-03-08 15:08:14 +01:00
Author
Owner
Copy Link

Le patch est très simple, mais la doc de Django précise ceci par rapport à l'usage de __unaccent__:

« unaccent lookups should perform fine in most use cases. However, queries using this filter will generally perform full table scans, which can be slow on large tables. In those cases, using dedicated full text indexing tools might be appropriate. »

Et là je ne sais pas très bien ce qu'ils appellent des « large tables », si c'est un grand nombre de colonnes ou la présence de beaucoup de lignes (dans ce cas ça n'ira pas, on veut quelque chose qui répond (très) vite).

Cf https://docs.djangoproject.com/en/3.2/ref/contrib/postgres/lookups/#unaccent

Le patch est très simple, mais la doc de Django précise ceci par rapport à l'usage de `__unaccent__`: _« unaccent lookups should perform fine in most use cases. However, queries using this filter will generally perform full table scans, which can be slow on large tables. In those cases, using dedicated full text indexing tools might be appropriate. »_ Et là je ne sais pas très bien ce qu'ils appellent des « large tables », si c'est un grand nombre de colonnes ou la présence de beaucoup de lignes (dans ce cas ça n'ira pas, on veut quelque chose qui répond (très) vite). Cf https://docs.djangoproject.com/en/3.2/ref/contrib/postgres/lookups/#unaccent
tnoel changed title from WIP: manager: search role with unaccent lookup (#87906) to manager: search role with unaccent lookup (#87906) 2024-03-08 15:08:33 +01:00
tnoel changed title from manager: search role with unaccent lookup (#87906) to WIP: manager: search role with unaccent lookup (#87906) 2024-03-11 10:51:01 +01:00
tnoel commented 2024-03-11 10:51:07 +01:00
Author
Owner
Copy Link

Vu avec des collègues : il faut "ajouter un index au niveau unaccent" et c'est déjà quelque chose qui existe, cf https://git.entrouvert.org/entrouvert/authentic/src/branch/main/src/authentic2/migrations/0028_trigram_unaccent_index.py

Vu avec des collègues : il faut "ajouter un index au niveau unaccent" et c'est déjà quelque chose qui existe, cf https://git.entrouvert.org/entrouvert/authentic/src/branch/main/src/authentic2/migrations/0028_trigram_unaccent_index.py
tnoel commented 2024-03-11 13:46:56 +01:00
Author
Owner
Copy Link

Collègues, je veux bien un peu plus d'éclairage sur ce qu'il faut faire ici. Quel index faut-il créer pour que le lookup __unaccent__fonctionne ? Ou faut-il faire autrement que ce d'utiliser ce lookup ?

Collègues, je veux bien un peu plus d'éclairage sur ce qu'il faut faire ici. Quel index faut-il créer pour que le lookup `__unaccent__`fonctionne ? Ou faut-il faire autrement que ce d'utiliser ce lookup ?
pducroquet commented 2024-03-11 14:29:11 +01:00
Owner
Copy Link

J'ai pas toute l'histoire de django et d'authentic sur tout ça. De ma compréhension de Django, il manque en plus du lookup une définition de la fonction (classe ImmutableUnaccent héritant de Func), qu'on peut alors utiliser dans un index.
La solution actuelle d'authentic n'a pas tout ça et se base sur un index créé en dur dans une migration, et non dans un modèle.

J'ai pas toute l'histoire de django et d'authentic sur tout ça. De ma compréhension de Django, il manque en plus du lookup une définition de la fonction (classe ImmutableUnaccent héritant de Func), qu'on peut alors utiliser dans un index. La solution actuelle d'authentic n'a pas tout ça et se base sur un index créé en dur dans une migration, et non dans un modèle.
bdauvergne commented 2024-03-11 14:34:35 +01:00
Owner
Copy Link

J'ai pas toute l'histoire de django et d'authentic sur tout ça. De ma compréhension de Django, il manque en plus du lookup une définition de la fonction (classe ImmutableUnaccent héritant de Func), qu'on peut alors utiliser dans un index.
La solution actuelle d'authentic n'a pas tout ça et se base sur un index créé en dur dans une migration, et non dans un modèle.

Oui ça n'existait pas à l'époque dans le django qu'on utilisait je pense (surtout les index custom), mais le lookup est déjà là (https://git.entrouvert.org/entrouvert/authentic/src/branch/main/src/authentic2/utils/lookups.py) il manque la fonction pour l'index. Thomas je peux ajouter ça dans la branche si tu veux.

> J'ai pas toute l'histoire de django et d'authentic sur tout ça. De ma compréhension de Django, il manque en plus du lookup une définition de la fonction (classe ImmutableUnaccent héritant de Func), qu'on peut alors utiliser dans un index. > La solution actuelle d'authentic n'a pas tout ça et se base sur un index créé en dur dans une migration, et non dans un modèle. Oui ça n'existait pas à l'époque dans le django qu'on utilisait je pense (surtout les index custom), mais le lookup est déjà là (https://git.entrouvert.org/entrouvert/authentic/src/branch/main/src/authentic2/utils/lookups.py) il manque la fonction pour l'index. Thomas je peux ajouter ça dans la branche si tu veux.
tnoel commented 2024-03-11 15:38:50 +01:00
Author
Owner
Copy Link

Thomas je peux ajouter ça dans la branche si tu veux.

Tip top cool ; je suis toute ouïe.

> Thomas je peux ajouter ça dans la branche si tu veux. Tip top cool ; je suis toute ouïe.
bdauvergne added 10 commits 2024-03-28 17:26:48 +01:00
gitea/authentic/pipeline/head There was a failure building this commit Details
722f0d9bd9 wip: add index of unaccented upper value of role.name
bdauvergne force-pushed wip/87906-search-role-unaccent from 722f0d9bd9 to 3150bd9884 2024-03-28 17:27:33 +01:00 Compare
bdauvergne force-pushed wip/87906-search-role-unaccent from 3150bd9884 to 48ce80f38a 2024-03-28 17:28:00 +01:00 Compare
bdauvergne force-pushed wip/87906-search-role-unaccent from 48ce80f38a to 12fc4ef9f0 2024-03-28 18:19:47 +01:00 Compare
bdauvergne force-pushed wip/87906-search-role-unaccent from 12fc4ef9f0 to 3af432c6c5 2024-03-28 18:57:45 +01:00 Compare
bdauvergne commented 2024-03-28 18:58:45 +01:00
Owner
Copy Link

Tip top cool ; je suis toute ouïe.

Voilà, mais bon à bien y réfléchir ça ne va rien apporter de grandiose vu qu'on a pas tant de rôles que ça dans la table des rôles. Mais bon c'est sympa juste pour voir comment on fait je pense.

> Tip top cool ; je suis toute ouïe. Voilà, mais bon à bien y réfléchir ça ne va rien apporter de grandiose vu qu'on a pas tant de rôles que ça dans la table des rôles. Mais bon c'est sympa juste pour voir comment on fait je pense.
bdauvergne force-pushed wip/87906-search-role-unaccent from 3af432c6c5 to a73235b076 2024-03-28 23:00:05 +01:00 Compare
bdauvergne commented 2024-03-28 23:06:43 +01:00
Owner
Copy Link

Dernière version, il me manquait encore la définition d'un objet "Transform" enregistré sur TextField et CharField, dans les tests l'index n'est pas utilisé, il n'y a qu'une dizaine de rôle, mais si on rajoute quelques centaines ça le fait (et le gain est quasi nul).

Dernière version, il me manquait encore la définition d'un objet "Transform" enregistré sur TextField et CharField, dans les tests l'index n'est pas utilisé, il n'y a qu'une dizaine de rôle, mais si on rajoute quelques centaines ça le fait (et le gain est quasi nul).
bdauvergne force-pushed wip/87906-search-role-unaccent from a73235b076 to 53a76492e9 2024-03-29 10:09:38 +01:00 Compare
bdauvergne changed title from WIP: manager: search role with unaccent lookup (#87906) to manager: search role with unaccent lookup (#87906) 2024-03-29 10:09:44 +01:00
bdauvergne approved these changes 2024-03-29 20:44:54 +01:00
bdauvergne left a comment
Owner
Copy Link

J'aime beaucoup ce qu'on a fait.

J'aime beaucoup ce qu'on a fait.
tnoel commented 2024-03-29 20:46:46 +01:00
Author
Owner
Copy Link

J'aime beaucoup ce qu'on a fait.

C'est un grand travail d'équipe, je me suis senti tellement utile, c'est fou.

> J'aime beaucoup ce qu'on a fait. C'est un grand travail d'équipe, je me suis senti tellement utile, c'est fou.
bdauvergne force-pushed wip/87906-search-role-unaccent from 53a76492e9 to 67674f56f9 2024-03-29 23:31:16 +01:00 Compare
bdauvergne merged commit 67674f56f9 into main 2024-03-30 08:56:19 +01:00
bdauvergne deleted branch wip/87906-search-role-unaccent 2024-03-30 08:56:20 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
bdauvergne
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/authentic#272
No description provided.
Delete Branch "wip/87906-search-role-unaccent"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?