From a554a0c602b299170b29583f6cd548b3612742f3 Mon Sep 17 00:00:00 2001 From: yushen Date: Fri, 10 Apr 2026 14:23:38 +0800 Subject: [PATCH] fix(search): lowercase pattern in Go, add buildSearchQuery unit tests - Lowercase phrase/terms in Go (strings.ToLower) so SQL only needs LOWER() on the column side, avoiding redundant per-query LOWER() on the pattern - Add 5 unit tests for buildSearchQuery asserting SQL shape: no ILIKE, LOWER on columns only, lowercased args, multi-term AND, number match, include-closed flag, special char escaping Co-Authored-By: Claude Opus 4.6 (1M context) --- server/internal/handler/issue.go | 56 ++++++++------- server/internal/handler/search_test.go | 95 ++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 25 deletions(-) create mode 100644 server/internal/handler/search_test.go diff --git a/server/internal/handler/issue.go b/server/internal/handler/issue.go index 08ea44a94..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 case-insensitive matching via LOWER/LIKE (pg_bigm compatible), -// 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{} @@ -257,7 +263,7 @@ func buildSearchQuery(phrase string, terms []string, queryNum int, hasNum bool, // Full phrase match: title, description, or comment phraseMatch := fmt.Sprintf( - "(LOWER(i.title) LIKE LOWER(%s) OR LOWER(COALESCE(i.description, '')) LIKE LOWER(%s) OR EXISTS (SELECT 1 FROM comment c WHERE c.issue_id = i.id AND LOWER(c.content) LIKE LOWER(%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( - "(LOWER(i.title) LIKE LOWER(%s) OR LOWER(COALESCE(i.description, '')) LIKE LOWER(%s) OR EXISTS (SELECT 1 FROM comment c WHERE c.issue_id = i.id AND LOWER(c.content) LIKE LOWER(%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 LOWER(i.title) LIKE LOWER(%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 LOWER(i.title) LIKE LOWER(%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("LOWER(i.title) LIKE LOWER('%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 LOWER(COALESCE(i.description, '')) LIKE LOWER(%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("LOWER(COALESCE(i.description, '')) LIKE LOWER('%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 LOWER(i.title) LIKE LOWER(%s) THEN 'title' - WHEN LOWER(COALESCE(i.description, '')) LIKE LOWER(%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("LOWER(i.title) LIKE LOWER('%s' || %s || '%s')", "%", tp, "%")) - descTerms = append(descTerms, fmt.Sprintf("LOWER(COALESCE(i.description, '')) LIKE LOWER('%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 LOWER(i.title) LIKE LOWER(%s) THEN 'title' + WHEN LOWER(i.title) LIKE %s THEN 'title' WHEN (%s) THEN 'title' - WHEN LOWER(COALESCE(i.description, '')) LIKE LOWER(%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 LOWER(i.title) LIKE LOWER(%s) THEN '' - WHEN LOWER(COALESCE(i.description, '')) LIKE LOWER(%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 LOWER(c.content) LIKE LOWER(%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("LOWER(i.title) LIKE LOWER('%s' || %s || '%s')", "%", tp, "%")) - descTerms = append(descTerms, fmt.Sprintf("LOWER(COALESCE(i.description, '')) LIKE LOWER('%s' || %s || '%s')", "%", tp, "%")) - commentTerms = append(commentTerms, fmt.Sprintf("LOWER(c.content) LIKE LOWER('%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 LOWER(i.title) LIKE LOWER(%s) THEN '' + WHEN LOWER(i.title) LIKE %s THEN '' WHEN (%s) THEN '' - WHEN LOWER(COALESCE(i.description, '')) LIKE LOWER(%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 (LOWER(c.content) LIKE LOWER(%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]) + } +}