sql: new FTS mechanism with fuzzy match #1201

Open
pducroquet wants to merge 3 commits from wip/86527-better-fts into main
Owner

3 parts in this work:

  1. introduce the wcs_search_tokens table, that will store tokens taken from the tsvector columns in wcs_all_forms and searchable_formdefs
  2. maintain that table with triggers and a cron to delete items later on
  3. a new wcs_tsquery function to replace plainto_tsquery and do a fuzzy-match of the requested words agains wcs_search_tokens (with a priority on full match obviously)
3 parts in this work: 1) introduce the wcs_search_tokens table, that will store tokens taken from the tsvector columns in wcs_all_forms and searchable_formdefs 2) maintain that table with triggers and a cron to delete items later on 3) a new `wcs_tsquery` function to replace `plainto_tsquery` and do a fuzzy-match of the requested words agains wcs_search_tokens (with a priority on full match obviously)
pducroquet added 2 commits 2024-02-28 11:31:45 +01:00
fpeters reviewed 2024-02-28 12:02:30 +01:00
wcs/sql.py Outdated
@ -1679,0 +1728,4 @@
$function$;""")
# Second part : insert and update triggers
cur.execute("CREATE OR REPLACE TRIGGER wcs_all_forms_fts_trg_ins AFTER INSERT ON wcs_all_forms FOR EACH ROW WHEN (NEW.fts IS NOT NULL) EXECUTE PROCEDURE wcs_search_tokens_trigger_fn();")
Owner

J'ai bien noté que c'était encore WIP mais point d'attention quand même, le ticket en question (/api/formdefs) concerne les démarches (formdef, indexées via SearchableFormDef), pas les demandes (formdata).

Mais il sera bien d'avoir quelque chose de générique, qui pourra s'appliquer à l'ensemble des demandes (wcs_all_forms) mais aussi sur les tables de demandes par formulaire (formdata_*) et les tables des fiches (carddata_*).

J'ai bien noté que c'était encore WIP mais point d'attention quand même, le ticket en question (/api/formdefs) concerne les démarches (formdef, indexées via SearchableFormDef), pas les demandes (formdata). Mais il sera bien d'avoir quelque chose de générique, qui pourra s'appliquer à l'ensemble des demandes (wcs_all_forms) mais aussi sur les tables de demandes par formulaire (`formdata_*`) et les tables des fiches (`carddata_*`).
Author
Owner

Merci pour ce point, c'est bien noté.
C'était beaucoup plus simple pour moi de commencer par les demandes que par les démarches. La grosse partie est surtout dans le commit 3, et elle est indépendante. Une fois que cette base fonctionnera, j'y ajouterai les démarches.

Merci pour ce point, c'est bien noté. C'était beaucoup plus simple pour moi de commencer par les demandes que par les démarches. La grosse partie est surtout dans le commit 3, et elle est indépendante. Une fois que cette base fonctionnera, j'y ajouterai les démarches.
pducroquet marked this conversation as resolved
pducroquet added 1 commit 2024-02-28 15:33:35 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
2c328847ee
wcs_search_tokens: last part, search function using our tokens (#86527)
pducroquet added 1 commit 2024-02-28 15:43:17 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
c84d2c2e65
don't rely on PG14
pducroquet force-pushed wip/86527-better-fts from c84d2c2e65 to 768174fb85 2024-02-28 15:53:23 +01:00 Compare
pducroquet added 1 commit 2024-02-28 16:16:33 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
04d87f5259
fix migration
pducroquet added 1 commit 2024-02-28 16:38:50 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
11f0998225
add missing extension
pducroquet added 1 commit 2024-02-28 17:13:16 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
8bf2e33553
crude id and phone protection
pducroquet added 1 commit 2024-02-28 17:28:42 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
8a7301f810
well test something else now...
pducroquet force-pushed wip/86527-better-fts from 8a7301f810 to d40f540a00 2024-02-28 17:38:20 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from d40f540a00 to 0807aaa1ca 2024-02-28 17:46:23 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from 0807aaa1ca to d3a1dd3a6f 2024-02-28 17:52:48 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from d3a1dd3a6f to b6fbb639ec 2024-02-28 17:59:05 +01:00 Compare
pducroquet added 1 commit 2024-02-29 09:11:37 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
4b9acaa7c1
also index SearchableFormDef
pducroquet added 1 commit 2024-02-29 12:46:52 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
1bdea59526
redo search tokens creation
pducroquet added 1 commit 2024-02-29 12:56:05 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
b7fdbd4c32
add another safety
pducroquet force-pushed wip/86527-better-fts from b7fdbd4c32 to 01f74d5c4b 2024-02-29 13:11:25 +01:00 Compare
pducroquet added 1 commit 2024-02-29 13:19:20 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
aa33052887
fix placement of cur.close
pducroquet added 1 commit 2024-02-29 13:39:11 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
e485210e23
fix placement of migration in publisher.py
pducroquet added 1 commit 2024-03-04 09:05:27 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
2615fcc7bd
try to improve dependencies in SQL migrations
pducroquet added 1 commit 2024-03-04 09:12:22 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
4e228fb3ed
further protect, work aroung PG13 missing OR REPLACE for triggers
pducroquet added 1 commit 2024-03-04 10:25:49 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
eb175c397e
so simple fix
pducroquet added 1 commit 2024-03-04 10:44:08 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
0627f6274c
Move wcs_tsquery to a specific criteria
pducroquet added 1 commit 2024-03-04 10:50:30 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
3378c21e1a
fix
pducroquet added 1 commit 2024-03-04 10:56:29 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
fcfbf36450
fix
pducroquet added 1 commit 2024-03-04 11:07:10 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
3a9e0b21e5
further fix migration
pducroquet added 1 commit 2024-03-04 11:19:56 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
8f663490c6
fix long lines
pducroquet added 1 commit 2024-03-04 11:28:09 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
88c718424b
pre-commit
pducroquet force-pushed wip/86527-better-fts from 88c718424b to f80aa275b5 2024-03-04 11:36:02 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from f80aa275b5 to 5ac8da9b4e 2024-03-04 11:46:22 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from 5ac8da9b4e to 900ebf1d06 2024-03-04 12:19:43 +01:00 Compare
pducroquet changed title from WIP: wip/86527-better-fts to sql: new FTS mechanism with fuzzy match 2024-03-04 12:21:32 +01:00
fpeters requested changes 2024-03-04 19:12:30 +01:00
Dismissed
fpeters left a comment
Owner

Il faudrait quand même un peu de tests pour montrer ce que ça change / que ça marche mieux.

Il faudrait quand même un peu de tests pour montrer ce que ça change / que ça marche mieux.
pducroquet added 1 commit 2024-03-05 10:17:52 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
7d9908b00f
break CI on purpose.
pducroquet added 2 commits 2024-03-05 10:28:45 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
ac991e24c0
broken on purpose
pducroquet added 1 commit 2024-03-05 10:33:10 +01:00
gitea/wcs/pipeline/head This commit looks good Details
a9f810689b
and now unbroken
pducroquet force-pushed wip/86527-better-fts from a9f810689b to 17ac728f78 2024-03-05 10:41:39 +01:00 Compare
pducroquet added 1 commit 2024-03-05 10:42:20 +01:00
gitea/wcs/pipeline/head This commit looks good Details
f9c2866ba9
wcs_search_tokens: new FTS mechanism with fuzzy-match (#86527)
introduce a new mechanism to implement FTS with fuzzy-match.
This is made possible by adding and maintaining a table of the
FTS tokens, wcs_search_tokens, fed with searchable_formdefs
and wcs_all_forms.
When a query is issued, its tokens are matched against the
tokens with a fuzzy match when no direct match is found, and
the query is then rebuilt.
Author
Owner

Il faudrait quand même un peu de tests pour montrer ce que ça change / que ça marche mieux.

J'ai intégré un test pour SearchableFormdef déjà (en reconstruisant l'historique pour qu'on voit bien le test arriver en échec, puis le reste du code qui corrige l'échec).
Ça correspond donc au test des triggers et de la fonction de recherche. Je dois voir encore pour tester la méthode du cron

> Il faudrait quand même un peu de tests pour montrer ce que ça change / que ça marche mieux. J'ai intégré un test pour SearchableFormdef déjà (en reconstruisant l'historique pour qu'on voit bien le test arriver en échec, puis le reste du code qui corrige l'échec). Ça correspond donc au test des triggers et de la fonction de recherche. Je dois voir encore pour tester la méthode du cron
pducroquet added 1 commit 2024-03-05 11:14:09 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
04975493ac
first broken test for purge of search tokens
pducroquet added 1 commit 2024-03-05 11:21:25 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
7278029828
progress on test
pducroquet added 1 commit 2024-03-05 11:27:11 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
12413e78d3
progress (and speed up for now)
pducroquet added 1 commit 2024-03-05 11:29:26 +01:00
gitea/wcs/pipeline/head There was a failure building this commit Details
45a7ccf97b
progress
pducroquet force-pushed wip/86527-better-fts from 45a7ccf97b to fc6e252770 2024-03-05 11:34:44 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from fc6e252770 to 627f5a6f12 2024-03-05 11:41:03 +01:00 Compare
Author
Owner

Et maintenant avec en plus un test de la purge et de l'indexation des formdata.

Et maintenant avec en plus un test de la purge et de l'indexation des formdata.
pducroquet requested review from fpeters 2024-03-05 11:49:36 +01:00
tnoel reviewed 2024-03-07 23:13:35 +01:00
@ -703,2 +708,4 @@
CronJob(cls.clean_loggederrors, hours=[3], minutes=[0], name='clean_loggederrors')
)
cls.register_cronjob(
CronJob(cls.clean_search_tokens, weekdays=[0], hours=[1], minutes=[0], name='clean_tokens')
Owner

name='clean_search_tokens' donnera un peu de cohérence.

name='clean_search_tokens' donnera un peu de cohérence.
Author
Owner

Corrigé, merci

Corrigé, merci
pducroquet marked this conversation as resolved
pducroquet force-pushed wip/86527-better-fts from 627f5a6f12 to 56fb3fb2fc 2024-03-14 16:34:40 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from 56fb3fb2fc to 0843793ac3 2024-03-15 11:08:09 +01:00 Compare
fpeters reviewed 2024-03-15 11:41:16 +01:00
wcs/sql.py Outdated
@ -97,2 +97,4 @@
def _table_exists(cur, table_name):
cur.execute('SELECT 1 FROM pg_class WHERE relname = %s;', (table_name,))
Owner

Je profite de l'occasion pour demander le sens des point-virgules en fin de ligne; ça n'est pas quelque chose que je mettais, est-ce important ?

Je profite de l'occasion pour demander le sens des point-virgules en fin de ligne; ça n'est pas quelque chose que je mettais, est-ce important ?
Author
Owner

Vieille habitude de ma part, ça ne change rien normalement, je peux le virer si tu préfères.

Vieille habitude de ma part, ça ne change rien normalement, je peux le virer si tu préfères.
Owner

Non ok pour moi, j'étais juste curieux de peut-être un effet.

Non ok pour moi, j'étais juste curieux de peut-être un effet.
pducroquet marked this conversation as resolved
wcs/sql.py Outdated
@ -1679,3 +1696,4 @@
cur.close()
def init_search_tokens(conn=None, cur=None):
Owner

Ça m'irait bien d'avoir ici un récapitulatif du fonctionnement, les tables/triggers/fonctions créées, comment les choses s'agencent et pour quel résultat.

Ça m'irait bien d'avoir ici un récapitulatif du fonctionnement, les tables/triggers/fonctions créées, comment les choses s'agencent et pour quel résultat.
Author
Owner

Fait, je te laisse juger, avec la tête dans le guidon c'est plus dur de réaliser si la documentation est complète ou non.

Fait, je te laisse juger, avec la tête dans le guidon c'est plus dur de réaliser si la documentation est complète ou non.
wcs/sql.py Outdated
@ -1682,0 +1730,4 @@
tokenized as (select unnest(regexp_split_to_array($1, '\s+')) w),
super_tokenized as (
select w,
coalesce((select plainto_tsquery(perfect.token) from wcs_search_tokens perfect where perfect.token = plainto_tsquery(w)::text),
Owner

Je suis toujours perdu par la lecture ici, la forme raccourcie pour renommer la table, ça marcherait pareil (je pense) et plus clair pour moi si c'était wcs_search_tokens AS perfect.

Peut-être plus généralement décrire la requête, et ça rejoint peut-être le point plus haut, si je comprends bien c'est "perfect" pour "perfect match" (ce que je capte en voyant plus bas "partial"), mais ça ne m'éclaire pas totalement sur ce qui est fait.

Je suis toujours perdu par la lecture ici, la forme raccourcie pour renommer la table, ça marcherait pareil (je pense) et plus clair pour moi si c'était wcs_search_tokens AS perfect. Peut-être plus généralement décrire la requête, et ça rejoint peut-être le point plus haut, si je comprends bien c'est "perfect" pour "perfect match" (ce que je capte en voyant plus bas "partial"), mais ça ne m'éclaire pas totalement sur ce qui est fait.
Author
Owner

Fait, idem, n'hésite pas si y'a besoin de plus d'éléments.

Fait, idem, n'hésite pas si y'a besoin de plus d'éléments.
wcs/sql.py Outdated
@ -5121,3 +5281,3 @@
# programmaticaly but will make sure git conflicts if two migrations are
# separately added with the same number)
SQL_LEVEL = (106, 'add context column to logged_errors table')
SQL_LEVEL = (107, 'improved fts method')
Owner

j'aime bien garder le même commentaire ici et plus bas dans le migrate(), donc plutôt "new fts mechanism with tokens table". (et numéro à incrémenter).

j'aime bien garder le même commentaire ici et plus bas dans le migrate(), donc plutôt "new fts mechanism with tokens table". (et numéro à incrémenter).
pducroquet marked this conversation as resolved
pducroquet force-pushed wip/86527-better-fts from 0843793ac3 to 171ade9840 2024-03-19 09:55:14 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from 171ade9840 to af28be9910 2024-03-19 18:23:07 +01:00 Compare
fpeters requested changes 2024-03-26 18:04:56 +01:00
Dismissed
wcs/sql.py Outdated
@ -1682,0 +1740,4 @@
# And last: functions to use this brand new table
# These two aggregates make the search query far simpler to write
cur.execute('CREATE OR REPLACE AGGREGATE tsquery_agg_or (tsquery) (sfunc=tsquery_or, stype=tsquery);')
cur.execute('CREATE OR REPLACE AGGREGATE tsquery_agg_and (tsquery) (sfunc=tsquery_and, stype=tsquery);')
Owner

Je crois totalement que ça rend l'affaire plus simple à écrire mais je ne trouve pas de référence à tsquery_or dans la documentation de postgresql. Peut-être que ça serait utile d'expliciter ici en commentaire ce que font tsquery_agg_or et tsquery_agg_and.

Je crois totalement que ça rend l'affaire plus simple à écrire mais je ne trouve pas de référence à tsquery_or dans la documentation de postgresql. Peut-être que ça serait utile d'expliciter ici en commentaire ce que font tsquery_agg_or et tsquery_agg_and.
Author
Owner

Documentation améliorée avec info sur l'origine des fonctions dans PG, dis moi si c'est mieux

Documentation améliorée avec info sur l'origine des fonctions dans PG, dis moi si c'est mieux
wcs/sql.py Outdated
@ -1682,0 +1758,4 @@
tsquery_agg_or(plainto_tsquery(partial.token) order by partial.token <-> w desc),
plainto_tsquery(w)) tokens
FROM tokenized
LEFT JOIN wcs_search_tokens AS partial ON partial.token % w AND w not similar to '%[0-9]{2,}%'
Owner

Ok, donc sur un formdef ("Intervention du service hygiène, salubrité et environnement lorem ipsum plop plop plop") qui a été indexé avec :

'environ':7A 'hygien':4A 'intervent':1A 'ipsum':9A 'lorem':8A 'plop':10A,11A,12A 'salubrit':5A 'servic':3A

Il y a un match sur hien j'imagine parce que hygien, parce que match "partial", qui se voit ainsi :

# select * from wcs_search_tokens WHERE token % 'hien';
 token
--------
 chien
 hygien

Je n'ai pas trouvé de description de l'opérateur % dans la documentation de postgresql. (pas le caractère le plus facile à chercher, certes).


Sinon dans le code je n'arrive pas à suivre ce qu'est w, je n'arrive même pas à dire si c'est tout le temps la même chose. J'encouragerais à allonger un peu le nom de cette variable, par exemple si c'était "token" tout le temps (si ça a du sens) ça pourrait être plus clair, modulo confusion avec le nom de la colonne, donc allonger encore, searched_token.


À reprendre pour essayer de comprendre comment tout ça fonctionne, je n'explique pas ceci, toujours ma démarche "Intervention du service hygiène, salubrité et environnement ..."

 select * from wcs_search_tokens where token like 'salub%';
  token
----------
 salubrit

mais

select wcs_tsquery('salubrite');
 wcs_tsquery
-------------
 'salubr'

vs

select plainto_tsquery('salubrite');
 plainto_tsquery
-----------------
 'salubrit'

(l'indexation et la recherche lancée depuis wcs ramènent à de l'ascii, cf FtsMatch.get_fts_value())

En suivant, ça semble venir du dernier cas ("otherwise..."), plainto_tsquery(w), qui serait appelé avec "salubrit", ce qui le raccourcit à nouveau et tombe sur

auquo_local=# select wcs_tsquery('salubrit');
   wcs_tsquery
------------------
 'sal' | 'salubr'

Je ne sais que faire de tout ça mais j'espère que ça peut t'aider à voir quelque chose.

Ok, donc sur un formdef ("Intervention du service hygiène, salubrité et environnement lorem ipsum plop plop plop") qui a été indexé avec : > 'environ':7A 'hygien':4A 'intervent':1A 'ipsum':9A 'lorem':8A 'plop':10A,11A,12A 'salubrit':5A 'servic':3A Il y a un match sur hien j'imagine parce que hygien, parce que match "partial", qui se voit ainsi : ``` # select * from wcs_search_tokens WHERE token % 'hien'; token -------- chien hygien ``` Je n'ai pas trouvé de description de l'opérateur % dans la documentation de postgresql. (pas le caractère le plus facile à chercher, certes). ---- Sinon dans le code je n'arrive pas à suivre ce qu'est w, je n'arrive même pas à dire si c'est tout le temps la même chose. J'encouragerais à allonger un peu le nom de cette variable, par exemple si c'était "token" tout le temps (si ça a du sens) ça pourrait être plus clair, modulo confusion avec le nom de la colonne, donc allonger encore, searched_token. ---- À reprendre pour essayer de comprendre comment tout ça fonctionne, je n'explique pas ceci, toujours ma démarche "Intervention du service hygiène, salubrité et environnement ..." ``` select * from wcs_search_tokens where token like 'salub%'; token ---------- salubrit ``` mais ``` select wcs_tsquery('salubrite'); wcs_tsquery ------------- 'salubr' ``` vs ``` select plainto_tsquery('salubrite'); plainto_tsquery ----------------- 'salubrit' ``` (l'indexation et la recherche lancée depuis wcs ramènent à de l'ascii, cf FtsMatch.get_fts_value()) En suivant, ça semble venir du dernier cas ("otherwise..."), plainto_tsquery(w), qui serait appelé avec "salubrit", ce qui le raccourcit à nouveau et tombe sur ``` auquo_local=# select wcs_tsquery('salubrit'); wcs_tsquery ------------------ 'sal' | 'salubr' ``` Je ne sais que faire de tout ça mais j'espère que ça peut t'aider à voir quelque chose.
Author
Owner

J'ai corrigé le w en word dans la requête, pour ne pas confondre avec un token de recherche justement (c'est avant la transformation finalement), et supprimé un order by inutile en l'état. J'ai ajouté un lien vers la documentation de pgtrgm pour l'opérateur %.

Concernant ton problème, je n'arrive pas à le reproduire, je cherche

J'ai corrigé le w en word dans la requête, pour ne pas confondre avec un token de recherche justement (c'est avant la transformation finalement), et supprimé un order by inutile en l'état. J'ai ajouté un lien vers la documentation de pgtrgm pour l'opérateur %. Concernant ton problème, je n'arrive pas à le reproduire, je cherche
wcs/sql.py Outdated
@ -1682,0 +1798,4 @@
):
# Second part: insert and update triggers for wcs_all_forms
cur.execute(
"""CREATE TRIGGER wcs_all_forms_fts_trg_ins
Owner

