From 75f141d242872b6ac6d5e0733302f1c09bb8b718 Mon Sep 17 00:00:00 2001 From: yushen Date: Fri, 10 Apr 2026 14:19:48 +0800 Subject: [PATCH] fix(search): lowercase pattern in Go, replace ILIKE with LOWER(col) LIKE - Lowercase phrase/terms in Go before building SQL, eliminating redundant LOWER() on the pattern side - Replace all ILIKE with LOWER(column) LIKE for pg_bigm 1.2 GIN index compat - Add migration 036 to rebuild bigm indexes on LOWER() expressions - Add unit tests for buildSearchQuery asserting SQL shape Co-Authored-By: Claude Opus 4.6 (1M context) --- server/internal/handler/issue.go | 58 ++++++----- server/internal/handler/search_test.go | 95 +++++++++++++++++++ .../036_search_index_lower.down.sql | 14 +++ .../migrations/036_search_index_lower.up.sql | 17 ++++ 4 files changed, 158 insertions(+), 26 deletions(-) create mode 100644 server/internal/handler/search_test.go create mode 100644 server/migrations/036_search_index_lower.down.sql create mode 100644 server/migrations/036_search_index_lower.up.sql diff --git a/server/internal/handler/issue.go b/server/internal/handler/issue.go index 82bbb7f56..2648199fa 100644 --- a/server/internal/handler/issue.go +++ b/server/internal/handler/issue.go @@ -222,9 +222,15 @@ type searchResult struct { } // buildSearchQuery builds a dynamic SQL query for issue search. -// It supports ILIKE matching, identifier search, multi-word search, -// and refined ranking. +// It uses LOWER(column) LIKE for case-insensitive matching compatible with pg_bigm 1.2 GIN indexes. +// Search patterns are lowercased in Go to avoid redundant LOWER() on the pattern side in SQL. func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, includeClosed bool) (string, []any) { + // Lowercase in Go so SQL only needs LOWER() on the column side. + phrase = strings.ToLower(phrase) + for i, t := range terms { + terms[i] = strings.ToLower(t) + } + // Parameter index tracker argIdx := 1 args := []any{} @@ -242,7 +248,7 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, wsParam := nextArg(nil) // $2 — workspace_id, will be filled by caller position - // Build per-term ILIKE conditions only for multi-word search. + // Build per-term LIKE conditions only for multi-word search. // For single-word queries, the phrase parameter already covers the term. var termParams []string if len(terms) > 1 { @@ -257,7 +263,7 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, // Full phrase match: title, description, or comment phraseMatch := fmt.Sprintf( - "(i.title ILIKE %s OR COALESCE(i.description, '') ILIKE %s OR EXISTS (SELECT 1 FROM comment c WHERE c.issue_id = i.id AND c.content ILIKE %s))", + "(LOWER(i.title) LIKE %s OR LOWER(COALESCE(i.description, '')) LIKE %s OR EXISTS (SELECT 1 FROM comment c WHERE c.issue_id = i.id AND LOWER(c.content) LIKE %s))", phraseContains, phraseContains, phraseContains, ) whereParts = append(whereParts, phraseMatch) @@ -268,7 +274,7 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, for _, tp := range termParams { tc := "'%' || " + tp + " || '%'" termConditions = append(termConditions, fmt.Sprintf( - "(i.title ILIKE %s OR COALESCE(i.description, '') ILIKE %s OR EXISTS (SELECT 1 FROM comment c WHERE c.issue_id = i.id AND c.content ILIKE %s))", + "(LOWER(i.title) LIKE %s OR LOWER(COALESCE(i.description, '')) LIKE %s OR EXISTS (SELECT 1 FROM comment c WHERE c.issue_id = i.id AND LOWER(c.content) LIKE %s))", tc, tc, tc, )) } @@ -298,31 +304,31 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, } // Tier 1: Exact title match - rankCases = append(rankCases, fmt.Sprintf("WHEN LOWER(i.title) = LOWER(%s) THEN 1", phraseParam)) + rankCases = append(rankCases, fmt.Sprintf("WHEN LOWER(i.title) = %s THEN 1", phraseParam)) // Tier 2: Title starts with phrase - rankCases = append(rankCases, fmt.Sprintf("WHEN i.title ILIKE %s THEN 2", phraseStartsWith)) + rankCases = append(rankCases, fmt.Sprintf("WHEN LOWER(i.title) LIKE %s THEN 2", phraseStartsWith)) // Tier 3: Title contains phrase - rankCases = append(rankCases, fmt.Sprintf("WHEN i.title ILIKE %s THEN 3", phraseContains)) + rankCases = append(rankCases, fmt.Sprintf("WHEN LOWER(i.title) LIKE %s THEN 3", phraseContains)) // Tier 4: Title matches all words (multi-word only) if len(termParams) > 1 { var titleTerms []string for _, tp := range termParams { - titleTerms = append(titleTerms, fmt.Sprintf("i.title ILIKE '%s' || %s || '%s'", "%", tp, "%")) + titleTerms = append(titleTerms, fmt.Sprintf("LOWER(i.title) LIKE '%s' || %s || '%s'", "%", tp, "%")) } rankCases = append(rankCases, fmt.Sprintf("WHEN (%s) THEN 4", strings.Join(titleTerms, " AND "))) } // Tier 5: Description contains phrase - rankCases = append(rankCases, fmt.Sprintf("WHEN COALESCE(i.description, '') ILIKE %s THEN 5", phraseContains)) + rankCases = append(rankCases, fmt.Sprintf("WHEN LOWER(COALESCE(i.description, '')) LIKE %s THEN 5", phraseContains)) // Tier 6: Description matches all words (multi-word only) if len(termParams) > 1 { var descTerms []string for _, tp := range termParams { - descTerms = append(descTerms, fmt.Sprintf("COALESCE(i.description, '') ILIKE '%s' || %s || '%s'", "%", tp, "%")) + descTerms = append(descTerms, fmt.Sprintf("LOWER(COALESCE(i.description, '')) LIKE '%s' || %s || '%s'", "%", tp, "%")) } rankCases = append(rankCases, fmt.Sprintf("WHEN (%s) THEN 6", strings.Join(descTerms, " AND "))) } @@ -343,8 +349,8 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, // --- match_source expression --- matchSourceExpr := fmt.Sprintf(`CASE - WHEN i.title ILIKE %s THEN 'title' - WHEN COALESCE(i.description, '') ILIKE %s THEN 'description' + WHEN LOWER(i.title) LIKE %s THEN 'title' + WHEN LOWER(COALESCE(i.description, '')) LIKE %s THEN 'description' ELSE 'comment' END`, phraseContains, phraseContains) @@ -353,13 +359,13 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, var titleTerms []string var descTerms []string for _, tp := range termParams { - titleTerms = append(titleTerms, fmt.Sprintf("i.title ILIKE '%s' || %s || '%s'", "%", tp, "%")) - descTerms = append(descTerms, fmt.Sprintf("COALESCE(i.description, '') ILIKE '%s' || %s || '%s'", "%", tp, "%")) + titleTerms = append(titleTerms, fmt.Sprintf("LOWER(i.title) LIKE '%s' || %s || '%s'", "%", tp, "%")) + descTerms = append(descTerms, fmt.Sprintf("LOWER(COALESCE(i.description, '')) LIKE '%s' || %s || '%s'", "%", tp, "%")) } matchSourceExpr = fmt.Sprintf(`CASE - WHEN i.title ILIKE %s THEN 'title' + WHEN LOWER(i.title) LIKE %s THEN 'title' WHEN (%s) THEN 'title' - WHEN COALESCE(i.description, '') ILIKE %s THEN 'description' + WHEN LOWER(COALESCE(i.description, '')) LIKE %s THEN 'description' WHEN (%s) THEN 'description' ELSE 'comment' END`, @@ -371,11 +377,11 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, // --- matched_comment_content subquery --- // Find the most recent matching comment for comment-source matches. commentSubquery := fmt.Sprintf(`CASE - WHEN i.title ILIKE %s THEN '' - WHEN COALESCE(i.description, '') ILIKE %s THEN '' + WHEN LOWER(i.title) LIKE %s THEN '' + WHEN LOWER(COALESCE(i.description, '')) LIKE %s THEN '' ELSE COALESCE( (SELECT c.content FROM comment c - WHERE c.issue_id = i.id AND c.content ILIKE %s + WHERE c.issue_id = i.id AND LOWER(c.content) LIKE %s ORDER BY c.created_at DESC LIMIT 1), '' ) @@ -387,18 +393,18 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, var descTerms []string var commentTerms []string for _, tp := range termParams { - titleTerms = append(titleTerms, fmt.Sprintf("i.title ILIKE '%s' || %s || '%s'", "%", tp, "%")) - descTerms = append(descTerms, fmt.Sprintf("COALESCE(i.description, '') ILIKE '%s' || %s || '%s'", "%", tp, "%")) - commentTerms = append(commentTerms, fmt.Sprintf("c.content ILIKE '%s' || %s || '%s'", "%", tp, "%")) + titleTerms = append(titleTerms, fmt.Sprintf("LOWER(i.title) LIKE '%s' || %s || '%s'", "%", tp, "%")) + descTerms = append(descTerms, fmt.Sprintf("LOWER(COALESCE(i.description, '')) LIKE '%s' || %s || '%s'", "%", tp, "%")) + commentTerms = append(commentTerms, fmt.Sprintf("LOWER(c.content) LIKE '%s' || %s || '%s'", "%", tp, "%")) } commentSubquery = fmt.Sprintf(`CASE - WHEN i.title ILIKE %s THEN '' + WHEN LOWER(i.title) LIKE %s THEN '' WHEN (%s) THEN '' - WHEN COALESCE(i.description, '') ILIKE %s THEN '' + WHEN LOWER(COALESCE(i.description, '')) LIKE %s THEN '' WHEN (%s) THEN '' ELSE COALESCE( (SELECT c.content FROM comment c - WHERE c.issue_id = i.id AND (c.content ILIKE %s OR (%s)) + WHERE c.issue_id = i.id AND (LOWER(c.content) LIKE %s OR (%s)) ORDER BY c.created_at DESC LIMIT 1), '' ) diff --git a/server/internal/handler/search_test.go b/server/internal/handler/search_test.go new file mode 100644 index 000000000..1572e7d38 --- /dev/null +++ b/server/internal/handler/search_test.go @@ -0,0 +1,95 @@ +package handler + +import ( + "strings" + "testing" +) + +func TestBuildSearchQuery_SingleTerm(t *testing.T) { + query, args := buildSearchQuery("Hello", []string{"Hello"}, 0, false, false) + + // Pattern should be lowercased in Go. + if args[0] != "hello" { + t.Errorf("expected phrase arg to be lowercased, got %q", args[0]) + } + + // Must use LOWER(column) LIKE, not ILIKE. + if strings.Contains(query, "ILIKE") { + t.Error("query should not contain ILIKE") + } + if !strings.Contains(query, "LOWER(i.title) LIKE") { + t.Error("query should contain LOWER(i.title) LIKE") + } + if !strings.Contains(query, "LOWER(COALESCE(i.description, '')) LIKE") { + t.Error("query should contain LOWER(COALESCE(i.description, '')) LIKE") + } + if !strings.Contains(query, "LOWER(c.content) LIKE") { + t.Error("query should contain LOWER(c.content) LIKE") + } + + // Exact title rank should not double-LOWER the pattern. + if strings.Contains(query, "LOWER(i.title) = LOWER(") { + t.Error("exact title rank should not wrap pattern in LOWER (already lowercased in Go)") + } + if !strings.Contains(query, "LOWER(i.title) = $1") { + t.Error("exact title rank should compare LOWER(i.title) = $1 directly") + } + + // Should exclude closed issues by default. + if !strings.Contains(query, "NOT IN ('done', 'cancelled')") { + t.Error("query should exclude done/cancelled when includeClosed=false") + } +} + +func TestBuildSearchQuery_MultiTerm(t *testing.T) { + query, args := buildSearchQuery("Foo Bar", []string{"Foo", "Bar"}, 0, false, false) + + // Both phrase and terms should be lowercased. + if args[0] != "foo bar" { + t.Errorf("expected phrase arg lowercased, got %q", args[0]) + } + // args[1] is workspace_id placeholder; term args start at args[2]. + if args[2] != "foo" { + t.Errorf("expected first term arg lowercased, got %q", args[2]) + } + if args[3] != "bar" { + t.Errorf("expected second term arg lowercased, got %q", args[3]) + } + + // Multi-word query should have AND conditions. + if !strings.Contains(query, " AND ") { + t.Error("multi-word query should contain AND conditions for per-term matching") + } +} + +func TestBuildSearchQuery_WithNumber(t *testing.T) { + query, args := buildSearchQuery("MUL-42", []string{"MUL-42"}, 42, true, false) + + _ = args + // Number match should be in WHERE. + if !strings.Contains(query, "i.number = ") { + t.Error("query should contain number match in WHERE clause") + } + // Tier 0 rank for identifier match. + if !strings.Contains(query, "THEN 0") { + t.Error("query should contain tier 0 rank for identifier match") + } +} + +func TestBuildSearchQuery_IncludeClosed(t *testing.T) { + query, _ := buildSearchQuery("test", []string{"test"}, 0, false, true) + + if strings.Contains(query, "NOT IN ('done', 'cancelled')") { + t.Error("query should not exclude done/cancelled when includeClosed=true") + } +} + +func TestBuildSearchQuery_SpecialChars(t *testing.T) { + query, args := buildSearchQuery("100%", []string{"100%"}, 0, false, false) + + _ = query + // % should be escaped in the phrase arg. + if escaped, ok := args[0].(string); !ok || !strings.Contains(escaped, `\%`) { + t.Errorf("expected %% to be escaped in phrase arg, got %q", args[0]) + } +} diff --git a/server/migrations/036_search_index_lower.down.sql b/server/migrations/036_search_index_lower.down.sql new file mode 100644 index 000000000..18b38a394 --- /dev/null +++ b/server/migrations/036_search_index_lower.down.sql @@ -0,0 +1,14 @@ +-- Revert to original non-LOWER indexes. +DO $$ +BEGIN + DROP INDEX IF EXISTS idx_issue_title_bigm; + DROP INDEX IF EXISTS idx_issue_description_bigm; + DROP INDEX IF EXISTS idx_comment_content_bigm; + + CREATE INDEX idx_issue_title_bigm ON issue USING gin (title gin_bigm_ops); + CREATE INDEX idx_issue_description_bigm ON issue USING gin (COALESCE(description, '') gin_bigm_ops); + CREATE INDEX idx_comment_content_bigm ON comment USING gin (content gin_bigm_ops); +EXCEPTION WHEN OTHERS THEN + RAISE NOTICE 'skipping bigram index revert (pg_bigm not installed)'; +END +$$; diff --git a/server/migrations/036_search_index_lower.up.sql b/server/migrations/036_search_index_lower.up.sql new file mode 100644 index 000000000..b9a956566 --- /dev/null +++ b/server/migrations/036_search_index_lower.up.sql @@ -0,0 +1,17 @@ +-- Rebuild pg_bigm GIN indexes on LOWER() expressions so that +-- LOWER(column) LIKE queries can use the index (pg_bigm 1.2 does not support ILIKE). +DO $$ +BEGIN + -- Drop old indexes that were on raw columns + DROP INDEX IF EXISTS idx_issue_title_bigm; + DROP INDEX IF EXISTS idx_issue_description_bigm; + DROP INDEX IF EXISTS idx_comment_content_bigm; + + -- Recreate on LOWER() expressions + CREATE INDEX idx_issue_title_bigm ON issue USING gin (LOWER(title) gin_bigm_ops); + CREATE INDEX idx_issue_description_bigm ON issue USING gin (LOWER(COALESCE(description, '')) gin_bigm_ops); + CREATE INDEX idx_comment_content_bigm ON comment USING gin (LOWER(content) gin_bigm_ops); +EXCEPTION WHEN OTHERS THEN + RAISE NOTICE 'skipping bigram index rebuild (pg_bigm not installed)'; +END +$$;