Add searchable field column to handle full text search #8544

Netacci · 2025-03-04T10:59:37Z

This PR is an alternative approach for handling full-text search using SearchVector.
Changes:
Added a search_vector field to the Commit model:
search_vector = SearchVectorField(null=True, blank=True)

Now when we run a query, instead of creating the SearchVector dynamically, the search now runs against the already populated search_vector field:

filtered_commits = (
                Commit.objects.filter(
                    search_vector=SearchQuery(search_param, config="english"),
                    push__repository=repository,
                )
                .values_list("push_id", flat=True)
                .order_by("-push__time")
                .distinct()[:200]
            )

The search_vector is updated when a new commit is added

   def save(self, *args, **kwargs):
        self.search_vector = SearchVector(
            self.revision, self.author, Substr(self.comments, 1, 100000), config="english"
        )
        super().save(*args, **kwargs)

This can also be done with Postgres Triggers(but I didn't want to tamper with the db) also it's easier to debug if django handles it.

Endpoint: http://localhost:8000/api/project/try/push/?search=1906541
Result: Returns results matching the query across relevant fields such as bug_numbers, summary, author, and revisions.

When I print filtered_commits.query I get

SELECT DISTINCT "commit"."push_id", "push"."time" FROM "commit" INNER JOIN "push" ON ("commit"."push_id" = "push"."id") WHERE ("push"."repository_id" = 4 AND "commit"."search_vector" @@ (plainto_tsquery(english::regconfig, 1906541))) ORDER BY "push"."time" DESC LIMIT 200

and when I run EXPLAIN ANALYZE on the query, I get

 Limit  (cost=75.49..75.50 rows=1 width=12) (actual time=4.219..4.223 rows=1 loops=1)
   ->  Unique  (cost=75.49..75.50 rows=1 width=12) (actual time=4.218..4.221 rows=1 loops=1)
         ->  Sort  (cost=75.49..75.50 rows=1 width=12) (actual time=4.216..4.219 rows=1 loops=1)
               Sort Key: push."time" DESC, commit.push_id
               Sort Method: quicksort  Memory: 25kB
               ->  Hash Join  (cost=23.65..75.48 rows=1 width=12) (actual time=2.337..3.025 rows=1 loops=1)
                     Hash Cond: (commit.push_id = push.id)
                     ->  Bitmap Heap Scan on commit  (cost=16.10..67.89 rows=14 width=4) (actual time=0.171..0.852 rows=5 loops=1)
                           Recheck Cond: (search_vector @@ '''1906541'''::tsquery)
                           Heap Blocks: exact=5
                           ->  Bitmap Index Scan on search_vector_idx  (cost=0.00..16.10 rows=14 width=0) (actual time=0.111..0.112 rows=5 loops=1)
                                 Index Cond: (search_vector @@ '''1906541'''::tsquery)
                     ->  Hash  (cost=7.35..7.35 rows=16 width=12) (actual time=0.796..0.797 rows=16 loops=1)
                           Buckets: 1024  Batches: 1  Memory Usage: 9kB
                           ->  Seq Scan on push  (cost=0.00..7.35 rows=16 width=12) (actual time=0.540..0.575 rows=16 loops=1)
                                 Filter: (repository_id = 4)
                                 Rows Removed by Filter: 252
 Planning Time: 17.443 ms
 Execution Time: 8.398 ms
(19 rows)

Video of http://localhost:8000/api/project/try/push/?search=1906541 results on browser

17.01.2025_07.48.28_REC.mp4

julienw · 2025-03-04T12:23:44Z

treeherder/model/models.py

        ]