À regarder le commit dans mon terminal je vois qu'il y a des lignes qui comme celle-ci se terminent par un caractère espace; il faudrait les retirer. (j'imagine qu'on pourrait trouver un hook pre-commit pour assurer ça).

À regarder le commit dans mon terminal je vois qu'il y a des lignes qui comme celle-ci se terminent par un caractère espace; il faudrait les retirer. (j'imagine qu'on pourrait trouver un hook pre-commit pour assurer ça).
pducroquet marked this conversation as resolved
pducroquet force-pushed wip/86527-better-fts from af28be9910 to ac48ebb70d 2024-03-27 17:03:28 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from ac48ebb70d to fcdc2581ca 2024-03-27 17:06:43 +01:00 Compare
pducroquet force-pushed wip/86527-better-fts from fcdc2581ca to 5cdb00e496 2024-03-27 17:21:19 +01:00 Compare
fpeters requested changes 2024-03-29 14:12:53 +01:00
Dismissed
fpeters left a comment
Owner

Un truc a du changé, la recherche sur "tes" ne trouve plus :

        resp = get_url('/api/formdefs/?backoffice-submission=on&q=test')
        assert len(resp.json['data']) == 2
    
        resp = get_url('/api/formdefs/?backoffice-submission=on&q=tes')
>       assert len(resp.json['data']) == 2
Un truc a du changé, la recherche sur "tes" ne trouve plus : ``` resp = get_url('/api/formdefs/?backoffice-submission=on&q=test') assert len(resp.json['data']) == 2 resp = get_url('/api/formdefs/?backoffice-submission=on&q=tes') > assert len(resp.json['data']) == 2 ```
fpeters dismissed fpeters’s review 2024-03-29 14:14:32 +01:00
Reason:

(je notais juste l'erreur jenkins, je n'ai pas encore regardé les changements)

pducroquet force-pushed wip/86527-better-fts from 5cdb00e496 to 614ff32a23 2024-04-09 16:02:52 +02:00 Compare
pducroquet added 1 commit 2024-04-09 16:11:57 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
6bc9588111
should fix the test
pducroquet force-pushed wip/86527-better-fts from 6bc9588111 to 7baeb009d4 2024-04-09 16:17:48 +02:00 Compare
pducroquet force-pushed wip/86527-better-fts from 7baeb009d4 to 89a165c975 2024-04-09 16:39:02 +02:00 Compare
pducroquet added 1 commit 2024-04-09 16:51:49 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
ec0ca087a6
investigate
pducroquet added 1 commit 2024-04-09 17:04:25 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
7f535cdfbe
investigate
pducroquet added 1 commit 2024-04-09 17:12:56 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
8c03da88d6
investigating...
pducroquet added 1 commit 2024-04-09 17:21:43 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
ff77d5f873
oops...
pducroquet added 1 commit 2024-04-09 17:35:40 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
ad6044cf60
trying to figure it out
pducroquet added 1 commit 2024-04-09 18:08:44 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
2bc52f9ccd
switch comparison to simple fts configuration
pducroquet force-pushed wip/86527-better-fts from 2bc52f9ccd to 4bd581c32b 2024-04-09 18:15:25 +02:00 Compare
Author
Owner

