From 057c04f707bc5874d6b16bbd81affc4f617d3ebf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Mar 2026 05:13:10 +0000 Subject: [PATCH 1/3] Initial plan From 5afefe26544e19b519864381b5f6b3a9c7369a0e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Mar 2026 05:21:05 +0000 Subject: [PATCH 2/3] filter out noisy n+1 queries and add n+1/slow filter keywords Co-authored-by: mickamy <11856337+mickamy@users.noreply.github.com> --- README.md | 6 +- cmd/sql-tapd/main.go | 18 +++++- cmd/sql-tapd/main_test.go | 129 ++++++++++++++++++++++++++++++++++++++ go.mod | 2 +- tui/filter.go | 28 +++++++-- tui/filter_test.go | 63 +++++++++++++++++++ web/static/app.js | 6 ++ 7 files changed, 244 insertions(+), 8 deletions(-) create mode 100644 cmd/sql-tapd/main_test.go diff --git a/README.md b/README.md index b351b81..6dc45cd 100644 --- a/README.md +++ b/README.md @@ -317,6 +317,8 @@ search. | `d>100ms` | Duration greater than | `d>1s`, `d>500us` | | `d<10ms` | Duration less than | `d<50ms` | | `error` | Events with errors only | | +| `n+1` | N+1 flagged queries | | +| `slow` | Slow queries only | | | `op:select` | SQL keyword prefix | `op:insert`, `op:update`, `op:delete` | | `op:begin` | Protocol operation | `op:commit`, `op:rollback` | | _(other)_ | Text substring match | `users`, `WHERE id` | @@ -351,7 +353,9 @@ Detection is enabled by default and runs server-side, so both TUI and Web UI ben | `--nplus1-cooldown` | `10s` | Minimum interval between alert notifications for the same query | Only SELECT queries are monitored. INSERT, UPDATE, DELETE, and transaction lifecycle commands (BEGIN, COMMIT, etc.) are -excluded. +excluded. Metadata queries — SELECT statements without a FROM clause, such as `SELECT database()`, `SELECT @@version`, +or `SELECT 1` — are also excluded, as they are typically driver health checks or system introspection calls rather than +application data queries. Once the threshold is crossed, all subsequent executions of the same template within the window are flagged. The cooldown only affects the notification frequency — the Status column marker always appears. diff --git a/cmd/sql-tapd/main.go b/cmd/sql-tapd/main.go index beeb43d..5973264 100644 --- a/cmd/sql-tapd/main.go +++ b/cmd/sql-tapd/main.go @@ -8,6 +8,7 @@ import ( "net" "os" "os/signal" + "regexp" "strings" "syscall" "time" @@ -235,9 +236,24 @@ func isSelectQuery(op proxy.Op, q string) bool { switch op { case proxy.OpQuery, proxy.OpExec, proxy.OpExecute: trimmed := strings.TrimSpace(q) - return len(trimmed) >= 6 && strings.EqualFold(trimmed[:6], "SELECT") + if len(trimmed) < 6 || !strings.EqualFold(trimmed[:6], "SELECT") { + return false + } + return !isMetadataQuery(trimmed) case proxy.OpPrepare, proxy.OpBind, proxy.OpBegin, proxy.OpCommit, proxy.OpRollback: return false } return false } + +// reFromClause matches the SQL FROM keyword as a whole word, used to detect +// whether a SELECT query references any table. +var reFromClause = regexp.MustCompile(`(?i)\bFROM\b`) + +// isMetadataQuery reports whether q is a system/metadata SELECT that should be +// excluded from N+1 detection. These are selects that do not reference any +// table (i.e. have no FROM clause), such as SELECT database(), SELECT @@version, +// or SELECT 1. +func isMetadataQuery(q string) bool { + return !reFromClause.MatchString(q) +} diff --git a/cmd/sql-tapd/main_test.go b/cmd/sql-tapd/main_test.go new file mode 100644 index 0000000..885f868 --- /dev/null +++ b/cmd/sql-tapd/main_test.go @@ -0,0 +1,129 @@ +package main + +import ( + "testing" + + "github.com/mickamy/sql-tap/proxy" +) + +func TestIsSelectQuery(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + op proxy.Op + q string + want bool + }{ + { + name: "regular select", + op: proxy.OpQuery, + q: "SELECT id FROM users WHERE id = 1", + want: true, + }, + { + name: "metadata: SELECT database()", + op: proxy.OpQuery, + q: "SELECT database()", + want: false, + }, + { + name: "metadata: SELECT @@version", + op: proxy.OpQuery, + q: "SELECT @@version", + want: false, + }, + { + name: "metadata: SELECT 1", + op: proxy.OpQuery, + q: "SELECT 1", + want: false, + }, + { + name: "metadata: SELECT NOW()", + op: proxy.OpQuery, + q: "SELECT NOW()", + want: false, + }, + { + name: "metadata: SELECT current_database()", + op: proxy.OpQuery, + q: "SELECT current_database()", + want: false, + }, + { + name: "select with FROM (not metadata)", + op: proxy.OpQuery, + q: "SELECT 1 FROM dual", + want: true, + }, + { + name: "insert not select", + op: proxy.OpQuery, + q: "INSERT INTO users VALUES (1)", + want: false, + }, + { + name: "begin op", + op: proxy.OpBegin, + q: "", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := isSelectQuery(tt.op, tt.q) + if got != tt.want { + t.Errorf("isSelectQuery(%v, %q) = %v, want %v", tt.op, tt.q, got, tt.want) + } + }) + } +} + +func TestIsMetadataQuery(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + q string + want bool + }{ + { + name: "no FROM clause", + q: "SELECT database()", + want: true, + }, + { + name: "system variable", + q: "SELECT @@session.transaction_read_only", + want: true, + }, + { + name: "constant", + q: "SELECT 1", + want: true, + }, + { + name: "has FROM clause", + q: "SELECT id FROM users", + want: false, + }, + { + name: "subquery with FROM", + q: "SELECT (SELECT COUNT(*) FROM orders)", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := isMetadataQuery(tt.q) + if got != tt.want { + t.Errorf("isMetadataQuery(%q) = %v, want %v", tt.q, got, tt.want) + } + }) + } +} diff --git a/go.mod b/go.mod index edebbe0..1ecc35d 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/testcontainers/testcontainers-go/modules/mysql v0.40.0 google.golang.org/grpc v1.79.1 google.golang.org/protobuf v1.36.11 + gopkg.in/yaml.v3 v3.0.1 ) require ( @@ -96,5 +97,4 @@ require ( golang.org/x/sys v0.39.0 // indirect golang.org/x/text v0.34.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260209200024-4cfbd4190f57 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/tui/filter.go b/tui/filter.go index ce79c4d..1d847a3 100644 --- a/tui/filter.go +++ b/tui/filter.go @@ -16,6 +16,8 @@ const ( filterDuration // d>100ms, d<10ms filterError // "error" keyword filterOp // op:select, op:begin, etc. + filterNPlus1 // "n+1" or "nplus1" keyword + filterSlow // "slow" keyword ) type durationOp int @@ -70,18 +72,27 @@ func parseFilter(input string) []filterCondition { conds = append(conds, c) continue } - if strings.ToLower(tok) == "error" { + lower := strings.ToLower(tok) + if lower == "error" { conds = append(conds, filterCondition{kind: filterError}) continue } - if c, ok := parseOp(tok); ok { + if lower == "n+1" || lower == "nplus1" { + conds = append(conds, filterCondition{kind: filterNPlus1}) + continue + } + if lower == "slow" { + conds = append(conds, filterCondition{kind: filterSlow}) + continue + } + if c, ok := parseOp(lower); ok { conds = append(conds, c) continue } // Fallback: plain text match. conds = append(conds, filterCondition{ kind: filterText, - text: strings.ToLower(tok), + text: lower, }) } return conds @@ -124,8 +135,7 @@ func unitSuffix(unit string) string { return "ms" } -func parseOp(tok string) (filterCondition, bool) { - lower := strings.ToLower(tok) +func parseOp(lower string) (filterCondition, bool) { if !strings.HasPrefix(lower, "op:") { return filterCondition{}, false } @@ -157,6 +167,10 @@ func (c filterCondition) matchesEvent(ev *tapv1.QueryEvent) bool { } case filterError: return ev.GetError() != "" + case filterNPlus1: + return ev.GetNPlus_1() + case filterSlow: + return ev.GetSlowQuery() case filterOp: return matchOp(ev, c.opPattern) } @@ -203,6 +217,10 @@ func describeFilter(input string) string { parts = append(parts, "d"+op+c.durValue.String()) case filterError: parts = append(parts, "error") + case filterNPlus1: + parts = append(parts, "n+1") + case filterSlow: + parts = append(parts, "slow") case filterOp: parts = append(parts, "op:"+c.opPattern) } diff --git a/tui/filter_test.go b/tui/filter_test.go index 1254e36..9d3a3c7 100644 --- a/tui/filter_test.go +++ b/tui/filter_test.go @@ -79,6 +79,27 @@ func TestParseFilter(t *testing.T) { {kind: filterOp, opPattern: "begin"}, }, }, + { + name: "n+1 keyword", + input: "n+1", + want: []filterCondition{ + {kind: filterNPlus1}, + }, + }, + { + name: "nplus1 keyword", + input: "nplus1", + want: []filterCondition{ + {kind: filterNPlus1}, + }, + }, + { + name: "slow keyword", + input: "slow", + want: []filterCondition{ + {kind: filterSlow}, + }, + }, { name: "combined filter", input: "op:select d>100ms", @@ -227,6 +248,38 @@ func TestMatchesEvent(t *testing.T) { ev: makeEvent(proxy.OpQuery, "INSERT INTO users (name) VALUES ('alice')", 5*time.Millisecond, ""), want: true, }, + { + name: "n+1 match", + cond: filterCondition{kind: filterNPlus1}, + ev: func() *tapv1.QueryEvent { + ev := makeEvent(proxy.OpQuery, "SELECT id FROM users WHERE id = 1", 5*time.Millisecond, "") + ev.NPlus_1 = true + return ev + }(), + want: true, + }, + { + name: "n+1 no match", + cond: filterCondition{kind: filterNPlus1}, + ev: makeEvent(proxy.OpQuery, "SELECT id FROM users WHERE id = 1", 5*time.Millisecond, ""), + want: false, + }, + { + name: "slow match", + cond: filterCondition{kind: filterSlow}, + ev: func() *tapv1.QueryEvent { + ev := makeEvent(proxy.OpQuery, "SELECT id FROM users", 500*time.Millisecond, "") + ev.SlowQuery = true + return ev + }(), + want: true, + }, + { + name: "slow no match", + cond: filterCondition{kind: filterSlow}, + ev: makeEvent(proxy.OpQuery, "SELECT id FROM users", 5*time.Millisecond, ""), + want: false, + }, } for _, tt := range tests { @@ -356,6 +409,16 @@ func TestDescribeFilter(t *testing.T) { input: "error", want: "error", }, + { + name: "n+1 keyword", + input: "n+1", + want: "n+1", + }, + { + name: "slow keyword", + input: "slow", + want: "slow", + }, { name: "text fallback", input: "users", diff --git a/web/static/app.js b/web/static/app.js index 1768905..78b0eb3 100644 --- a/web/static/app.js +++ b/web/static/app.js @@ -202,6 +202,8 @@ function parseFilterTokens(input) { return {kind: 'duration', op, ms}; } if (tok.toLowerCase() === 'error') return {kind: 'error'}; + if (tok.toLowerCase() === 'n+1' || tok.toLowerCase() === 'nplus1') return {kind: 'nplus1'}; + if (tok.toLowerCase() === 'slow') return {kind: 'slow'}; const lower = tok.toLowerCase(); if (lower.startsWith('op:') && lower.length > 3) return {kind: 'op', pattern: lower.slice(3)}; return {kind: 'text', text: lower}; @@ -214,6 +216,10 @@ function matchesFilter(ev, cond) { return cond.op === '>' ? ev.duration_ms > cond.ms : ev.duration_ms < cond.ms; case 'error': return !!ev.error; + case 'nplus1': + return !!ev.n_plus_1; + case 'slow': + return !!ev.slow_query; case 'op': if (PROTOCOL_OPS.has(cond.pattern)) return ev.op.toLowerCase() === cond.pattern; if (OP_KEYWORDS.has(cond.pattern)) return (ev.query || '').trim().toLowerCase().startsWith(cond.pattern); From 1c985ec6e7a88fc8ffa52075bd429e0208c82412 Mon Sep 17 00:00:00 2001 From: Tetsuro Mikami Date: Sun, 15 Mar 2026 09:44:29 +0900 Subject: [PATCH 3/3] fix: document FROM heuristic limitation and add nplus1 alias to README --- README.md | 2 +- cmd/sql-tapd/main.go | 4 ++++ cmd/sql-tapd/main_test.go | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6dc45cd..bb0a636 100644 --- a/README.md +++ b/README.md @@ -317,7 +317,7 @@ search. | `d>100ms` | Duration greater than | `d>1s`, `d>500us` | | `d<10ms` | Duration less than | `d<50ms` | | `error` | Events with errors only | | -| `n+1` | N+1 flagged queries | | +| `n+1` | N+1 flagged queries | alias: `nplus1` | | `slow` | Slow queries only | | | `op:select` | SQL keyword prefix | `op:insert`, `op:update`, `op:delete` | | `op:begin` | Protocol operation | `op:commit`, `op:rollback` | diff --git a/cmd/sql-tapd/main.go b/cmd/sql-tapd/main.go index 5973264..743b833 100644 --- a/cmd/sql-tapd/main.go +++ b/cmd/sql-tapd/main.go @@ -248,6 +248,10 @@ func isSelectQuery(op proxy.Op, q string) bool { // reFromClause matches the SQL FROM keyword as a whole word, used to detect // whether a SELECT query references any table. +// NOTE: This is a simple keyword check; it cannot distinguish a FROM clause +// from FROM inside expressions (e.g. EXTRACT(EPOCH FROM NOW()) in Postgres). +// Such queries will not be classified as metadata, which is a safe default +// (they stay in N+1 detection rather than being silently dropped). var reFromClause = regexp.MustCompile(`(?i)\bFROM\b`) // isMetadataQuery reports whether q is a system/metadata SELECT that should be diff --git a/cmd/sql-tapd/main_test.go b/cmd/sql-tapd/main_test.go index 885f868..280bb60 100644 --- a/cmd/sql-tapd/main_test.go +++ b/cmd/sql-tapd/main_test.go @@ -115,6 +115,11 @@ func TestIsMetadataQuery(t *testing.T) { q: "SELECT (SELECT COUNT(*) FROM orders)", want: false, }, + { + name: "expression-level FROM (Postgres EXTRACT)", + q: "SELECT EXTRACT(EPOCH FROM NOW())", + want: false, // false negative: no table, but FROM in expression + }, } for _, tt := range tests {