+    def save(self, *args, **kwargs):
+        self.search_vector = SearchVector(
+            self.revision, self.author, Substr(self.comments, 1, 100000), config="english"


I have a couple of questions:

would it be possible to add the search_vector column to the push model instead of the commit model? This would save us a join.

I believe we can reduce the value 100000, we are really interested in the first line. Actually I wonder if we could just get the first line (that is, substr to the first "\n")?

I think we would still need a join to access comments if we move up to the push model

julienw · 2025-03-04T12:24:15Z

treeherder/webapp/api/push.py

-                )
-                .filter(
-                    search=SearchQuery(search_param, config="english"),
+                Commit.objects.filter(


we can probably switch to the code that you tried before and that avoids the subquery.

Sorry what code did I try before? you mean the one without the search vector field?

julienw · 2025-03-04T12:24:52Z

treeherder/model/migrations/0038_commit_search_vector_idx.py

+            field=django.contrib.postgres.search.SearchVectorField(
+                blank=True, null=True
+            ),


should we populate it for all existing commits?

Oh yes. I only did that locally by running

from django.contrib.postgres.search import SearchVector from treeherder.model.models import Commit Commit.objects.update( search_vector=SearchVector('comments', 'author', 'revision') )

I'll have to create a migration to update existing data

Archaeopteryx · 2025-03-05T14:45:18Z

treeherder/model/migrations/0038_commit_search_vector_idx.py

@@ -1,8 +1,7 @@
-# Generated by Django 5.1.5 on 2025-02-27 18:06


The migration has been applied, a new migration is needed.

I rolled back to the old migration 0037_bugjob... then deleted my 0038_commit_search.. migration. so when I ran makemigration and another one got generated, I renamed it to 0038_commit_search.. and applied migration. Would that still be a problem?

A migration 0039 landed which depends on the old 0038. Please use a new migration 0040.

Okay got it

I have created a new migration for this

Archaeopteryx · 2025-03-06T14:16:01Z

treeherder/model/migrations/0041_update_search_vector.py

@@ -0,0 +1,20 @@
+# Generated by Django 5.1.5 on 2025-03-05 23:46


Please fold this migration into the 0040 one.

Archaeopteryx · 2025-03-06T18:05:43Z

tests/webapp/api/test_push_api.py

    # Test search by comments
    resp = client.get(
        reverse("push-list", kwargs={"project": test_repository.name}) + "?search=bug"
    )
    assert resp.status_code == 200

    results = resp.json()["results"]
+


debug code to remove

Yea missed that :)

Archaeopteryx · 2025-03-06T18:09:12Z

...commit_search_vector_idx_commit_search_vector_and_more_squashed_0041_update_search_vector.py

+class Migration(migrations.Migration):
+
+    replaces = [
+        ("model", "0040_remove_commit_search_vector_idx_commit_search_vector_and_more"),


These two migrations don't exist (outside your local database if it hasn't been reset). replaces might even break the migration of the ones mentioned here are missing.

Okay will take replaces out

Archaeopteryx · 2025-03-06T18:10:56Z

tests/webapp/api/test_push_api.py

    # Test search by comments
    resp = client.get(
        reverse("push-list", kwargs={"project": test_repository.name}) + "?search=bug"
    )
    assert resp.status_code == 200

    results = resp.json()["results"]
+
+    print(list(Commit.objects.values("revision", "author", "comments", "search_vector")))


Substr for comments should still be necessary.

I've added Substr to the migration

Hi @Archaeopteryx when you have the time, can you review the PR? I've updated the recommended changes and rebased branch

Archaeopteryx · 2025-03-25T09:35:37Z

treeherder/model/migrations/0040_update_search_vector.py

@@ -0,0 +1,39 @@
+# Generated by Django 5.1.5 on 2025-03-06 14:49


This file needs a small rebase, another migration has landed.

…earch vector column and update existing fields

…tr to comments

julienw reviewed Mar 4, 2025

View reviewed changes

Archaeopteryx reviewed Mar 5, 2025

View reviewed changes

Netacci force-pushed the set-full-text-search-using-search-vector branch from 7cbf478 to 7114688 Compare March 6, 2025 10:48

Archaeopteryx reviewed Mar 6, 2025

View reviewed changes

Netacci force-pushed the set-full-text-search-using-search-vector branch from 2f4baa3 to d08e40d Compare March 18, 2025 06:54

Netacci requested a review from Archaeopteryx March 25, 2025 08:49

Archaeopteryx requested changes Mar 25, 2025

View reviewed changes

Netacci added 6 commits March 25, 2025 10:50

create migration for removing previously applied search vector, add s…

d697d6b

…earch vector column and update existing fields

fix tests for search fields

7f50f32

squash migrations

d1a71f0

remove replace from migration, print statement from test and add Subs…

a7895ff

…tr to comments

Retrigger GitHub Actions

b0b8ca2

rebase and update migration

2c3e665

Netacci force-pushed the set-full-text-search-using-search-vector branch from d08e40d to 2c3e665 Compare March 25, 2025 09:59

Archaeopteryx approved these changes Mar 25, 2025

View reviewed changes

Netacci and others added 3 commits March 27, 2025 13:13

update query to search both push and commit models

563e6e8

update test script

ea3f398

Remove debug print statement from push API.

c4fb4fb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add searchable field column to handle full text search #8544

Add searchable field column to handle full text search #8544

Netacci commented Mar 4, 2025

julienw Mar 4, 2025

Netacci Mar 4, 2025

julienw Mar 4, 2025

Netacci Mar 4, 2025

julienw Mar 4, 2025

Netacci Mar 4, 2025 •

edited

Loading

Archaeopteryx Mar 5, 2025

Netacci Mar 5, 2025

Archaeopteryx Mar 5, 2025

Netacci Mar 5, 2025

Netacci Mar 6, 2025

Archaeopteryx Mar 6, 2025

Archaeopteryx Mar 6, 2025

Netacci Mar 6, 2025

Archaeopteryx Mar 6, 2025

Netacci Mar 6, 2025

Archaeopteryx Mar 6, 2025

Netacci Mar 11, 2025

Netacci Mar 18, 2025

Archaeopteryx Mar 25, 2025

		@@ -1,8 +1,7 @@
		# Generated by Django 5.1.5 on 2025-02-27 18:06

		@@ -0,0 +1,20 @@
		# Generated by Django 5.1.5 on 2025-03-05 23:46

		@@ -0,0 +1,39 @@
		# Generated by Django 5.1.5 on 2025-03-06 14:49

Add searchable field column to handle full text search #8544

Are you sure you want to change the base?

Add searchable field column to handle full text search #8544

Conversation

Netacci commented Mar 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Netacci Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Netacci Mar 4, 2025 •

edited

Loading