Un truc a du changé, la recherche sur "tes" ne trouve plus :

        resp = get_url('/api/formdefs/?backoffice-submission=on&q=test')
        assert len(resp.json['data']) == 2
    
        resp = get_url('/api/formdefs/?backoffice-submission=on&q=tes')
>       assert len(resp.json['data']) == 2

Il m'a fallu beaucoup de temps pour trouver ce qui causait cette régression pour jenkins et pas en local. Ce test est malheureux puisque "tes" est considéré comme un stop-word sur le dictionnaire français de postgresql. Donc un plainto_tsquery('tes') renvoie '', j'ai corrigé en passant sur le dictionnaire 'simple' pour la recherche de similarités.

> Un truc a du changé, la recherche sur "tes" ne trouve plus : > > ``` > resp = get_url('/api/formdefs/?backoffice-submission=on&q=test') > assert len(resp.json['data']) == 2 > > resp = get_url('/api/formdefs/?backoffice-submission=on&q=tes') > > assert len(resp.json['data']) == 2 > ``` Il m'a fallu beaucoup de temps pour trouver ce qui causait cette régression pour jenkins et pas en local. Ce test est malheureux puisque "tes" est considéré comme un stop-word sur le dictionnaire français de postgresql. Donc un plainto_tsquery('tes') renvoie '', j'ai corrigé en passant sur le dictionnaire 'simple' pour la recherche de similarités.
fpeters added 1 commit 2024-04-14 11:19:53 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
f3d8949e85
tests: salubrité
fpeters requested changes 2024-04-14 11:31:07 +02:00
Dismissed
fpeters left a comment
Owner

