entrouvert/wcs
entrouvert
/
wcs
14
1
Fork 0
Code Issues Pull Requests 30 Releases Activity

sql: implement new FTS for cards (#86903) #1436

Merged
fpeters merged 1 commits from wip/86903-fts-cards into main 2024-05-02 14:06:21 +02:00
Conversation 6 Commits 1 Files Changed 4 +74 -14
pducroquet commented 2024-04-22 17:49:04 +02:00
Owner
Copy Link

Extension de la nouvelle fonctionnalité FTS introduite précédemment (#1201) pour indexer les fiches en plus des formulaires, et utiliser cette nouvelle rechercher partout où c'est utile.

Extension de la nouvelle fonctionnalité FTS introduite précédemment (https://git.entrouvert.org/entrouvert/wcs/pulls/1201) pour indexer les fiches en plus des formulaires, et utiliser cette nouvelle rechercher partout où c'est utile.
pducroquet added 1 commit 2024-04-22 17:49:05 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
46bace38c1 implement first indexing, and purge, for cards
pducroquet force-pushed wip/86903-fts-cards from 46bace38c1 to a2a1cdf90a 2024-04-22 18:11:53 +02:00 Compare
pducroquet changed target branch from wip/86527-better-fts to main 2024-04-23 07:53:44 +02:00
pducroquet added 1 commit 2024-04-23 07:57:58 +02:00
gitea/wcs/pipeline/head This commit looks good Details
787d7e5dd0 launch migration
pducroquet added 1 commit 2024-04-23 08:32:08 +02:00
gitea/wcs/pipeline/head This commit looks good Details
ec618f21b9 proper triggers for carddef
pducroquet added 1 commit 2024-04-23 08:45:33 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
5cc47c9e1d (TEST) switch all fts to wcs_tsquery
pducroquet added 1 commit 2024-04-23 09:11:17 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
498db54e32 dbg test
pducroquet added 1 commit 2024-04-23 13:15:57 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
5d5901a670 regression found
pducroquet force-pushed wip/86903-fts-cards from 5d5901a670 to fa6c1365e0 2024-04-23 13:23:24 +02:00 Compare
pducroquet added 1 commit 2024-04-23 16:48:23 +02:00
gitea/wcs/pipeline/head This commit looks good Details
f341408f27 drop useless criteria
pducroquet force-pushed wip/86903-fts-cards from f341408f27 to 50ad0d50e3 2024-04-23 17:10:16 +02:00 Compare
pducroquet force-pushed wip/86903-fts-cards from 50ad0d50e3 to 136dca4904 2024-04-29 08:54:56 +02:00 Compare
pducroquet changed title from WIP: implement new FTS for cards to sql: implement new FTS for cards 2024-04-29 09:29:48 +02:00
pducroquet force-pushed wip/86903-fts-cards from 136dca4904 to 391eb190d1 2024-04-29 09:30:27 +02:00 Compare
fpeters requested changes 2024-04-29 10:36:06 +02:00
Dismissed
wcs/sql_criterias.py Outdated
@ -379,4 +378,0 @@
return 'fts @@ plainto_tsquery(%%(c%s)s)' % id(self.value)
class WcsFtsMatch(FtsMatch):
fpeters commented 2024-04-29 10:35:59 +02:00
Owner
Copy Link

Ça m'irait bien d'avoir un feature flag pour contrôler ça, pour débuter, ça donnerait :

class FtsMatch(...):
    def as_sql(self):
         if get_publisher().has_site_option('enable-new-fts'):
              return 'fts @@ wcs_tsquery(%%(c%s)s)' % id(self.value)
         else:
              return 'fts @@ plainto_tsquery(%%(c%s)s)' % id(self.value)

(et comme ça on peut intégrer dès ce cycle, tester, et l'activer en vrai prochain cycle).

Ça m'irait bien d'avoir un feature flag pour contrôler ça, pour débuter, ça donnerait : ``` class FtsMatch(...): def as_sql(self): if get_publisher().has_site_option('enable-new-fts'): return 'fts @@ wcs_tsquery(%%(c%s)s)' % id(self.value) else: return 'fts @@ plainto_tsquery(%%(c%s)s)' % id(self.value) ``` (et comme ça on peut intégrer dès ce cycle, tester, et l'activer en vrai prochain cycle).
pducroquet commented 2024-04-29 11:20:15 +02:00
Author
Owner
Copy Link

J'avais pas en tête les feature flags de wcs, je fais ça

J'avais pas en tête les feature flags de wcs, je fais ça
pducroquet force-pushed wip/86903-fts-cards from 391eb190d1 to 904db585b8 2024-04-29 11:19:51 +02:00 Compare
pducroquet force-pushed wip/86903-fts-cards from 904db585b8 to 1860e6e448 2024-04-29 11:29:00 +02:00 Compare
pducroquet added 1 commit 2024-04-29 15:03:31 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
3c35c68446 try to test wcs_fts
pducroquet added 1 commit 2024-04-29 15:12:05 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
67d9721b09 typo
pducroquet added 1 commit 2024-04-29 15:27:41 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
caa5aa5fcc sigh
pducroquet force-pushed wip/86903-fts-cards from caa5aa5fcc to f39dbebd09 2024-04-29 18:56:03 +02:00 Compare
pducroquet requested review from fpeters 2024-04-30 10:27:55 +02:00
fpeters requested changes 2024-04-30 10:57:59 +02:00
Dismissed
tests/backoffice_pages/test_all.py Outdated
@ -535,0 +535,4 @@
def test_backoffice_listing_fts(pub, wcs_fts):
if wcs_fts:
pub.site_options.set('options', 'enable-new-fts', 'yes')
fpeters commented 2024-04-30 10:56:57 +02:00
Owner
Copy Link

Ajouter une requête où le résultat est différent selon l'activation ou pas ?

Ajouter une requête où le résultat est différent selon l'activation ou pas ?
pducroquet commented 2024-04-30 11:06:47 +02:00
Author
Owner
Copy Link

J'avais ciblé ce test parce que pendant le développement de la fonctionnalité il cassait tant que les triggers ne marchaient pas, mais oui je vais ajouter un cas

J'avais ciblé ce test parce que pendant le développement de la fonctionnalité il cassait tant que les triggers ne marchaient pas, mais oui je vais ajouter un cas
pducroquet added 1 commit 2024-04-30 11:04:51 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
f63ddf97c0 add test that should work only with wcs_fts
pducroquet added 1 commit 2024-04-30 11:11:40 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
be6c396388 fix test, 2 chars is not enough
pducroquet added 1 commit 2024-04-30 11:18:12 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
1d7fce62bf don't choose a character that is removed by plainto_tsquery
pducroquet force-pushed wip/86903-fts-cards from 1d7fce62bf to d70996743c 2024-04-30 11:39:58 +02:00 Compare
fpeters commented 2024-05-02 07:22:19 +02:00
Owner
Copy Link

(le test est en erreur, aussi j'ai créé #1462, sans savoir si ma crainte y était valide).

(le test est en erreur, aussi j'ai créé https://git.entrouvert.org/entrouvert/wcs/pulls/1462, sans savoir si ma crainte y était valide).
fpeters added 3 commits 2024-05-02 12:20:07 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
f6e4af6e1a (try) to check behaviour difference between fts
fpeters commented 2024-05-02 12:24:32 +02:00
Owner
Copy Link

J'ai poussé des commits en plus parce que j'aurais vraiment aimé ça ce cycle :

  • 9f5d2342bf : les feature flags s'activent avec "true" (pas "yes")
  • af49deca0e : suppression du test de comportemnet différent de test_backoffice_listing_fts, j'avais posé mon commentaire là à cause de la fixture mais c'est plus propre de faire un test isolé.
  • f6e4af6e1a : cette tentative de test isolé dans test_sql_criteria_fts, mais elle révèle que j'ai raté quelque chose, l'objectif était "recherche texte libre avec préfixe" mais en ajoutant ce test où j'espérais la différence de comportement, il se révèle que ça ne marche pas ?
J'ai poussé des commits en plus parce que j'aurais vraiment aimé ça ce cycle : * https://git.entrouvert.org/entrouvert/wcs/commit/9f5d2342bffd4c6c0adfcba2f3e270a6ef7f3655 : les feature flags s'activent avec "true" (pas "yes") * https://git.entrouvert.org/entrouvert/wcs/commit/af49deca0e45ea2dc2a8303ae7725740abb1c4b1 : suppression du test de comportemnet différent de test_backoffice_listing_fts, j'avais posé mon commentaire là à cause de la fixture mais c'est plus propre de faire un test isolé. * https://git.entrouvert.org/entrouvert/wcs/commit/f6e4af6e1aa89f7e8c654a36dadefe0c58a47002 : cette tentative de test isolé dans test_sql_criteria_fts, mais elle révèle que j'ai raté quelque chose, l'objectif était "recherche texte libre avec préfixe" mais en ajoutant ce test où j'espérais la différence de comportement, il se révèle que ça ne marche pas ?
fpeters changed title from sql: implement new FTS for cards to sql: implement new FTS for cards (#86903) 2024-05-02 12:25:06 +02:00
fpeters added 1 commit 2024-05-02 12:26:55 +02:00
gitea/wcs/pipeline/head This commit looks good Details
c1c1f5a046 fixup, fts, search with longer prefix
fpeters commented 2024-05-02 12:30:09 +02:00
Owner
Copy Link

mais elle révèle que j'ai raté quelque chose, l'objectif était "recherche texte libre avec préfixe" mais en ajoutant ce test où j'espérais la différence de comportement, il se révèle que ça ne marche pas ?

Ça marche en utilisant un préfixe plus long. Ça reste cependant pas très clair pour moi ce que couvre ou pas cette nouvelle recherche, avoir une explication fonctionnelle qui pourra être partagée/comprise par tout le monde (moi, CPF, clients) sera utile pour l'annonce.

(en attendant si jenkins passe, en réunissant tout ça en un seul commit cette PR pourrait être ok).

> mais elle révèle que j'ai raté quelque chose, l'objectif était "recherche texte libre avec préfixe" mais en ajoutant ce test où j'espérais la différence de comportement, il se révèle que ça ne marche pas ? Ça marche en utilisant un préfixe plus long. Ça reste cependant pas très clair pour moi ce que couvre ou pas cette nouvelle recherche, avoir une explication fonctionnelle qui pourra être partagée/comprise par tout le monde (moi, CPF, clients) sera utile pour l'annonce. (en attendant si jenkins passe, en réunissant tout ça en un seul commit cette PR pourrait être ok).
fpeters added 1 commit 2024-05-02 12:51:51 +02:00
gitea/wcs/pipeline/head This commit looks good Details
e917a87018 fixup extend fts search for more prefixes
fpeters force-pushed wip/86903-fts-cards from e917a87018 to 849ca6a455 2024-05-02 13:35:23 +02:00 Compare
fpeters commented 2024-05-02 13:36:07 +02:00
Owner
Copy Link

Voilà j'ai tout réuni en un seul commit, une fois jenkins ok je mergerai cette PR puis https://dev.entrouvert.org/issues/90237 rebasé.

Voilà j'ai tout réuni en un seul commit, une fois jenkins ok je mergerai cette PR puis https://dev.entrouvert.org/issues/90237 rebasé.
fpeters added 1 commit 2024-05-02 13:45:32 +02:00
gitea/wcs/pipeline/head This commit looks good Details
9e99523ead fixup: limit length of trigger name
fpeters commented 2024-05-02 13:47:11 +02:00
Owner
Copy Link

une fois jenkins ok je mergerai cette PR

Et non :/ j'ai ajouté un nouveau commit 9e99523ead parce qu'avec des tables aux noms un peu long, ajouter "__search_tokens_trg_ins" dépassait les limites, ce qui amenait par exemple :

psycopg2.errors.DuplicateObject: trigger "carddata_139_inscription_atelier_sportif_ad__search_tokens_trg_" for relation "carddata_139_inscription_atelier_sportif_ad" already exists
> une fois jenkins ok je mergerai cette PR Et non :/ j'ai ajouté un nouveau commit https://git.entrouvert.org/entrouvert/wcs/commit/9e99523ead3e517df85282aa620e7efe22526ca6 parce qu'avec des tables aux noms un peu long, ajouter "__search_tokens_trg_ins" dépassait les limites, ce qui amenait par exemple : ``` psycopg2.errors.DuplicateObject: trigger "carddata_139_inscription_atelier_sportif_ad__search_tokens_trg_" for relation "carddata_139_inscription_atelier_sportif_ad" already exists ```
fpeters force-pushed wip/86903-fts-cards from 9e99523ead to 4d2de3db31 2024-05-02 13:53:41 +02:00 Compare
fpeters approved these changes 2024-05-02 14:06:13 +02:00
fpeters left a comment
Owner
Copy Link

Cette fois c'est la bonne, je merge la PR dans la foulée.

Cette fois c'est la bonne, je merge la PR dans la foulée.
fpeters merged commit cb806c6b61 into main 2024-05-02 14:06:21 +02:00
fpeters deleted branch wip/86903-fts-cards 2024-05-02 14:06:21 +02:00
pducroquet commented 2024-05-02 15:40:28 +02:00
Author
Owner
Copy Link

Merci beaucoup

Merci beaucoup
Sign in to join this conversation.
Reviewers
No reviewers
fpeters
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 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#1436
No description provided.
Delete Branch "wip/86903-fts-cards"

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?