mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-16 14:49:09 +02:00
* fix(search): add pg_trgm index fallback + statement_timeout guard (MUL-4059) Root cause of the "search freezes with no response" symptom reported in MUL-4059: the search handler runs LOWER(col) LIKE '%pattern%' queries that expect a pg_bigm GIN index (migrations 032, 033, 036), but every migration wraps the CREATE EXTENSION + CREATE INDEX in a DO/EXCEPTION handler that silently skips when pg_bigm is unavailable. The bundled self-host / dev / CI Postgres image (pgvector/pgvector:pg17) does not ship pg_bigm, so on every self-hosted deployment the migrations no-op and no GIN indexes get built. Every /api/issues/search + /api/projects/search request then falls back to a Seq Scan on `issue` + correlated Seq Scans on `comment` — verified with EXPLAIN on the local dev DB, which has zero title/description/comment search indexes before this change. Two independent guardrails are added, either of which alone would have prevented the reported hang: 1) Migration 134 installs pg_trgm (ships in all standard Postgres + pgvector images) and builds GIN indexes with gin_trgm_ops on `LOWER(title)`, `LOWER(COALESCE(description, ''))`, and `LOWER(content)`. The expression signatures match the search handler's WHERE clauses exactly, so the planner picks the index without further changes. The pg_bigm indexes from 036 are left intact — deployments on AWS RDS with pg_bigm 1.2 keep the CJK-friendly bigram path; deployments without it get the trigram fallback. Verified against a local 25k-row fixture: the description LIKE hits `Bitmap Index Scan on idx_issue_description_trgm` in 0.5 ms. 2) runSearchQuery wraps both search handlers in a short-lived read-only transaction with SET LOCAL statement_timeout = 3 s. In the pathological case where indexes are still missing or the query plan is bad, callers see a fast 503 with a descriptive error instead of a stalled request. Verified against a live Postgres: a deliberate pg_sleep(2) with the test override at 200 ms is cut off in 230 ms with SQLSTATE 57014, as asserted by TestRunSearchQuery_StatementTimeoutFires. Non-goals: this change does not remove the pg_bigm code path, does not change the SQL the handler builds, and does not change the API response shape. It is the minimum diff to unblock production while preserving the CJK-search advantage that pg_bigm provides where it is available. Co-authored-by: multica-agent <github@multica.ai> * fix(search): scope comment subqueries to workspace to unblock prd hang (MUL-4059) Follow-up correction after PRD investigation: pg_bigm IS installed on prd `multica-prod` and all five bigm indexes exist in the correct `LOWER(...) gin_bigm_ops` form. The initial "missing index" hypothesis was wrong; migration 134 (pg_trgm fallback) still helps self-host but does not touch the production hang path. Actual prd EXPLAIN (workspace with 60k issues, keyword "search"): Index Scan using idx_issue_workspace on issue i Rows Removed by Filter: 59123 SubPlan 2 Bitmap Heap Scan on comment c Rows Removed by Index Recheck: 1928275 Heap Blocks: exact=48297 lossy=164696 Bitmap Index Scan on idx_comment_content_bigm rows=536761 Execution Time: 32345.002 ms Root cause: the correlated `EXISTS` over `comment` gets rewritten by the planner into a *hashed* subplan. Without a workspace_id filter in the subquery, that hashed set covers every comment in every workspace matching the LIKE — 536k rows for "search" — which spills work_mem into a lossy bitmap and rechecks 1.9M rows. Two-part fix: 1. Query rewrite. buildSearchQuery now emits `c.workspace_id = $wsParam` inside every comment subquery (WHERE phrase match, WHERE multi-term match, tier 7 rank, tier 8 rank, and the matched_comment_content COALESCE). The same $4 parameter is reused so Postgres treats it as a compile-time constant and pushes it into the hashed subplan's key, collapsing the set to this workspace's comments. 2. Supporting index (migration 135). New `idx_comment_workspace ON comment (workspace_id)`. Without it, the pushed-down filter still triggers a Seq Scan on `comment` because comment has no btree on workspace_id (only the FK constraint and composite (issue_id, ...) indexes). Locally verified against a repro that mirrors prd (5k issues in the target workspace, 100k comments in a sibling workspace all containing "search"): the plan drops from 60 ms (hashed global scan, no support index) to 3 ms (subplan uses idx_comment_workspace). Prd extrapolation from the same shape: 32.3 s → tens of milliseconds. Regression test TestBuildSearchQuery_CommentSubqueryWorkspaceScope asserts every `FROM comment c` in the generated SQL is followed by a `c.workspace_id = $4` filter, so a future refactor can't silently regress the plan back to the global-hash pathology. The statement_timeout guard from the earlier commit in this branch is kept — it still bounds the worst case if any future query shape regresses. Co-authored-by: multica-agent <github@multica.ai> * fix(search): address PR review — unwrap 135 + add project trigram indexes (MUL-4059) Both must-fix items from GPT-Boy's review: 1. Migration 135 unwrapped. The previous version buried `CREATE INDEX idx_comment_workspace` inside `DO $$ ... EXCEPTION WHEN OTHERS $$` — exactly the anti-pattern that caused MUL-4059 in the first place. `idx_comment_workspace` is not a CJK-bonus fallback; it is the critical support that makes the query rewrite land on an Index Scan instead of a Seq Scan. A silent failure (lock timeout, disk full, permission denied, schema drift) MUST abort the migration and fail deployment, not slip through as green. The unwrapped `CREATE INDEX IF NOT EXISTS` now propagates real errors to the migration runner, which aborts and does NOT record the version as applied. IF NOT EXISTS keeps idempotency for the operator-precreated case (`CREATE INDEX CONCURRENTLY ...` before running migrations on large prd tables). 2. Migration 134 now covers project search too. SearchProjects reads `LOWER(project.title)` and `LOWER(COALESCE(project.description, ''))`, and the pg_bigm equivalents in migration 039 silently no-op on pg_bigm-less images just like 032/033/036. Without the trigram fallback, project searches on self-host would still Seq Scan and hit the 3 s statement_timeout guard as a 503 — technically bounded but not actually fixed. Added `idx_project_title_trgm` and `idx_project_description_trgm`; the down migration drops them too. Also: fixed the search.go comment that said callers get a "standard 500" — they get a 503 with SQLSTATE-57014 mapping; the comment now matches reality. Verified: build clean, vet clean, existing search / timeout tests still green. Migration 135 dry-run (dropping the index, re-applying the unwrapped SQL under `ON_ERROR_STOP=1`) creates the index cleanly; a deliberate `CREATE INDEX` on a non-existent column now aborts psql with exit 3, confirming the migration runner would fail loudly on any real error. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Eve <eve@multica-ai.local> Co-authored-by: multica-agent <github@multica.ai>
297 lines
11 KiB
Go
297 lines
11 KiB
Go
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[0]=exact, args[1]=%phrase%, args[2]=phrase%, args[3]=workspace_id placeholder; term args start at args[4].
|
|
if args[4] != "%foo%" {
|
|
t.Errorf("expected first term arg as contains pattern, got %q", args[4])
|
|
}
|
|
if args[5] != "%bar%" {
|
|
t.Errorf("expected second term arg as contains pattern, got %q", args[5])
|
|
}
|
|
|
|
// 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])
|
|
}
|
|
}
|
|
|
|
// --- Project search tests ---
|
|
|
|
func TestBuildProjectSearchQuery_SingleTerm(t *testing.T) {
|
|
query, args := buildProjectSearchQuery("Hello", []string{"Hello"}, false)
|
|
|
|
if args[0] != "hello" {
|
|
t.Errorf("expected phrase arg to be lowercased, got %q", args[0])
|
|
}
|
|
|
|
if strings.Contains(query, "ILIKE") {
|
|
t.Error("query should not contain ILIKE")
|
|
}
|
|
if !strings.Contains(query, "LOWER(p.title) LIKE") {
|
|
t.Error("query should contain LOWER(p.title) LIKE")
|
|
}
|
|
if !strings.Contains(query, "LOWER(COALESCE(p.description, '')) LIKE") {
|
|
t.Error("query should contain LOWER(COALESCE(p.description, '')) LIKE")
|
|
}
|
|
|
|
// Should exclude completed/cancelled by default.
|
|
if !strings.Contains(query, "NOT IN ('completed', 'cancelled')") {
|
|
t.Error("query should exclude completed/cancelled when includeClosed=false")
|
|
}
|
|
}
|
|
|
|
func TestBuildProjectSearchQuery_MultiTerm(t *testing.T) {
|
|
query, args := buildProjectSearchQuery("Foo Bar", []string{"Foo", "Bar"}, false)
|
|
|
|
if args[0] != "foo bar" {
|
|
t.Errorf("expected phrase arg lowercased, got %q", args[0])
|
|
}
|
|
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])
|
|
}
|
|
|
|
if !strings.Contains(query, " AND ") {
|
|
t.Error("multi-word query should contain AND conditions for per-term matching")
|
|
}
|
|
}
|
|
|
|
func TestBuildProjectSearchQuery_IncludeClosed(t *testing.T) {
|
|
query, _ := buildProjectSearchQuery("test", []string{"test"}, true)
|
|
|
|
if strings.Contains(query, "NOT IN ('completed', 'cancelled')") {
|
|
t.Error("query should not exclude completed/cancelled when includeClosed=true")
|
|
}
|
|
}
|
|
|
|
// --- extractSnippet regression tests ---
|
|
|
|
func TestExtractSnippet_PhraseMatch(t *testing.T) {
|
|
content := "The quick brown fox jumps over the lazy dog near the river bank"
|
|
snippet := extractSnippet(content, "brown fox")
|
|
if !strings.Contains(snippet, "brown fox") {
|
|
t.Errorf("snippet should contain the phrase 'brown fox', got %q", snippet)
|
|
}
|
|
}
|
|
|
|
func TestExtractSnippet_MultiWordNonContiguous(t *testing.T) {
|
|
// "deploy" and "kubernetes" both appear but not as a contiguous phrase.
|
|
content := "We need to deploy the new service. The kubernetes cluster is ready for production workloads."
|
|
snippet := extractSnippet(content, "deploy kubernetes")
|
|
// Should NOT fall back to first 120 chars blindly — should center on earliest term.
|
|
if !strings.Contains(strings.ToLower(snippet), "deploy") && !strings.Contains(strings.ToLower(snippet), "kubernetes") {
|
|
t.Errorf("snippet should contain at least one search term, got %q", snippet)
|
|
}
|
|
// Specifically, "deploy" appears first so snippet should be centered around it.
|
|
if !strings.Contains(strings.ToLower(snippet), "deploy") {
|
|
t.Errorf("snippet should center on earliest term 'deploy', got %q", snippet)
|
|
}
|
|
}
|
|
|
|
func TestExtractSnippet_FallbackWhenNoMatch(t *testing.T) {
|
|
content := strings.Repeat("a", 200)
|
|
snippet := extractSnippet(content, "zzz")
|
|
if len([]rune(snippet)) > 124 { // 120 + "..."
|
|
t.Errorf("snippet should be truncated to ~120 runes when no match, got len=%d", len([]rune(snippet)))
|
|
}
|
|
}
|
|
|
|
func TestExtractSnippet_ShortContent(t *testing.T) {
|
|
content := "short text"
|
|
snippet := extractSnippet(content, "missing")
|
|
if snippet != content {
|
|
t.Errorf("short content with no match should return as-is, got %q", snippet)
|
|
}
|
|
}
|
|
|
|
func TestExtractSnippet_CaseInsensitive(t *testing.T) {
|
|
content := "Error in HTML rendering pipeline"
|
|
snippet := extractSnippet(content, "html")
|
|
if !strings.Contains(snippet, "HTML") {
|
|
t.Errorf("snippet should find case-insensitive match, got %q", snippet)
|
|
}
|
|
}
|
|
|
|
func TestExtractSnippet_CJKContent(t *testing.T) {
|
|
content := "这是一段很长的中文内容,包含了搜索关键词测试用例,用来验证多字节字符不会被截断的情况"
|
|
snippet := extractSnippet(content, "搜索关键词")
|
|
if !strings.Contains(snippet, "搜索关键词") {
|
|
t.Errorf("snippet should contain CJK phrase, got %q", snippet)
|
|
}
|
|
}
|
|
|
|
// --- Ranking regression tests ---
|
|
|
|
func TestBuildSearchQuery_CommentRankTiers(t *testing.T) {
|
|
query, _ := buildSearchQuery("test phrase", []string{"test", "phrase"}, 0, false, false)
|
|
|
|
// Comment phrase match should be tier 7
|
|
if !strings.Contains(query, "THEN 7") {
|
|
t.Error("query should contain tier 7 for comment phrase match")
|
|
}
|
|
// Comment all-term match should be tier 8
|
|
if !strings.Contains(query, "THEN 8") {
|
|
t.Error("query should contain tier 8 for comment all-term match")
|
|
}
|
|
// Fallback should be 9, not 7
|
|
if !strings.Contains(query, "ELSE 9") {
|
|
t.Error("query fallback should be ELSE 9")
|
|
}
|
|
}
|
|
|
|
func TestBuildSearchQuery_DescriptionRankTiers(t *testing.T) {
|
|
query, _ := buildSearchQuery("foo bar", []string{"foo", "bar"}, 0, false, false)
|
|
|
|
// Description phrase match should be tier 5
|
|
if !strings.Contains(query, "THEN 5") {
|
|
t.Error("query should contain tier 5 for description phrase match")
|
|
}
|
|
// Description all-term match should be tier 6
|
|
if !strings.Contains(query, "THEN 6") {
|
|
t.Error("query should contain tier 6 for description all-term match")
|
|
}
|
|
}
|
|
|
|
func TestBuildSearchQuery_SingleTermNoAllTermTiers(t *testing.T) {
|
|
query, _ := buildSearchQuery("html", []string{"html"}, 0, false, false)
|
|
|
|
// Extract the rank CASE expression (ends with "ELSE 9 END") to avoid
|
|
// false matches against statusRank which also contains THEN 4/6.
|
|
rankEnd := strings.Index(query, "ELSE 9 END")
|
|
if rankEnd == -1 {
|
|
t.Fatal("query should contain rank expression with ELSE 9 END")
|
|
}
|
|
rankExpr := query[:rankEnd]
|
|
|
|
// Single-term queries should NOT have tier 4 (title all-terms), 6 (desc all-terms), or 8 (comment all-terms)
|
|
if strings.Contains(rankExpr, "THEN 4") {
|
|
t.Error("single-term query should not have tier 4 (title all-terms)")
|
|
}
|
|
if strings.Contains(rankExpr, "THEN 6") {
|
|
t.Error("single-term query should not have tier 6 (description all-terms)")
|
|
}
|
|
if strings.Contains(rankExpr, "THEN 8") {
|
|
t.Error("single-term query should not have tier 8 (comment all-terms)")
|
|
}
|
|
}
|
|
|
|
// TestBuildSearchQuery_CommentSubqueryWorkspaceScope regressions the
|
|
// MUL-4059 fix: every EXISTS / correlated subquery over `comment` MUST
|
|
// filter by c.workspace_id = $wsParam. Without this, Postgres rewrites
|
|
// the correlated subquery into a hashed subplan that materializes every
|
|
// comment in the entire table matching the LIKE — on prd this was
|
|
// 536k rows / 32.3 s for '%search%'. With the filter the hashed set
|
|
// collapses to this workspace's comments and the plan uses the
|
|
// idx_comment_workspace supporting btree.
|
|
//
|
|
// $4 is buildSearchQuery's canonical workspace_id placeholder (the
|
|
// caller writes wsUUID into args[3] before executing).
|
|
func TestBuildSearchQuery_CommentSubqueryWorkspaceScope(t *testing.T) {
|
|
singleQuery, _ := buildSearchQuery("html", []string{"html"}, 0, false, false)
|
|
|
|
// Every occurrence of `FROM comment c` must be followed by the
|
|
// c.workspace_id = $4 constraint. Counting is safer than a single
|
|
// substring check because the WHERE, rank CASE, matched_comment_content
|
|
// subqueries all touch `comment` and must each carry the filter.
|
|
fromCount := strings.Count(singleQuery, "FROM comment c")
|
|
scopedCount := strings.Count(singleQuery, "c.workspace_id = $4")
|
|
if fromCount == 0 {
|
|
t.Fatalf("single-term query has no comment subquery — did buildSearchQuery drop it?")
|
|
}
|
|
if scopedCount < fromCount {
|
|
t.Errorf("single-term query has %d comment subqueries but only %d workspace_id filters — %d unscoped subquery(ies) will trigger the MUL-4059 global-hash plan",
|
|
fromCount, scopedCount, fromCount-scopedCount)
|
|
}
|
|
|
|
// Multi-term uses one extra comment subquery in the WHERE and one in
|
|
// the rank CASE for the all-terms match — same invariant applies.
|
|
multiQuery, _ := buildSearchQuery("foo bar", []string{"foo", "bar"}, 0, false, false)
|
|
fromCountMulti := strings.Count(multiQuery, "FROM comment c")
|
|
scopedCountMulti := strings.Count(multiQuery, "c.workspace_id = $4")
|
|
if scopedCountMulti < fromCountMulti {
|
|
t.Errorf("multi-term query has %d comment subqueries but only %d workspace_id filters",
|
|
fromCountMulti, scopedCountMulti)
|
|
}
|
|
}
|