Concernant ton problème, je n'arrive pas à le reproduire, je cherche

Je viens de mettre à jour la branche locale et je tombe toujours sur le même comportement; alors qu'il y a un token "salubrit" dans wcs_search_tokens, select wcs_tsquery('salubrite'); retourne 'salubr', ce qui fait qu'une recherche de "salubrité" ne trouve pas le formdef qui a salubrité dans son nom (puisque dans sa colonne fts de searchable_formdefs c'est 'salubrit' qui s'y trouve).

Je ne sais pas ce qui pourrait aider dans comme description, pour reproduire, mais je viens d'ajouter un commit dans la branche, qui échoue aussi sur jenkins. (et qui fonctionne appliqué sur main).

> Concernant ton problème, je n'arrive pas à le reproduire, je cherche Je viens de mettre à jour la branche locale et je tombe toujours sur le même comportement; alors qu'il y a un token "salubrit" dans wcs_search_tokens, `select wcs_tsquery('salubrite');` retourne 'salubr', ce qui fait qu'une recherche de "salubrité" ne trouve pas le formdef qui a salubrité dans son nom (puisque dans sa colonne fts de searchable_formdefs c'est 'salubrit' qui s'y trouve). Je ne sais pas ce qui pourrait aider dans comme description, pour reproduire, mais je viens d'ajouter un commit dans la branche, qui échoue aussi sur jenkins. (et qui fonctionne appliqué sur main).
pducroquet added 1 commit 2024-04-16 13:31:55 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
0263dfda62
dump data to understand the issue
pducroquet added 1 commit 2024-04-16 17:30:18 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
aa2c7b3fe8
got a lead
Author
Owner

