Phase 2: unified query language spanning ClickHouse and Tantivy
Replaces the separate SQL-only /query and text-only /search endpoints with one pipe-syntax query language (plus raw SQL escape hatch) that compiles to a single IR and execution plan across both backends, so a query like `message:"connection refused" | stats count by host` runs as one request instead of two disjoint tools. - api/internal/querylang: lexer -> ast -> parser -> ir -> planner -> executor, each layer independently tested. - Execution generalizes Phase 1's proven Tantivy-prefilter pattern into a 4-way routing table (pure ClickHouse / text-only / text + aggregation / raw SQL passthrough). - Unified web query page and `sentryctl query`, both hitting the same POST /query endpoint. - Benchmarked against a real 1,022,000-row dataset (hack/benchmark-fixture); caught and fixed a real bug where the Tantivy prefilter cap (10,000) produced an IN-clause exceeding ClickHouse's default max_query_size -- lowered to 5,000, documented in docs/query-language-design.md and docs/phase-2-runbook.md. - docs/query-language-reference.md: customer-facing syntax reference.
This commit is contained in:
@@ -1,60 +0,0 @@
|
||||
package queryapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"reflect"
|
||||
|
||||
"github.com/ClickHouse/clickhouse-go/v2/lib/driver"
|
||||
)
|
||||
|
||||
type QueryResult struct {
|
||||
Columns []string `json:"columns"`
|
||||
Rows [][]any `json:"rows"`
|
||||
}
|
||||
|
||||
// Executor runs arbitrary (pre-validated) SELECT statements against
|
||||
// ClickHouse and shapes the result into JSON-friendly columns/rows,
|
||||
// discovering the result's column set at query time via reflection since
|
||||
// the query itself is arbitrary.
|
||||
type Executor struct {
|
||||
conn driver.Conn
|
||||
}
|
||||
|
||||
func NewExecutor(conn driver.Conn) *Executor {
|
||||
return &Executor{conn: conn}
|
||||
}
|
||||
|
||||
func (e *Executor) Execute(ctx context.Context, sql string) (*QueryResult, error) {
|
||||
rows, err := e.conn.Query(ctx, sql)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("executing query: %w", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
columnTypes := rows.ColumnTypes()
|
||||
result := &QueryResult{
|
||||
Columns: rows.Columns(),
|
||||
Rows: [][]any{},
|
||||
}
|
||||
|
||||
for rows.Next() {
|
||||
dest := make([]any, len(columnTypes))
|
||||
for i, ct := range columnTypes {
|
||||
dest[i] = reflect.New(ct.ScanType()).Interface()
|
||||
}
|
||||
if err := rows.Scan(dest...); err != nil {
|
||||
return nil, fmt.Errorf("scanning row: %w", err)
|
||||
}
|
||||
row := make([]any, len(dest))
|
||||
for i, d := range dest {
|
||||
row[i] = reflect.ValueOf(d).Elem().Interface()
|
||||
}
|
||||
result.Rows = append(result.Rows, row)
|
||||
}
|
||||
if err := rows.Err(); err != nil {
|
||||
return nil, fmt.Errorf("iterating rows: %w", err)
|
||||
}
|
||||
|
||||
return result, nil
|
||||
}
|
||||
@@ -1,13 +1,14 @@
|
||||
// Package queryapi is Sentry's query API: POST /query (Phase 0, a crude
|
||||
// raw-SQL passthrough allowlisted to SELECT) and POST /search (Phase 1,
|
||||
// free-text search via the search service, joined back against
|
||||
// ClickHouse). This is a deliberate simplification of the pinned "gRPC +
|
||||
// REST gateway" control-plane pattern (see CLAUDE.md's tech stack table):
|
||||
// plain net/http REST handlers, not a gRPC service transcoded through
|
||||
// grpc-gateway. That machinery (proto definitions, googleapis
|
||||
// annotations, gateway codegen) doesn't buy much for two crude endpoints
|
||||
// that Phase 2's real SPL-like query layer replaces outright. Revisit
|
||||
// gRPC+gateway once /api's endpoint count and lifespan justify it.
|
||||
// Package queryapi is Sentry's query API: a single POST /query endpoint
|
||||
// accepting either the pipe syntax or raw SQL, compiled by
|
||||
// querylang/planner and executed by querylang/executor. Replaces Phase
|
||||
// 0/1's two separate placeholder endpoints (raw-SQL-only /query,
|
||||
// free-text-only /search) -- see /docs/query-language-design.md.
|
||||
//
|
||||
// Still plain net/http, not the pinned gRPC+REST-gateway pattern, for
|
||||
// the same reason as Phase 0/1: this is one endpoint, and the
|
||||
// proto/annotations/codegen machinery doesn't buy much at that size.
|
||||
// `/api` does speak gRPC internally (to /search) — this simplification
|
||||
// is about the public-facing surface only.
|
||||
package queryapi
|
||||
|
||||
import (
|
||||
@@ -15,38 +16,34 @@ import (
|
||||
"encoding/json"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// queryExecutor is the narrow interface handleQuery depends on, so tests
|
||||
// can substitute a fake without a real ClickHouse connection. *Executor
|
||||
// satisfies it.
|
||||
type queryExecutor interface {
|
||||
Execute(ctx context.Context, sql string) (*QueryResult, error)
|
||||
}
|
||||
"github.com/sentry/sentry/api/internal/querylang/executor"
|
||||
"github.com/sentry/sentry/api/internal/querylang/planner"
|
||||
)
|
||||
|
||||
type Handler struct {
|
||||
logger *slog.Logger
|
||||
exec queryExecutor
|
||||
search searchClient
|
||||
sqlRunner executor.SQLRunner
|
||||
search executor.SearchClient
|
||||
queryTimeout time.Duration
|
||||
allowedOrigin string
|
||||
}
|
||||
|
||||
func NewHandler(logger *slog.Logger, exec queryExecutor, search searchClient, queryTimeout time.Duration, allowedOrigin string) *Handler {
|
||||
return &Handler{logger: logger, exec: exec, search: search, queryTimeout: queryTimeout, allowedOrigin: allowedOrigin}
|
||||
func NewHandler(logger *slog.Logger, sqlRunner executor.SQLRunner, search executor.SearchClient, queryTimeout time.Duration, allowedOrigin string) *Handler {
|
||||
return &Handler{logger: logger, sqlRunner: sqlRunner, search: search, queryTimeout: queryTimeout, allowedOrigin: allowedOrigin}
|
||||
}
|
||||
|
||||
func (h *Handler) Routes() http.Handler {
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("POST /query", h.handleQuery)
|
||||
mux.HandleFunc("POST /search", h.handleSearch)
|
||||
mux.HandleFunc("GET /healthz", h.handleHealthz)
|
||||
return h.withCORS(mux)
|
||||
}
|
||||
|
||||
// withCORS is deliberately permissive by default (see CORSAllowedOrigin in
|
||||
// internal/config) since Phase 0 has no auth and the SvelteKit dev server
|
||||
// internal/config) since there's no auth yet and the SvelteKit dev server
|
||||
// runs on a different origin. Tighten alongside adding real auth.
|
||||
func (h *Handler) withCORS(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -66,14 +63,24 @@ func (h *Handler) handleHealthz(w http.ResponseWriter, _ *http.Request) {
|
||||
}
|
||||
|
||||
type queryRequest struct {
|
||||
SQL string `json:"sql"`
|
||||
Query string `json:"query"`
|
||||
// Language overrides auto-detection ("" / omitted). "sql" or "spl" --
|
||||
// see planner.Language and /docs/query-language-design.md's
|
||||
// "Detection" section for why this exists: the rare case a pipe
|
||||
// query legitimately starts with the literal word "select".
|
||||
Language string `json:"language"`
|
||||
}
|
||||
|
||||
type queryResponse struct {
|
||||
Columns []string `json:"columns"`
|
||||
Rows [][]any `json:"rows"`
|
||||
}
|
||||
|
||||
type errorResponse struct {
|
||||
Error string `json:"error"`
|
||||
}
|
||||
|
||||
// maxBodyBytes caps the request body: a raw SQL string has no legitimate
|
||||
// maxBodyBytes caps the request body: a query string has no legitimate
|
||||
// reason to be larger than this.
|
||||
const maxBodyBytes = 1 << 20 // 1 MiB
|
||||
|
||||
@@ -85,8 +92,19 @@ func (h *Handler) handleQuery(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, http.StatusBadRequest, "invalid JSON body: "+err.Error())
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.Query) == "" {
|
||||
writeError(w, http.StatusBadRequest, "query must not be empty")
|
||||
return
|
||||
}
|
||||
|
||||
if err := validateSelectOnly(req.SQL); err != nil {
|
||||
lang := planner.Language(req.Language)
|
||||
if lang != planner.Auto && lang != planner.SQL && lang != planner.SPL {
|
||||
writeError(w, http.StatusBadRequest, `language must be "sql", "spl", or omitted`)
|
||||
return
|
||||
}
|
||||
|
||||
plan, err := planner.Compile(req.Query, lang, time.Now())
|
||||
if err != nil {
|
||||
writeError(w, http.StatusBadRequest, err.Error())
|
||||
return
|
||||
}
|
||||
@@ -94,14 +112,19 @@ func (h *Handler) handleQuery(w http.ResponseWriter, r *http.Request) {
|
||||
ctx, cancel := context.WithTimeout(r.Context(), h.queryTimeout)
|
||||
defer cancel()
|
||||
|
||||
result, err := h.exec.Execute(ctx, req.SQL)
|
||||
result, err := executor.Execute(ctx, plan, h.sqlRunner, h.search)
|
||||
if err != nil {
|
||||
h.logger.Error("query execution failed", "error", err)
|
||||
h.logger.Error("query execution failed", "query", req.Query, "error", err)
|
||||
writeError(w, http.StatusBadGateway, "query failed: "+err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
writeJSON(w, result)
|
||||
writeJSON(w, queryResponse{Columns: result.Columns, Rows: result.Rows})
|
||||
}
|
||||
|
||||
func writeJSON(w http.ResponseWriter, v any) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(v)
|
||||
}
|
||||
|
||||
func writeError(w http.ResponseWriter, status int, msg string) {
|
||||
|
||||
@@ -11,111 +11,179 @@ import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/sentry/sentry/api/internal/querylang/executor"
|
||||
)
|
||||
|
||||
type fakeExecutor struct {
|
||||
result *QueryResult
|
||||
type fakeSQLRunner struct {
|
||||
result *executor.Result
|
||||
err error
|
||||
gotSQL string
|
||||
}
|
||||
|
||||
func (f *fakeExecutor) Execute(_ context.Context, sql string) (*QueryResult, error) {
|
||||
func (f *fakeSQLRunner) RunSQL(_ context.Context, sql string) (*executor.Result, error) {
|
||||
f.gotSQL = sql
|
||||
if f.err != nil {
|
||||
return nil, f.err
|
||||
}
|
||||
return f.result, nil
|
||||
if f.result != nil {
|
||||
return f.result, nil
|
||||
}
|
||||
return &executor.Result{Columns: []string{}, Rows: [][]any{}}, nil
|
||||
}
|
||||
|
||||
type fakeSearchClient struct {
|
||||
recordIDs []string
|
||||
err error
|
||||
gotQuery string
|
||||
}
|
||||
|
||||
func (f *fakeSearchClient) Search(_ context.Context, _ string, _ uint32) ([]string, error) {
|
||||
func (f *fakeSearchClient) Search(_ context.Context, query string, _ uint32) ([]string, error) {
|
||||
f.gotQuery = query
|
||||
if f.err != nil {
|
||||
return nil, f.err
|
||||
}
|
||||
return f.recordIDs, nil
|
||||
}
|
||||
|
||||
func newTestHandler(exec queryExecutor) *Handler {
|
||||
return newTestHandlerWithSearch(exec, &fakeSearchClient{})
|
||||
func newTestHandler(sqlRunner *fakeSQLRunner, search *fakeSearchClient) *Handler {
|
||||
if search == nil {
|
||||
search = &fakeSearchClient{}
|
||||
}
|
||||
return NewHandler(slog.New(slog.NewTextHandler(io.Discard, nil)), sqlRunner, search, time.Second, "*")
|
||||
}
|
||||
|
||||
func newTestHandlerWithSearch(exec queryExecutor, search searchClient) *Handler {
|
||||
return NewHandler(slog.New(slog.NewTextHandler(io.Discard, nil)), exec, search, time.Second, "*")
|
||||
func postQuery(t *testing.T, h *Handler, body string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest(http.MethodPost, "/query", strings.NewReader(body))
|
||||
rec := httptest.NewRecorder()
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
return rec
|
||||
}
|
||||
|
||||
func TestHandleQuerySuccess(t *testing.T) {
|
||||
fe := &fakeExecutor{result: &QueryResult{
|
||||
func TestHandleQuerySQLSuccess(t *testing.T) {
|
||||
sr := &fakeSQLRunner{result: &executor.Result{
|
||||
Columns: []string{"host", "count"},
|
||||
Rows: [][]any{{"h1", 3}},
|
||||
}}
|
||||
h := newTestHandler(fe)
|
||||
h := newTestHandler(sr, nil)
|
||||
|
||||
body := strings.NewReader(`{"sql": "SELECT host, count(*) FROM logs GROUP BY host"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/query", body)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
rec := postQuery(t, h, `{"query": "SELECT host, count(*) FROM logs GROUP BY host"}`)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var got QueryResult
|
||||
var got queryResponse
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decoding response: %v", err)
|
||||
}
|
||||
if len(got.Columns) != 2 || len(got.Rows) != 1 {
|
||||
t.Fatalf("unexpected result: %+v", got)
|
||||
}
|
||||
if fe.gotSQL != "SELECT host, count(*) FROM logs GROUP BY host" {
|
||||
t.Fatalf("executor received unexpected SQL: %q", fe.gotSQL)
|
||||
if sr.gotSQL != "SELECT host, count(*) FROM logs GROUP BY host" {
|
||||
t.Fatalf("unexpected SQL passed through: %q", sr.gotSQL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryRejectsNonSelect(t *testing.T) {
|
||||
fe := &fakeExecutor{}
|
||||
h := newTestHandler(fe)
|
||||
func TestHandleQueryPipeSyntaxSuccess(t *testing.T) {
|
||||
sr := &fakeSQLRunner{result: &executor.Result{
|
||||
Columns: []string{"host"},
|
||||
Rows: [][]any{{"api"}},
|
||||
}}
|
||||
h := newTestHandler(sr, nil)
|
||||
|
||||
body := strings.NewReader(`{"sql": "DELETE FROM logs"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/query", body)
|
||||
rec := httptest.NewRecorder()
|
||||
rec := postQuery(t, h, `{"query": "service=api"}`)
|
||||
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if !strings.Contains(sr.gotSQL, "`service` = 'api'") {
|
||||
t.Fatalf("expected compiled SQL to filter on service, got: %s", sr.gotSQL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryTextSearchRoutesThroughSearchClient(t *testing.T) {
|
||||
sr := &fakeSQLRunner{}
|
||||
fs := &fakeSearchClient{recordIDs: []string{"id-1"}}
|
||||
h := newTestHandler(sr, fs)
|
||||
|
||||
rec := postQuery(t, h, `{"query": "message:\"connection refused\""}`)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if fs.gotQuery != `"connection refused"` {
|
||||
t.Fatalf("search query = %q", fs.gotQuery)
|
||||
}
|
||||
if !strings.Contains(sr.gotSQL, "record_id IN ('id-1')") {
|
||||
t.Fatalf("expected the search prefilter in the generated SQL, got: %s", sr.gotSQL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryRejectsEmptyQuery(t *testing.T) {
|
||||
h := newTestHandler(&fakeSQLRunner{}, nil)
|
||||
rec := postQuery(t, h, `{"query": " "}`)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("status = %d, want 400", rec.Code)
|
||||
}
|
||||
if fe.gotSQL != "" {
|
||||
t.Fatal("executor should not have been called for a rejected query")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryRejectsInvalidJSON(t *testing.T) {
|
||||
h := newTestHandler(&fakeExecutor{})
|
||||
|
||||
body := strings.NewReader(`not json`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/query", body)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
|
||||
h := newTestHandler(&fakeSQLRunner{}, nil)
|
||||
rec := postQuery(t, h, `not json`)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("status = %d, want 400", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryRejectsCompileError(t *testing.T) {
|
||||
h := newTestHandler(&fakeSQLRunner{}, nil)
|
||||
rec := postQuery(t, h, `{"query": "service=api | bogus"}`)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("status = %d, want 400; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryRejectsNonSelectSQL(t *testing.T) {
|
||||
h := newTestHandler(&fakeSQLRunner{}, nil)
|
||||
rec := postQuery(t, h, `{"query": "DELETE FROM logs", "language": "sql"}`)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("status = %d, want 400", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryRejectsInvalidLanguage(t *testing.T) {
|
||||
h := newTestHandler(&fakeSQLRunner{}, nil)
|
||||
rec := postQuery(t, h, `{"query": "service=api", "language": "cobol"}`)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("status = %d, want 400", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryExplicitLanguageOverridesAutoDetect(t *testing.T) {
|
||||
sr := &fakeSQLRunner{}
|
||||
fs := &fakeSearchClient{recordIDs: []string{"id-1"}}
|
||||
h := newTestHandler(sr, fs)
|
||||
|
||||
// "select" alone would auto-detect as (nonsensical but
|
||||
// syntactically-valid-looking) SQL without the override -- the
|
||||
// override forces pipe-syntax parsing instead, where a bare word
|
||||
// with no comparator is a free-text search term.
|
||||
rec := postQuery(t, h, `{"query": "select", "language": "spl"}`)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if fs.gotQuery != "select" {
|
||||
t.Fatalf("expected 'select' to be treated as a free-text search term, got query=%q", fs.gotQuery)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleQueryExecutorErrorReturnsBadGateway(t *testing.T) {
|
||||
fe := &fakeExecutor{err: errors.New("boom")}
|
||||
h := newTestHandler(fe)
|
||||
sr := &fakeSQLRunner{err: errors.New("boom")}
|
||||
h := newTestHandler(sr, nil)
|
||||
|
||||
body := strings.NewReader(`{"sql": "SELECT 1"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/query", body)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
rec := postQuery(t, h, `{"query": "SELECT 1"}`)
|
||||
|
||||
if rec.Code != http.StatusBadGateway {
|
||||
t.Fatalf("status = %d, want 502", rec.Code)
|
||||
@@ -123,7 +191,7 @@ func TestHandleQueryExecutorErrorReturnsBadGateway(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestHandleHealthz(t *testing.T) {
|
||||
h := newTestHandler(&fakeExecutor{})
|
||||
h := newTestHandler(&fakeSQLRunner{}, nil)
|
||||
req := httptest.NewRequest(http.MethodGet, "/healthz", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
@@ -135,7 +203,7 @@ func TestHandleHealthz(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestCORSPreflight(t *testing.T) {
|
||||
h := newTestHandler(&fakeExecutor{})
|
||||
h := newTestHandler(&fakeSQLRunner{}, nil)
|
||||
req := httptest.NewRequest(http.MethodOptions, "/query", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
|
||||
@@ -1,96 +0,0 @@
|
||||
package queryapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// searchClient is the narrow interface handleSearch depends on, so tests
|
||||
// can substitute a fake without a real search service. A small gRPC
|
||||
// adapter in cmd/api satisfies this.
|
||||
type searchClient interface {
|
||||
Search(ctx context.Context, query string, limit uint32) ([]string, error)
|
||||
}
|
||||
|
||||
type searchRequest struct {
|
||||
Query string `json:"query"`
|
||||
Limit uint32 `json:"limit"`
|
||||
}
|
||||
|
||||
func (h *Handler) handleSearch(w http.ResponseWriter, r *http.Request) {
|
||||
r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes)
|
||||
|
||||
var req searchRequest
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
writeError(w, http.StatusBadRequest, "invalid JSON body: "+err.Error())
|
||||
return
|
||||
}
|
||||
if strings.TrimSpace(req.Query) == "" {
|
||||
writeError(w, http.StatusBadRequest, "query must not be empty")
|
||||
return
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(r.Context(), h.queryTimeout)
|
||||
defer cancel()
|
||||
|
||||
recordIDs, err := h.search.Search(ctx, req.Query, req.Limit)
|
||||
if err != nil {
|
||||
h.logger.Error("search failed", "error", err)
|
||||
writeError(w, http.StatusBadGateway, "search failed: "+err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
if len(recordIDs) == 0 {
|
||||
writeJSON(w, &QueryResult{Columns: []string{}, Rows: [][]any{}})
|
||||
return
|
||||
}
|
||||
|
||||
sql, err := recordIDsQuery(recordIDs)
|
||||
if err != nil {
|
||||
h.logger.Error("building record_id query", "error", err)
|
||||
writeError(w, http.StatusBadGateway, "search returned unusable results")
|
||||
return
|
||||
}
|
||||
|
||||
result, err := h.exec.Execute(ctx, sql)
|
||||
if err != nil {
|
||||
h.logger.Error("joining search results against clickhouse failed", "error", err)
|
||||
writeError(w, http.StatusBadGateway, "query failed: "+err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
writeJSON(w, result)
|
||||
}
|
||||
|
||||
// recordIDsQuery builds a SELECT ... WHERE record_id IN (...) against the
|
||||
// IDs the search service returned. Every ID is validated as a real UUID
|
||||
// before being embedded in the query string -- record_ids come from an
|
||||
// internal, trusted service (not raw user input), but a UUID that fails
|
||||
// to parse can't contain SQL-breaking characters either way, so this is
|
||||
// defense in depth, not a response to a specific threat.
|
||||
func recordIDsQuery(recordIDs []string) (string, error) {
|
||||
quoted := make([]string, 0, len(recordIDs))
|
||||
for _, id := range recordIDs {
|
||||
if _, err := uuid.Parse(id); err != nil {
|
||||
continue // skip anything not a valid UUID rather than failing the whole query
|
||||
}
|
||||
quoted = append(quoted, "'"+id+"'")
|
||||
}
|
||||
if len(quoted) == 0 {
|
||||
return "", fmt.Errorf("no valid record_ids in search response")
|
||||
}
|
||||
return fmt.Sprintf(
|
||||
"SELECT * FROM logs WHERE record_id IN (%s) ORDER BY timestamp DESC",
|
||||
strings.Join(quoted, ","),
|
||||
), nil
|
||||
}
|
||||
|
||||
func writeJSON(w http.ResponseWriter, v any) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_ = json.NewEncoder(w).Encode(v)
|
||||
}
|
||||
@@ -1,125 +0,0 @@
|
||||
package queryapi
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestHandleSearchSuccess(t *testing.T) {
|
||||
id := "5754b062-ec8b-45b1-b1b8-a50f263adcd3"
|
||||
fe := &fakeExecutor{result: &QueryResult{
|
||||
Columns: []string{"message"},
|
||||
Rows: [][]any{{"hello world"}},
|
||||
}}
|
||||
fs := &fakeSearchClient{recordIDs: []string{id}}
|
||||
h := newTestHandlerWithSearch(fe, fs)
|
||||
|
||||
body := strings.NewReader(`{"query": "hello"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/search", body)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if !strings.Contains(fe.gotSQL, id) {
|
||||
t.Fatalf("expected the record_id in the generated SQL, got %q", fe.gotSQL)
|
||||
}
|
||||
if !strings.Contains(fe.gotSQL, "WHERE record_id IN") {
|
||||
t.Fatalf("expected an IN clause, got %q", fe.gotSQL)
|
||||
}
|
||||
|
||||
var got QueryResult
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decoding response: %v", err)
|
||||
}
|
||||
if len(got.Rows) != 1 {
|
||||
t.Fatalf("unexpected result: %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSearchRejectsEmptyQuery(t *testing.T) {
|
||||
fe := &fakeExecutor{}
|
||||
fs := &fakeSearchClient{}
|
||||
h := newTestHandlerWithSearch(fe, fs)
|
||||
|
||||
body := strings.NewReader(`{"query": " "}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/search", body)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("status = %d, want 400", rec.Code)
|
||||
}
|
||||
if fe.gotSQL != "" {
|
||||
t.Fatal("executor should not have been called for an empty query")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSearchNoResultsReturnsEmptyNotError(t *testing.T) {
|
||||
fe := &fakeExecutor{}
|
||||
fs := &fakeSearchClient{recordIDs: nil}
|
||||
h := newTestHandlerWithSearch(fe, fs)
|
||||
|
||||
body := strings.NewReader(`{"query": "nothing matches this"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/search", body)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if fe.gotSQL != "" {
|
||||
t.Fatal("executor should not have been called when search returns no IDs")
|
||||
}
|
||||
|
||||
var got QueryResult
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decoding response: %v", err)
|
||||
}
|
||||
if len(got.Rows) != 0 {
|
||||
t.Fatalf("expected empty rows, got %+v", got.Rows)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleSearchServiceErrorReturnsBadGateway(t *testing.T) {
|
||||
fe := &fakeExecutor{}
|
||||
fs := &fakeSearchClient{err: errors.New("search service unreachable")}
|
||||
h := newTestHandlerWithSearch(fe, fs)
|
||||
|
||||
body := strings.NewReader(`{"query": "hello"}`)
|
||||
req := httptest.NewRequest(http.MethodPost, "/search", body)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
h.Routes().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusBadGateway {
|
||||
t.Fatalf("status = %d, want 502", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRecordIDsQuerySkipsInvalidUUIDs(t *testing.T) {
|
||||
sql, err := recordIDsQuery([]string{"not-a-uuid", "5754b062-ec8b-45b1-b1b8-a50f263adcd3"})
|
||||
if err != nil {
|
||||
t.Fatalf("recordIDsQuery() error = %v", err)
|
||||
}
|
||||
if strings.Contains(sql, "not-a-uuid") {
|
||||
t.Fatalf("expected the invalid UUID to be skipped, got %q", sql)
|
||||
}
|
||||
if !strings.Contains(sql, "5754b062-ec8b-45b1-b1b8-a50f263adcd3") {
|
||||
t.Fatalf("expected the valid UUID to be included, got %q", sql)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRecordIDsQueryAllInvalidReturnsError(t *testing.T) {
|
||||
if _, err := recordIDsQuery([]string{"not-a-uuid", "also-not-one"}); err == nil {
|
||||
t.Fatal("expected an error when no IDs are valid UUIDs")
|
||||
}
|
||||
}
|
||||
@@ -1,47 +0,0 @@
|
||||
package queryapi
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"regexp"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// disallowedKeyword is defense-in-depth on top of the SELECT-only gate: it
|
||||
// catches mutating/administrative statements appearing anywhere in the
|
||||
// query (e.g. smuggled into a subquery), not just at the start. This is
|
||||
// word-boundary matching, not a real SQL parser.
|
||||
var disallowedKeyword = regexp.MustCompile(`(?i)\b(insert|update|delete|alter|drop|truncate|create|grant|revoke|attach|detach|rename|kill|optimize|system|set|exchange|watch)\b`)
|
||||
|
||||
// validateSelectOnly enforces the Phase 0 query API contract: exactly one
|
||||
// SELECT statement and nothing else. This is "basic injection guarding" as
|
||||
// specced, not a SQL parser: it will reject some unusual-but-valid SELECTs
|
||||
// (e.g. one that references a column literally named "delete") and will
|
||||
// not catch every possible abuse (e.g. a syntactically pure SELECT that's
|
||||
// simply expensive to run). Both are acceptable for a Phase 0 placeholder
|
||||
// that's explicitly superseded by a real query layer in Phase 2 — see
|
||||
// /docs/architecture.md.
|
||||
func validateSelectOnly(sql string) error {
|
||||
trimmed := strings.TrimSpace(sql)
|
||||
if trimmed == "" {
|
||||
return errors.New("query must not be empty")
|
||||
}
|
||||
|
||||
trimmed = strings.TrimSpace(strings.TrimSuffix(trimmed, ";"))
|
||||
if trimmed == "" {
|
||||
return errors.New("query must not be empty")
|
||||
}
|
||||
if strings.Contains(trimmed, ";") {
|
||||
return errors.New("only a single statement is allowed")
|
||||
}
|
||||
|
||||
firstWord := strings.ToUpper(strings.Fields(trimmed)[0])
|
||||
if firstWord != "SELECT" {
|
||||
return errors.New("only SELECT queries are allowed")
|
||||
}
|
||||
|
||||
if disallowedKeyword.MatchString(trimmed) {
|
||||
return errors.New("query contains a disallowed keyword")
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
@@ -1,38 +0,0 @@
|
||||
package queryapi
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestValidateSelectOnly(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
sql string
|
||||
wantErr bool
|
||||
}{
|
||||
{"plain select", "SELECT * FROM logs LIMIT 10", false},
|
||||
{"lowercase select", "select service, count(*) from logs group by service", false},
|
||||
{"trailing semicolon allowed", "SELECT 1;", false},
|
||||
{"trailing semicolon and whitespace allowed", "SELECT 1; ", false},
|
||||
{"empty", "", true},
|
||||
{"whitespace only", " ", true},
|
||||
{"only a semicolon", ";", true},
|
||||
{"multiple statements", "SELECT 1; SELECT 2", true},
|
||||
{"insert", "INSERT INTO logs VALUES (1)", true},
|
||||
{"delete", "DELETE FROM logs", true},
|
||||
{"drop", "DROP TABLE logs", true},
|
||||
{"select with drop keyword smuggled in", "SELECT * FROM logs WHERE message = 'DROP TABLE logs'", true},
|
||||
{"non-select start", "WITH x AS (SELECT 1) SELECT * FROM x", true},
|
||||
{"trailing garbage after semicolon", "SELECT 1; DROP TABLE logs", true},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
err := validateSelectOnly(tc.sql)
|
||||
if tc.wantErr && err == nil {
|
||||
t.Errorf("validateSelectOnly(%q) = nil, want error", tc.sql)
|
||||
}
|
||||
if !tc.wantErr && err != nil {
|
||||
t.Errorf("validateSelectOnly(%q) = %v, want nil", tc.sql, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user