Skip to content

Commit 0ac6aaf

Browse files
mickamymickamy
authored
Merge pull request #31 from mickamy/copilot/filter-out-noisy-n-plus-1-queries
Filter metadata queries from N+1 detection; add `n+1` and `slow` filter keywords
2 parents 8c308ae + 1c985ec commit 0ac6aaf

7 files changed

Lines changed: 253 additions & 8 deletions

File tree

‎README.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,8 @@ search.
317317
| `d>100ms` | Duration greater than | `d>1s`, `d>500us` |
318318
| `d<10ms` | Duration less than | `d<50ms` |
319319
| `error` | Events with errors only | |
320+
| `n+1` | N+1 flagged queries | alias: `nplus1` |
321+
| `slow` | Slow queries only | |
320322
| `op:select` | SQL keyword prefix | `op:insert`, `op:update`, `op:delete` |
321323
| `op:begin` | Protocol operation | `op:commit`, `op:rollback` |
322324
| _(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
351353
| `--nplus1-cooldown` | `10s` | Minimum interval between alert notifications for the same query |
352354

353355
Only SELECT queries are monitored. INSERT, UPDATE, DELETE, and transaction lifecycle commands (BEGIN, COMMIT, etc.) are
354-
excluded.
356+
excluded. Metadata queries — SELECT statements without a FROM clause, such as `SELECT database()`, `SELECT @@version`,
357+
or `SELECT 1` — are also excluded, as they are typically driver health checks or system introspection calls rather than
358+
application data queries.
355359

356360
Once the threshold is crossed, all subsequent executions of the same template within the window are flagged. The
357361
cooldown only affects the notification frequency — the Status column marker always appears.

‎cmd/sql-tapd/main.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net"
99
"os"
1010
"os/signal"
11+
"regexp"
1112
"strings"
1213
"syscall"
1314
"time"
@@ -235,9 +236,28 @@ func isSelectQuery(op proxy.Op, q string) bool {
235236
switch op {
236237
case proxy.OpQuery, proxy.OpExec, proxy.OpExecute:
237238
trimmed := strings.TrimSpace(q)
238-
return len(trimmed) >= 6 && strings.EqualFold(trimmed[:6], "SELECT")
239+
if len(trimmed) < 6 || !strings.EqualFold(trimmed[:6], "SELECT") {
240+
return false
241+
}
242+
return !isMetadataQuery(trimmed)
239243
case proxy.OpPrepare, proxy.OpBind, proxy.OpBegin, proxy.OpCommit, proxy.OpRollback:
240244
return false
241245
}
242246
return false
243247
}
248+
249+
// reFromClause matches the SQL FROM keyword as a whole word, used to detect
250+
// whether a SELECT query references any table.
251+
// NOTE: This is a simple keyword check; it cannot distinguish a FROM clause
252+
// from FROM inside expressions (e.g. EXTRACT(EPOCH FROM NOW()) in Postgres).
253+
// Such queries will not be classified as metadata, which is a safe default
254+
// (they stay in N+1 detection rather than being silently dropped).
255+
var reFromClause = regexp.MustCompile(`(?i)\bFROM\b`)
256+
257+
// isMetadataQuery reports whether q is a system/metadata SELECT that should be
258+
// excluded from N+1 detection. These are selects that do not reference any
259+
// table (i.e. have no FROM clause), such as SELECT database(), SELECT @@version,
260+
// or SELECT 1.
261+
func isMetadataQuery(q string) bool {
262+
return !reFromClause.MatchString(q)
263+
}

‎cmd/sql-tapd/main_test.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package main
2+
3+
import (
4+
"testing"
5+
6+
"github.com/mickamy/sql-tap/proxy"
7+
)
8+
9+
func TestIsSelectQuery(t *testing.T) {
10+
t.Parallel()
11+
12+
tests := []struct {
13+
name string
14+
op proxy.Op
15+
q string
16+
want bool
17+
}{
18+
{
19+
name: "regular select",
20+
op: proxy.OpQuery,
21+
q: "SELECT id FROM users WHERE id = 1",
22+
want: true,
23+
},
24+
{
25+
name: "metadata: SELECT database()",
26+
op: proxy.OpQuery,
27+
q: "SELECT database()",
28+
want: false,
29+
},
30+
{
31+
name: "metadata: SELECT @@version",
32+
op: proxy.OpQuery,
33+
q: "SELECT @@version",
34+
want: false,
35+
},
36+
{
37+
name: "metadata: SELECT 1",
38+
op: proxy.OpQuery,
39+
q: "SELECT 1",
40+
want: false,
41+
},
42+
{
43+
name: "metadata: SELECT NOW()",
44+
op: proxy.OpQuery,
45+
q: "SELECT NOW()",
46+
want: false,
47+
},
48+
{
49+
name: "metadata: SELECT current_database()",
50+
op: proxy.OpQuery,
51+
q: "SELECT current_database()",
52+
want: false,
53+
},
54+
{
55+
name: "select with FROM (not metadata)",
56+
op: proxy.OpQuery,
57+
q: "SELECT 1 FROM dual",
58+
want: true,
59+
},
60+
{
61+
name: "insert not select",
62+
op: proxy.OpQuery,
63+
q: "INSERT INTO users VALUES (1)",
64+
want: false,
65+
},
66+
{
67+
name: "begin op",
68+
op: proxy.OpBegin,
69+
q: "",
70+
want: false,
71+
},
72+
}
73+
74+
for _, tt := range tests {
75+
t.Run(tt.name, func(t *testing.T) {
76+
t.Parallel()
77+
got := isSelectQuery(tt.op, tt.q)
78+
if got != tt.want {
79+
t.Errorf("isSelectQuery(%v, %q) = %v, want %v", tt.op, tt.q, got, tt.want)
80+
}
81+
})
82+
}
83+
}
84+
85+
func TestIsMetadataQuery(t *testing.T) {
86+
t.Parallel()
87+
88+
tests := []struct {
89+
name string
90+
q string
91+
want bool
92+
}{
93+
{
94+
name: "no FROM clause",
95+
q: "SELECT database()",
96+
want: true,
97+
},
98+
{
99+
name: "system variable",
100+
q: "SELECT @@session.transaction_read_only",
101+
want: true,
102+
},
103+
{
104+
name: "constant",
105+
q: "SELECT 1",
106+
want: true,
107+
},
108+
{
109+
name: "has FROM clause",
110+
q: "SELECT id FROM users",
111+
want: false,
112+
},
113+
{
114+
name: "subquery with FROM",
115+
q: "SELECT (SELECT COUNT(*) FROM orders)",
116+
want: false,
117+
},
118+
{
119+
name: "expression-level FROM (Postgres EXTRACT)",
120+
q: "SELECT EXTRACT(EPOCH FROM NOW())",
121+
want: false, // false negative: no table, but FROM in expression
122+
},
123+
}
124+
125+
for _, tt := range tests {
126+
t.Run(tt.name, func(t *testing.T) {
127+
t.Parallel()
128+
got := isMetadataQuery(tt.q)
129+
if got != tt.want {
130+
t.Errorf("isMetadataQuery(%q) = %v, want %v", tt.q, got, tt.want)
131+
}
132+
})
133+
}
134+
}

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ require (
1515
github.com/testcontainers/testcontainers-go/modules/mysql v0.40.0
1616
google.golang.org/grpc v1.79.1
1717
google.golang.org/protobuf v1.36.11
18+
gopkg.in/yaml.v3 v3.0.1
1819
)
1920

2021
require (
@@ -96,5 +97,4 @@ require (
9697
golang.org/x/sys v0.39.0 // indirect
9798
golang.org/x/text v0.34.0 // indirect
9899
google.golang.org/genproto/googleapis/rpc v0.0.0-20260209200024-4cfbd4190f57 // indirect
99-
gopkg.in/yaml.v3 v3.0.1 // indirect
100100
)

‎tui/filter.go‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ const (
1616
filterDuration // d>100ms, d<10ms
1717
filterError // "error" keyword
1818
filterOp // op:select, op:begin, etc.
19+
filterNPlus1 // "n+1" or "nplus1" keyword
20+
filterSlow // "slow" keyword
1921
)
2022

2123
type durationOp int
@@ -70,18 +72,27 @@ func parseFilter(input string) []filterCondition {
7072
conds = append(conds, c)
7173
continue
7274
}
73-
if strings.ToLower(tok) == "error" {
75+
lower := strings.ToLower(tok)
76+
if lower == "error" {
7477
conds = append(conds, filterCondition{kind: filterError})
7578
continue
7679
}
77-
if c, ok := parseOp(tok); ok {
80+
if lower == "n+1" || lower == "nplus1" {
81+
conds = append(conds, filterCondition{kind: filterNPlus1})
82+
continue
83+
}
84+
if lower == "slow" {
85+
conds = append(conds, filterCondition{kind: filterSlow})
86+
continue
87+
}
88+
if c, ok := parseOp(lower); ok {
7889
conds = append(conds, c)
7990
continue
8091
}
8192
// Fallback: plain text match.
8293
conds = append(conds, filterCondition{
8394
kind: filterText,
84-
text: strings.ToLower(tok),
95+
text: lower,
8596
})
8697
}
8798
return conds
@@ -124,8 +135,7 @@ func unitSuffix(unit string) string {
124135
return "ms"
125136
}
126137

127-
func parseOp(tok string) (filterCondition, bool) {
128-
lower := strings.ToLower(tok)
138+
func parseOp(lower string) (filterCondition, bool) {
129139
if !strings.HasPrefix(lower, "op:") {
130140
return filterCondition{}, false
131141
}
@@ -157,6 +167,10 @@ func (c filterCondition) matchesEvent(ev *tapv1.QueryEvent) bool {
157167
}
158168
case filterError:
159169
return ev.GetError() != ""
170+
case filterNPlus1:
171+
return ev.GetNPlus_1()
172+
case filterSlow:
173+
return ev.GetSlowQuery()
160174
case filterOp:
161175
return matchOp(ev, c.opPattern)
162176
}
@@ -203,6 +217,10 @@ func describeFilter(input string) string {
203217
parts = append(parts, "d"+op+c.durValue.String())
204218
case filterError:
205219
parts = append(parts, "error")
220+
case filterNPlus1:
221+
parts = append(parts, "n+1")
222+
case filterSlow:
223+
parts = append(parts, "slow")
206224
case filterOp:
207225
parts = append(parts, "op:"+c.opPattern)
208226
}

‎tui/filter_test.go‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,27 @@ func TestParseFilter(t *testing.T) {
7979
{kind: filterOp, opPattern: "begin"},
8080
},
8181
},
82+
{
83+
name: "n+1 keyword",
84+
input: "n+1",
85+
want: []filterCondition{
86+
{kind: filterNPlus1},
87+
},
88+
},
89+
{
90+
name: "nplus1 keyword",
91+
input: "nplus1",
92+
want: []filterCondition{
93+
{kind: filterNPlus1},
94+
},
95+
},
96+
{
97+
name: "slow keyword",
98+
input: "slow",
99+
want: []filterCondition{
100+
{kind: filterSlow},
101+
},
102+
},
82103
{
83104
name: "combined filter",
84105
input: "op:select d>100ms",
@@ -227,6 +248,38 @@ func TestMatchesEvent(t *testing.T) {
227248
ev: makeEvent(proxy.OpQuery, "INSERT INTO users (name) VALUES ('alice')", 5*time.Millisecond, ""),
228249
want: true,
229250
},
251+
{
252+
name: "n+1 match",
253+
cond: filterCondition{kind: filterNPlus1},
254+
ev: func() *tapv1.QueryEvent {
255+
ev := makeEvent(proxy.OpQuery, "SELECT id FROM users WHERE id = 1", 5*time.Millisecond, "")
256+
ev.NPlus_1 = true
257+
return ev
258+
}(),
259+
want: true,
260+
},
261+
{
262+
name: "n+1 no match",
263+
cond: filterCondition{kind: filterNPlus1},
264+
ev: makeEvent(proxy.OpQuery, "SELECT id FROM users WHERE id = 1", 5*time.Millisecond, ""),
265+
want: false,
266+
},
267+
{
268+
name: "slow match",
269+
cond: filterCondition{kind: filterSlow},
270+
ev: func() *tapv1.QueryEvent {
271+
ev := makeEvent(proxy.OpQuery, "SELECT id FROM users", 500*time.Millisecond, "")
272+
ev.SlowQuery = true
273+
return ev
274+
}(),
275+
want: true,
276+
},
277+
{
278+
name: "slow no match",
279+
cond: filterCondition{kind: filterSlow},
280+
ev: makeEvent(proxy.OpQuery, "SELECT id FROM users", 5*time.Millisecond, ""),
281+
want: false,
282+
},
230283
}
231284

232285
for _, tt := range tests {
@@ -356,6 +409,16 @@ func TestDescribeFilter(t *testing.T) {
356409
input: "error",
357410
want: "error",
358411
},
412+
{
413+
name: "n+1 keyword",
414+
input: "n+1",
415+
want: "n+1",
416+
},
417+
{
418+
name: "slow keyword",
419+
input: "slow",
420+
want: "slow",
421+
},
359422
{
360423
name: "text fallback",
361424
input: "users",

‎web/static/app.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ function parseFilterTokens(input) {
202202
return {kind: 'duration', op, ms};
203203
}
204204
if (tok.toLowerCase() === 'error') return {kind: 'error'};
205+
if (tok.toLowerCase() === 'n+1' || tok.toLowerCase() === 'nplus1') return {kind: 'nplus1'};
206+
if (tok.toLowerCase() === 'slow') return {kind: 'slow'};
205207
const lower = tok.toLowerCase();
206208
if (lower.startsWith('op:') && lower.length > 3) return {kind: 'op', pattern: lower.slice(3)};
207209
return {kind: 'text', text: lower};
@@ -214,6 +216,10 @@ function matchesFilter(ev, cond) {
214216
return cond.op === '>' ? ev.duration_ms > cond.ms : ev.duration_ms < cond.ms;
215217
case 'error':
216218
return !!ev.error;
219+
case 'nplus1':
220+
return !!ev.n_plus_1;
221+
case 'slow':
222+
return !!ev.slow_query;
217223
case 'op':
218224
if (PROTOCOL_OPS.has(cond.pattern)) return ev.op.toLowerCase() === cond.pattern;
219225
if (OP_KEYWORDS.has(cond.pattern)) return (ev.query || '').trim().toLowerCase().startsWith(cond.pattern);

0 commit comments

Comments
 (0)