Concernant ton problème, je n'arrive pas à le reproduire, je cherche

Je viens de mettre à jour la branche locale et je tombe toujours sur le même comportement; alors qu'il y a un token "salubrit" dans wcs_search_tokens, select wcs_tsquery('salubrite'); retourne 'salubr', ce qui fait qu'une recherche de "salubrité" ne trouve pas le formdef qui a salubrité dans son nom (puisque dans sa colonne fts de searchable_formdefs c'est 'salubrit' qui s'y trouve).

Je ne sais pas ce qui pourrait aider dans comme description, pour reproduire, mais je viens d'ajouter un commit dans la branche, qui échoue aussi sur jenkins. (et qui fonctionne appliqué sur main).

Merci bien, en fait le problème est bien plus profond que ça, le code actuel casse complètement la mécanique de FTS donc je vais voir pour implémenter le même fonctionnement que l'existant, mais je ferai un autre ticket pour détailler le problème. En très court, le code actuel casse FTS en retirant tous les accents car le dictionnaire intégré au FTS s'effondre...

Exemple rapide et idiot : avec FTS, écolier et écolière sont le même mot normalement, mais sans les accents...

test=> select to_tsvector('french', 'ecolier'), to_tsvector('french', 'ecoliere');
 to_tsvector | to_tsvector 
-------------+-------------
 'ecoli':1   | 'ecolier':1
(1 row)

test=> select to_tsvector('french', 'écolier'), to_tsvector('french', 'écolière');
 to_tsvector | to_tsvector 
-------------+-------------
 'écoli':1   | 'écoli':1
(1 row)
> > Concernant ton problème, je n'arrive pas à le reproduire, je cherche > > Je viens de mettre à jour la branche locale et je tombe toujours sur le même comportement; alors qu'il y a un token "salubrit" dans wcs_search_tokens, `select wcs_tsquery('salubrite');` retourne 'salubr', ce qui fait qu'une recherche de "salubrité" ne trouve pas le formdef qui a salubrité dans son nom (puisque dans sa colonne fts de searchable_formdefs c'est 'salubrit' qui s'y trouve). > > Je ne sais pas ce qui pourrait aider dans comme description, pour reproduire, mais je viens d'ajouter un commit dans la branche, qui échoue aussi sur jenkins. (et qui fonctionne appliqué sur main). Merci bien, en fait le problème est bien plus profond que ça, le code actuel casse complètement la mécanique de FTS donc je vais voir pour implémenter le même fonctionnement que l'existant, mais je ferai un autre ticket pour détailler le problème. En très court, le code actuel casse FTS en retirant tous les accents car le dictionnaire intégré au FTS s'effondre... Exemple rapide et idiot : avec FTS, écolier et écolière sont le même mot normalement, mais sans les accents... ``` test=> select to_tsvector('french', 'ecolier'), to_tsvector('french', 'ecoliere'); to_tsvector | to_tsvector -------------+------------- 'ecoli':1 | 'ecolier':1 (1 row) test=> select to_tsvector('french', 'écolier'), to_tsvector('french', 'écolière'); to_tsvector | to_tsvector -------------+------------- 'écoli':1 | 'écoli':1 (1 row) ```
pducroquet force-pushed wip/86527-better-fts from aa2c7b3fe8 to 71a1489c88 2024-04-17 11:06:16 +02:00 Compare
Author
Owner

J'ai mis à jour la branche en incorporant ton test, qui me semble important à garder pour l'avenir, directement dans les tests du FTS.
Dans mon développement, je partais du principe que plainto_tsquery(plainto_tsquery('bidule')::text) était égal à plainto_tsquery('bidule'), ce qui n'est pas le cas quand on sort du dictionnaire avec le unaccent.
Donc je n'ai eu qu'à retirer cette supposition et la remplacer par un cast direct pour régler le problème.

J'ai mis à jour la branche en incorporant ton test, qui me semble important à garder pour l'avenir, directement dans les tests du FTS. Dans mon développement, je partais du principe que plainto_tsquery(plainto_tsquery('bidule')::text) était égal à plainto_tsquery('bidule'), ce qui n'est pas le cas quand on sort du dictionnaire avec le unaccent. Donc je n'ai eu qu'à retirer cette supposition et la remplacer par un cast direct pour régler le problème.
pducroquet requested review from fpeters 2024-04-17 11:12:31 +02:00
pducroquet force-pushed wip/86527-better-fts from 71a1489c88 to d5036ed141 2024-04-22 11:55:05 +02:00 Compare
pducroquet added 1 commit 2024-04-22 12:11:17 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
02b9ecf665
dump more test data for debug
pducroquet force-pushed wip/86527-better-fts from 02b9ecf665 to 6b0108f527 2024-04-22 13:18:15 +02:00 Compare
pducroquet force-pushed wip/86527-better-fts from 6b0108f527 to 9d8efbda51 2024-04-22 13:26:15 +02:00 Compare
pducroquet added 1 commit 2024-04-22 13:34:50 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
180f07752e
fix test following better purge of forms
pducroquet added 1 commit 2024-04-22 13:55:33 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
5d8ceee958
rework the test for better behaviour along other tests
pducroquet force-pushed wip/86527-better-fts from 5d8ceee958 to 4c2278fdaa 2024-04-22 14:02:06 +02:00 Compare
pducroquet force-pushed wip/86527-better-fts from 4c2278fdaa to 7dc8fe49b7 2024-04-22 14:07:56 +02:00 Compare
pducroquet force-pushed wip/86527-better-fts from 7dc8fe49b7 to 817875d6cb 2024-04-22 14:46:55 +02:00 Compare
pducroquet force-pushed wip/86527-better-fts from 817875d6cb to aec7181a78 2024-04-23 13:22:37 +02:00 Compare
fpeters approved these changes 2024-04-26 11:12:53 +02:00
fpeters left a comment
Owner

Passons ça ainsi.

Passons ça ainsi.
All checks were successful
gitea/wcs/pipeline/head This commit looks good
This pull request has changes conflicting with the target branch.
  • wcs/sql.py
Sign in to join this conversation.
No reviewers
No Label
No Milestone
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/wcs#1201
No description provided.