This is an automated email from the ASF dual-hosted git repository. mrproliu pushed a commit to branch bydb-parameter in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
commit 524825475457372b30e0e06a265768e10fe0ca26 Author: mrproliu <[email protected]> AuthorDate: Tue Jul 7 16:21:42 2026 +0800 Introduce parameter binding (`?` placeholders) into BydbQL --- CHANGES.md | 1 + api/proto/banyandb/bydbql/v1/query.proto | 8 + banyand/liaison/grpc/bydbql.go | 5 +- banyand/liaison/grpc/bydbql_test.go | 90 ++++ docs/api-reference.md | 1 + docs/interacting/bydbql.md | 73 +++ mcp/src/client/index.ts | 7 +- mcp/src/client/types.ts | 12 + mcp/src/query/llm-prompt.ts | 1 + mcp/src/query/validation.ts | 82 ++++ mcp/src/server/mcp.ts | 28 +- pkg/bydbql/binder.go | 311 +++++++++++++ pkg/bydbql/binder_equivalence_test.go | 413 +++++++++++++++++ pkg/bydbql/binder_test.go | 573 ++++++++++++++++++++++++ pkg/bydbql/grammar.go | 16 +- pkg/bydbql/parser.go | 1 + pkg/bydbql/transformer.go | 5 + pkg/test/helpers/bydbql.go | 63 +++ test/cases/measure/data/data.go | 18 +- test/cases/measure/data/input/params_bind.ql | 22 + test/cases/measure/data/input/params_bind.yaml | 30 ++ test/cases/measure/measure.go | 1 + test/cases/property/data/data.go | 18 +- test/cases/property/data/input/params_bind.ql | 21 + test/cases/property/data/input/params_bind.yaml | 30 ++ test/cases/property/property.go | 1 + test/cases/stream/data/data.go | 16 +- test/cases/stream/data/input/params_bind.ql | 21 + test/cases/stream/data/input/params_bind.yaml | 40 ++ test/cases/stream/stream.go | 1 + test/cases/topn/data/data.go | 16 +- test/cases/topn/topn.go | 1 + test/cases/trace/data/data.go | 18 +- test/cases/trace/data/input/params_bind.ql | 24 + test/cases/trace/data/input/params_bind.yml | 40 ++ test/cases/trace/trace.go | 2 + test/cases/tracepipeline/data/data.go | 18 +- ui/src/components/BydbQL/Index.vue | 135 +++++- 38 files changed, 2077 insertions(+), 86 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index bcaee768d..57f6be0a6 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -74,6 +74,7 @@ Release Notes. - Add the storage-node in-merge trace retention filter (`PIPELINE_EVENT_MERGE`): a per-group sampler chain evaluates traces at merge time and drops non-retained traces from both the core part and the coupled sidx (`sidx.Merge` gains a keep predicate; a nil predicate restores the wholesale merge). Chain execution fails open on panic/error/length-mismatch/timeout, with a consecutive-timeout circuit breaker. Samplers are native Go plugins (`.so`) loaded at startup from `--trace-pipeline-con [...] - Register/update/remove in-merge trace retention samplers at runtime without restarting the data node, replacing the static `--trace-pipeline-config` boot loader (now removed). The sampler pipeline config is carried on `common.v1.Group` (new optional `pipeline` field, with `TracePipelineConfig`/`SamplerPlugin` relocated from `pipeline/v1` into the leaf `common/v1` to avoid a `common`→`pipeline` import cycle) and delivered through the existing `KindGroup` schema watch; the data node rebu [...] - Add FODC memory-pressure pprof capture: agent grabs heap/goroutine profiles when RSS nears the cgroup limit, served via the proxy. +- Introduce positional parameter binding (`?` placeholders) into BydbQL to eliminate QL injection. ### Bug Fixes diff --git a/api/proto/banyandb/bydbql/v1/query.proto b/api/proto/banyandb/bydbql/v1/query.proto index 66ff0324d..2a0c5ee84 100644 --- a/api/proto/banyandb/bydbql/v1/query.proto +++ b/api/proto/banyandb/bydbql/v1/query.proto @@ -21,6 +21,7 @@ package banyandb.bydbql.v1; import "banyandb/measure/v1/query.proto"; import "banyandb/measure/v1/topn.proto"; +import "banyandb/model/v1/common.proto"; import "banyandb/property/v1/rpc.proto"; import "banyandb/stream/v1/query.proto"; import "banyandb/trace/v1/query.proto"; @@ -33,6 +34,13 @@ option java_package = "org.apache.skywalking.banyandb.bydbql.v1"; message QueryRequest { // query is the BydbQL query string string query = 1 [(validate.rules).string.min_len = 1]; + // params are bound to the `?` placeholders in the query string by their order of appearance. + // The number of params must match the number of placeholders. + // Accepted variants depend on the placeholder's position: str/timestamp for TIME values, + // str/int/null for scalar comparisons, str/int/null/str_array/int_array for IN/MATCH/HAVING + // value lists (arrays expand in place), and int for LIMIT/OFFSET/TOP N counts. + // binary_data is rejected everywhere. + repeated model.v1.TagValue params = 2; } // QueryResponse contains the result of a BydbQL query diff --git a/banyand/liaison/grpc/bydbql.go b/banyand/liaison/grpc/bydbql.go index a1d9c137f..e22420736 100644 --- a/banyand/liaison/grpc/bydbql.go +++ b/banyand/liaison/grpc/bydbql.go @@ -80,12 +80,15 @@ func (b *bydbQLService) Query(ctx context.Context, req *bydbqlv1.QueryRequest) ( } }() - // parse query and transform to native request + // parse query, bind parameters, and transform to native request parseStart := time.Now() query, err := bydbql.ParseQuery(req.Query) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "failed to parse query: %v", err) } + if err = bydbql.BindParams(query, req.Params); err != nil { + return nil, status.Errorf(codes.InvalidArgument, "failed to bind parameters: %v", err) + } result, err := b.transformer.Transform(ctx, query) if err != nil { return nil, status.Errorf(codes.Internal, "failed to transform to native request: %v", err) diff --git a/banyand/liaison/grpc/bydbql_test.go b/banyand/liaison/grpc/bydbql_test.go new file mode 100644 index 000000000..2d91de7f2 --- /dev/null +++ b/banyand/liaison/grpc/bydbql_test.go @@ -0,0 +1,90 @@ +// Licensed to Apache Software Foundation (ASF) under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Apache Software Foundation (ASF) licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package grpc + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + bydbqlv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/bydbql/v1" + modelv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/model/v1" +) + +func newTestBydbQLService() *bydbQLService { + return &bydbQLService{metrics: newBypassMetrics()} +} + +func bydbqlStrParam(v string) *modelv1.TagValue { + return &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: v}}} +} + +func TestBydbQLQuery_ParseError_ReturnsInvalidArgument(t *testing.T) { + svc := newTestBydbQLService() + _, err := svc.Query(context.Background(), &bydbqlv1.QueryRequest{Query: "NOT A QUERY"}) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) + assert.Contains(t, st.Message(), "failed to parse query") +} + +func TestBydbQLQuery_MissingParams_ReturnsInvalidArgument(t *testing.T) { + svc := newTestBydbQLService() + _, err := svc.Query(context.Background(), &bydbqlv1.QueryRequest{ + Query: "SELECT * FROM STREAM sw IN default WHERE service_id = ?", + }) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) + assert.Contains(t, st.Message(), "failed to bind parameters") + assert.Contains(t, st.Message(), "parameter count mismatch") +} + +func TestBydbQLQuery_ExtraParams_ReturnsInvalidArgument(t *testing.T) { + svc := newTestBydbQLService() + _, err := svc.Query(context.Background(), &bydbqlv1.QueryRequest{ + Query: "SELECT * FROM STREAM sw IN default", + Params: []*modelv1.TagValue{bydbqlStrParam("unused")}, + }) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) + assert.Contains(t, st.Message(), "0 placeholder(s) but 1 parameter(s)") +} + +func TestBydbQLQuery_ParamTypeMismatch_ReturnsInvalidArgument(t *testing.T) { + svc := newTestBydbQLService() + _, err := svc.Query(context.Background(), &bydbqlv1.QueryRequest{ + Query: "SELECT * FROM STREAM sw IN default WHERE service_id = ?", + Params: []*modelv1.TagValue{ + {Value: &modelv1.TagValue_StrArray{StrArray: &modelv1.StrArray{Value: []string{"a", "b"}}}}, + }, + }) + require.Error(t, err) + st, ok := status.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code()) + assert.Contains(t, st.Message(), "failed to bind parameters") +} diff --git a/docs/api-reference.md b/docs/api-reference.md index bc91b28c7..e9093d351 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -2117,6 +2117,7 @@ QueryRequest is the main request message for BydbQL queries | Field | Type | Label | Description | | ----- | ---- | ----- | ----------- | | query | [string](#string) | | query is the BydbQL query string | +| params | [banyandb.model.v1.TagValue](#banyandb-model-v1-TagValue) | repeated | params are bound to the `?` placeholders in the query string by their order of appearance. The number of params must match the number of placeholders. Accepted variants depend on the placeholder's position: str/timestamp for TIME values, str/int/null for scalar comparisons, str/int/null/str_array/int_array for IN/MATCH/HAVING value lists (arrays expand in place), and int for LIMIT/OFFSET/TOP N counts. b [...] diff --git a/docs/interacting/bydbql.md b/docs/interacting/bydbql.md index c6917ac75..257432854 100644 --- a/docs/interacting/bydbql.md +++ b/docs/interacting/bydbql.md @@ -34,11 +34,14 @@ BydbQL Query String ↓ Abstract Syntax Tree (AST) ↓ + Parameter Binding + ↓ Transformer ``` - **Lexer**: Breaks the query string into a sequence of tokens. - **Parser**: Builds an Abstract Syntax Tree (AST) from the tokens, validating the query's syntax. +- **Parameter Binding**: Fills the `?` placeholders in the AST from the request's `params` (see section 2.6); it always runs, so a query holding unbound placeholders is rejected. - **Transformer**: Traverses the AST, performs semantic analysis using a schema, and transforms the AST into the appropriate target protobuf `Stream/Measure/Property/Traces/TopN Request` message. ### 2.2. Distinguishing Query Types @@ -154,6 +157,76 @@ TIME < '-1d' The `parseTime` function automatically determines whether a timestamp is absolute (RFC3339) or relative (duration string) and converts it appropriately. +## 2.6. Parameter Binding + +BydbQL supports positional parameter binding with the `?` placeholder, so untrusted values never need to be concatenated into the query string. This eliminates QL injection: a bound value is always treated as a plain value and can never alter the query structure. + +```sql +SELECT trace_id, message +FROM STREAM logs IN group1 +TIME > ? +WHERE message MATCH(?) +``` + +Parameters are supplied alongside the query and are bound to the placeholders in their order of appearance. Each parameter is a `model.v1.TagValue`. Over HTTP: + +```json +POST /api/v1/bydbql/query +{ + "query": "SELECT trace_id, message FROM STREAM logs IN group1 TIME > ? WHERE message MATCH(?)", + "params": [ + {"str": {"value": "-30m"}}, + {"str": {"value": "error"}} + ] +} +``` + +### 2.6.1. Supported Positions + +Placeholders are accepted in *value* positions only. The complete list, with the parameter types each position accepts: + +| Position | Example | Accepted parameter types | +|---|---|---| +| `TIME` comparison value | `TIME > ?` | `str` (RFC3339, relative like `-30m`, or `now`), `timestamp` | +| `TIME BETWEEN` boundaries | `TIME BETWEEN ? AND ?` | same as above; each boundary binds independently | +| Scalar comparison value (`=`, `!=`, `>`, `>=`, `<`, `<=`) | `WHERE service_id = ?` | `str`, `int`, `null` | +| `IN` / `NOT IN` value list | `IN (?, ?)`, `IN (?)`, `IN ('a', ?)` | `str`, `int`, `null`, `str_array`, `int_array` | +| `MATCH` value list | `MATCH(?)`, `MATCH(?, 'analyzer')` | `str`, `int`, `null`, `str_array`, `int_array` | +| `HAVING` / `NOT HAVING` values | `HAVING ?`, `HAVING (?)` | `str`, `int`, `null`, `str_array`, `int_array` | +| `LIMIT` count | `LIMIT ?` | `int` only | +| `OFFSET` count | `OFFSET ?` | `int` only | +| `SHOW TOP N` count | `SHOW TOP ? FROM MEASURE ...` | `int` only | +| `SELECT TOP N` projection count | `SELECT TOP ? field ...` | `int` only | + +Array expansion: an array parameter bound inside a value list expands in place — `WHERE service_id IN (?)` with a three-entry `str_array` behaves exactly like listing the three strings literally, and literals can be mixed around it (`IN ('a', ?, 'b')`). The container keeps the form a literal query would have: a single-value `HAVING ?` stays a single value unless the array holds multiple entries, while a parenthesized `HAVING (?)` stays a list regardless of the entry count. + +### 2.6.2. Unsupported Positions + +Placeholders in any position that determines the query *structure* rather than a compared *value* are rejected at parse time with a syntax error. Bound parameters therefore can never change the shape of a query, which is what makes binding injection-proof. + +| Category | Positions | Example (all syntax errors) | +|---|---|---| +| Resource identifiers | resource name, group name, stage name | `FROM STREAM ?`, `IN ?`, `ON ? STAGES` | +| Column identifiers | projection columns, condition left-hand side, `ORDER BY` / `GROUP BY` columns, aggregate function arguments | `SELECT ?`, `WHERE ? = 'a'`, `ORDER BY ?`, `SUM(?)` | +| Function options | `MATCH` analyzer and operator options | `MATCH('x', ?)` | +| Structure | operators, `ASC`/`DESC`, keywords, doubled placeholders | `= ??` | + +Note that unlike some SQL implementations where `ORDER BY ?` silently degrades to a no-op constant sort, BydbQL rejects it outright. + +### 2.6.3. Type Rules Summary + +- `binary_data` parameters are rejected in every position (BydbQL has no binary literal). +- `timestamp` parameters are accepted only in `TIME` positions and are validated: a missing or out-of-range timestamp is rejected at bind time. +- An `int` parameter in a `TIME` position binds (matching an integer literal), but the transformer cannot parse a bare integer as a timestamp, so the query fails; use `str` or `timestamp` instead. +- `int` parameters in `LIMIT`/`OFFSET`/`TOP N` positions must be within `[0, 2147483647]`; out-of-range values are rejected at bind time instead of silently wrapping. +- Empty arrays, parameters without a value, and `nil` entries are rejected everywhere. +- A `?` inside a quoted string literal is part of the string, not a placeholder: `WHERE msg = 'why?'` consumes no parameter. +- After binding passes, values are checked against the tag schema by the same rules as literals: a `str` that cannot parse as an integer fails against an `INT` tag, while an `int` bound to a `STRING` tag is converted, exactly as if written literally. + +### 2.6.4. Errors + +The request fails with `InvalidArgument` when the number of parameters does not match the number of placeholders, or when a parameter type is not accepted at its placeholder's position. Error messages include the 1-based position of the offending parameter (the first `?` is parameter #1). + ## 3. WHERE Statement The WHERE clause in BydbQL provides powerful filtering capabilities with support for various operators, logical expressions, and full-text search. It supports binary tree structures with proper operator precedence and parentheses grouping. diff --git a/mcp/src/client/index.ts b/mcp/src/client/index.ts index d94ff1a71..7b2b1d57d 100644 --- a/mcp/src/client/index.ts +++ b/mcp/src/client/index.ts @@ -17,7 +17,7 @@ * under the License. */ -import type { QueryRequest, QueryResponse, Group, ResourceMetadata } from './types.js'; +import type { QueryRequest, QueryResponse, Group, ResourceMetadata, TagValueParam } from './types.js'; import { httpFetch } from '../utils/http.js'; const maxErrorBodyLength = 2048; @@ -69,10 +69,13 @@ export class BanyanDBClient { /** * Execute a BydbQL query and return the result as a formatted string. */ - async query(bydbqlQuery: string): Promise<string> { + async query(bydbqlQuery: string, params?: TagValueParam[]): Promise<string> { const request: QueryRequest = { query: bydbqlQuery, }; + if (params && params.length > 0) { + request.params = params; + } const url = `${this.baseUrl}/v1/bydbql/query`; diff --git a/mcp/src/client/types.ts b/mcp/src/client/types.ts index d643a8c71..635df6488 100644 --- a/mcp/src/client/types.ts +++ b/mcp/src/client/types.ts @@ -74,8 +74,20 @@ interface TopNResult { lists?: unknown[]; } +/** + * A single BydbQL parameter in protojson TagValue form, + * bound to a `?` placeholder by order of appearance. + */ +export type TagValueParam = + | { str: { value: string } } + | { int: { value: string } } + | { strArray: { value: string[] } } + | { intArray: { value: string[] } } + | { null: null }; + export interface QueryRequest { query: string; + params?: TagValueParam[]; } export interface QueryResponse { diff --git a/mcp/src/query/llm-prompt.ts b/mcp/src/query/llm-prompt.ts index 8cbdc3d85..6b133abab 100644 --- a/mcp/src/query/llm-prompt.ts +++ b/mcp/src/query/llm-prompt.ts @@ -229,6 +229,7 @@ Processing Rules: - LIMIT clause: Extract ONLY when requesting number of data points/results (NOT time ranges) - AGGREGATE BY: Extract from keywords or preserve explicit clauses - ORDER BY: Extract ONLY when explicitly requested, BUT MUST validate field name against indexed fields list +- Parameter Binding: BydbQL supports "?" placeholders in value positions (TIME values, WHERE/HAVING comparison values, IN/MATCH/HAVING value lists, and LIMIT/OFFSET/TOP N counts as int). When the query embeds untrusted or user-provided values, prefer placeholders and pass the values through the "params" argument of list_resources_bydbql instead of concatenating them into the query string. - CRITICAL Rules: - Do NOT add ORDER BY or LIMIT unless explicitly requested in the description - If description only mentions a time range (e.g., "list the last 3 days"), use ONLY TIME clause, no ORDER BY, no LIMIT diff --git a/mcp/src/query/validation.ts b/mcp/src/query/validation.ts index 2d91811c4..fd9e60f3f 100644 --- a/mcp/src/query/validation.ts +++ b/mcp/src/query/validation.ts @@ -15,6 +15,8 @@ * limitations under the License. */ +import type { TagValueParam } from '../client/types.js'; + const maxDescriptionLength = 2048; const maxIdentifierLength = 256; const maxBydbQLLength = 4096; @@ -24,9 +26,21 @@ const allowedQueryHintResourceTypes = ['stream', 'measure', 'trace', 'property'] const disallowedQueryTokenPatterns = [/[;]/, /--/, /\/\*/, /\*\//]; const allowedQueryPrefixPatterns = [/^\s*SELECT\b/i, /^\s*SHOW\s+TOP\b/i]; +const maxParamCount = 64; +const maxParamValueLength = 4096; +const maxParamArrayLength = 256; +const allowedParamTypes = ['str', 'int', 'str_array', 'int_array', 'null'] as const; +const integerPattern = /^-?\d+$/; + +export type BydbQLParam = { + type: (typeof allowedParamTypes)[number]; + value?: unknown; +}; + export type QueryHints = { description?: string; BydbQL?: string; + params?: BydbQLParam[]; resource_type?: string; resource_name?: string; group?: string; @@ -119,12 +133,77 @@ export function normalizeQueryHints(args: unknown): QueryHints { return { description: typeof rawArgs.description === 'string' ? rawArgs.description.trim() : undefined, BydbQL: typeof rawArgs.BydbQL === 'string' ? rawArgs.BydbQL.trim() : undefined, + params: Array.isArray(rawArgs.params) ? (rawArgs.params as BydbQLParam[]) : undefined, resource_type: typeof rawArgs.resource_type === 'string' ? rawArgs.resource_type.trim() : undefined, resource_name: typeof rawArgs.resource_name === 'string' ? rawArgs.resource_name.trim() : undefined, group: typeof rawArgs.group === 'string' ? rawArgs.group.trim() : undefined, }; } +function validateParamString(value: unknown, position: number): string { + if (typeof value !== 'string') { + throw new Error(`params[${position}]: str value must be a string`); + } + if (value.length > maxParamValueLength) { + throw new Error(`params[${position}]: value exceeds ${maxParamValueLength} characters`); + } + return value; +} + +function validateParamInteger(value: unknown, position: number): string { + if (typeof value === 'number' && Number.isSafeInteger(value)) { + return String(value); + } + if (typeof value === 'string' && integerPattern.test(value)) { + return value; + } + throw new Error(`params[${position}]: int value must be an integer`); +} + +function validateParamArray(value: unknown, position: number): unknown[] { + if (!Array.isArray(value) || value.length === 0) { + throw new Error(`params[${position}]: array value must be a non-empty array`); + } + if (value.length > maxParamArrayLength) { + throw new Error(`params[${position}]: array value exceeds ${maxParamArrayLength} entries`); + } + return value; +} + +/** + * Validate BydbQL parameters and convert them to protojson TagValue form + * accepted by the BanyanDB HTTP API. + * + * Keep in sync with `buildBoundParams` in ui/src/components/BydbQL/Index.vue, + * which builds the same shapes independently (no shared package exists between + * mcp/ and ui/). The server-side `timestamp` variant is deliberately not exposed + * here: an RFC3339 `str` is equivalent for TIME positions. + */ +export function toTagValueParams(params: BydbQLParam[]): TagValueParam[] { + if (params.length > maxParamCount) { + throw new Error(`params exceeds ${maxParamCount} entries`); + } + return params.map((param, position) => { + if (!param || typeof param !== 'object' || typeof param.type !== 'string') { + throw new Error(`params[${position}]: each parameter must be an object with a "type" field`); + } + switch (param.type) { + case 'str': + return { str: { value: validateParamString(param.value, position) } }; + case 'int': + return { int: { value: validateParamInteger(param.value, position) } }; + case 'str_array': + return { strArray: { value: validateParamArray(param.value, position).map((entry) => validateParamString(entry, position)) } }; + case 'int_array': + return { intArray: { value: validateParamArray(param.value, position).map((entry) => validateParamInteger(entry, position)) } }; + case 'null': + return { null: null }; + default: + throw new Error(`params[${position}]: type must be one of: ${allowedParamTypes.join(', ')}`); + } + }); +} + export function validateListGroupsArgs(args: unknown): ListGroupsArgs { if (!args || typeof args !== 'object') { throw new Error('tool arguments must be an object'); @@ -161,6 +240,9 @@ export function validateQueryHints(queryHints: QueryHints): QueryHints { if (queryHints.BydbQL) { validatedHints.BydbQL = validateBydbQL(queryHints.BydbQL); } + if (queryHints.params) { + validatedHints.params = queryHints.params; + } if (queryHints.resource_type) { validatedHints.resource_type = validateQueryHintResourceType(queryHints.resource_type); } diff --git a/mcp/src/server/mcp.ts b/mcp/src/server/mcp.ts index 1f237061b..3edaae6f7 100644 --- a/mcp/src/server/mcp.ts +++ b/mcp/src/server/mcp.ts @@ -23,7 +23,7 @@ import { BanyanDBClient, ResourceMetadata } from '../client/index.js'; import { MAX_TOOL_RESPONSE_LENGTH } from '../config.js'; import { loadQueryContext } from '../query/context.js'; import { generateBydbQL } from '../query/llm-prompt.js'; -import { normalizeQueryHints, validateListGroupsArgs, validateQueryHints } from '../query/validation.js'; +import { normalizeQueryHints, toTagValueParams, validateListGroupsArgs, validateQueryHints } from '../query/validation.js'; export function truncateText(text: string, maxLength: number): string { if (text.length <= maxLength) { @@ -78,7 +78,28 @@ function buildListToolsResponse() { properties: { BydbQL: { type: 'string', - description: 'BydbQL query to execute against BanyanDB.', + description: 'BydbQL query to execute against BanyanDB. May contain `?` placeholders bound by the params array.', + }, + params: { + type: 'array', + description: + 'Positional parameters bound to `?` placeholders in the query, in order of appearance. ' + + 'Always pass untrusted or user-provided values here instead of concatenating them into the query string.', + items: { + type: 'object', + properties: { + type: { + type: 'string', + description: 'Parameter type', + enum: ['str', 'int', 'str_array', 'int_array', 'null'], + }, + value: { + description: + 'Parameter value: a string for "str", an integer for "int", an array for "str_array"/"int_array"; omit for "null".', + }, + }, + required: ['type'], + }, }, resource_type: { type: 'string', @@ -183,9 +204,10 @@ async function handleListResourcesBydbql(banyandbClient: BanyanDBClient, args: u if (!queryHints.BydbQL) { throw new Error('BydbQL is required'); } + const params = queryHints.params ? toTagValueParams(queryHints.params) : undefined; try { - const result = await banyandbClient.query(queryHints.BydbQL); + const result = await banyandbClient.query(queryHints.BydbQL, params); const debugParts: string[] = []; if (queryHints.resource_type) debugParts.push(`Resource Type: ${queryHints.resource_type}`); diff --git a/pkg/bydbql/binder.go b/pkg/bydbql/binder.go new file mode 100644 index 000000000..32d10df25 --- /dev/null +++ b/pkg/bydbql/binder.go @@ -0,0 +1,311 @@ +// Licensed to Apache Software Foundation (ASF) under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Apache Software Foundation (ASF) licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package bydbql + +import ( + "fmt" + "math" + "time" + + modelv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/model/v1" +) + +// BindParams binds positional parameters to the `?` placeholders in the parsed grammar, +// in their order of appearance in the query text. After a successful call, the grammar is +// indistinguishable from one parsed with literal values, so the transformer needs no +// placeholder awareness. Callers must invoke it before Transform even when params is empty, +// so a query holding unbound placeholders is rejected instead of silently misinterpreted. +// Parameter positions in error messages are 1-based. On error the grammar may be partially +// bound and must be discarded; binding is not retryable on the same grammar. +func BindParams(g *Grammar, params []*modelv1.TagValue) error { + b := &binder{} + b.collect(g) + if len(b.slots) != len(params) { + return fmt.Errorf("parameter count mismatch: query contains %d placeholder(s) but %d parameter(s) provided", len(b.slots), len(params)) + } + for idx, slot := range b.slots { + if params[idx].GetValue() == nil { + return fmt.Errorf("parameter #%d has no value", idx+1) + } + if bindErr := slot(params[idx]); bindErr != nil { + return fmt.Errorf("failed to bind parameter #%d: %w", idx+1, bindErr) + } + } + b.expandLists() + return nil +} + +// countUnboundParams reports how many `?` placeholders in the grammar are still unbound. +func countUnboundParams(g *Grammar) int { + b := &binder{} + b.collect(g) + return len(b.slots) +} + +type listContainer struct { + assign func([]*GrammarValue) + values []*GrammarValue +} + +type binder struct { + expansions map[*GrammarValue][]*GrammarValue + slots []func(*modelv1.TagValue) error + lists []listContainer +} + +// collect gathers placeholder slots in their order of appearance in the query +// text: the grammar fixes the clause order and expression trees are visited +// depth-first left-to-right, so the slot order needs no re-sorting. +func (b *binder) collect(g *Grammar) { + if g.Select != nil { + if g.Select.Projection != nil && g.Select.Projection.TopN != nil { + topN := g.Select.Projection.TopN + b.collectIntSlot(&topN.N, &topN.NParam) + } + b.collectTime(g.Select.Time) + if g.Select.Where != nil { + b.collectOrExpr(g.Select.Where.Expr) + } + if g.Select.Limit != nil { + b.collectIntSlot(&g.Select.Limit.Value, &g.Select.Limit.Param) + } + if g.Select.Offset != nil { + b.collectIntSlot(&g.Select.Offset.Value, &g.Select.Offset.Param) + } + } + if g.TopN != nil { + b.collectIntSlot(&g.TopN.N, &g.TopN.NParam) + b.collectTime(g.TopN.Time) + if g.TopN.Where != nil { + b.collectAndExpr(g.TopN.Where.Expr) + } + } +} + +// collectIntSlot registers a placeholder in an integer-only position +// (LIMIT, OFFSET, or a TOP N count). +func (b *binder) collectIntSlot(value *int, param *bool) { + if !*param { + return + } + b.slots = append(b.slots, func(p *modelv1.TagValue) error { + intVal, ok := p.Value.(*modelv1.TagValue_Int) + if !ok { + return fmt.Errorf("this position only accepts int parameters, got %T", p.Value) + } + // The transformer narrows these counts to int32/uint32; reject values + // that would silently wrap instead of letting the cast corrupt them. + bound := intVal.Int.GetValue() + if bound < 0 || bound > math.MaxInt32 { + return fmt.Errorf("int parameter %d is out of range [0, %d] for this position", bound, math.MaxInt32) + } + *value = int(bound) + *param = false + return nil + }) +} + +func (b *binder) collectTime(clause *GrammarTimeClause) { + if clause == nil { + return + } + b.collectTimeValue(clause.Value) + if clause.Between != nil { + b.collectTimeValue(clause.Between.Begin) + b.collectTimeValue(clause.Between.End) + } +} + +func (b *binder) collectTimeValue(v *GrammarTimeValue) { + if v == nil || !v.Param { + return + } + b.slots = append(b.slots, func(p *modelv1.TagValue) error { return bindTimeValue(v, p) }) +} + +func (b *binder) collectOrExpr(expr *GrammarOrExpr) { + if expr == nil { + return + } + b.collectAndExpr(expr.Left) + for _, right := range expr.Right { + b.collectAndExpr(right.Right) + } +} + +func (b *binder) collectAndExpr(expr *GrammarAndExpr) { + if expr == nil { + return + } + b.collectPredicate(expr.Left) + for _, right := range expr.Right { + b.collectPredicate(right.Right) + } +} + +func (b *binder) collectPredicate(pred *GrammarPredicate) { + if pred == nil { + return + } + switch { + case pred.Paren != nil: + b.collectOrExpr(pred.Paren) + case pred.Binary != nil && pred.Binary.Tail != nil: + if compare := pred.Binary.Tail.Compare; compare != nil { + b.collectScalarValue(compare.Value) + } + if match := pred.Binary.Tail.Match; match != nil && match.Values != nil { + b.collectSingleOrArray(&match.Values.Single, &match.Values.Array) + } + case pred.In != nil: + inPred := pred.In + b.collectValueList(inPred.Values, func(vs []*GrammarValue) { inPred.Values = vs }) + case pred.Having != nil && pred.Having.Values != nil: + b.collectSingleOrArray(&pred.Having.Values.Single, &pred.Having.Values.Array) + } +} + +// collectSingleOrArray collects placeholders from a MATCH/HAVING container and +// splices bound values back preserving its original form: a single-value +// container stays single unless an array expansion produced multiple values, +// while a parenthesized list stays a list regardless of the value count, +// exactly as if written literally. +func (b *binder) collectSingleOrArray(single **GrammarValue, array *[]*GrammarValue) { + wasSingle := *single != nil + values := *array + if wasSingle { + values = []*GrammarValue{*single} + } + b.collectValueList(values, func(vs []*GrammarValue) { + if wasSingle && len(vs) == 1 { + *single, *array = vs[0], nil + return + } + *single, *array = nil, vs + }) +} + +func (b *binder) collectScalarValue(v *GrammarValue) { + if v == nil || !v.Param { + return + } + b.slots = append(b.slots, func(p *modelv1.TagValue) error { return bindScalarValue(v, p) }) +} + +func (b *binder) collectValueList(values []*GrammarValue, assign func([]*GrammarValue)) { + hasParam := false + for _, v := range values { + if v == nil || !v.Param { + continue + } + hasParam = true + b.slots = append(b.slots, func(p *modelv1.TagValue) error { return b.bindListValue(v, p) }) + } + if hasParam { + b.lists = append(b.lists, listContainer{values: values, assign: assign}) + } +} + +func (b *binder) bindListValue(v *GrammarValue, p *modelv1.TagValue) error { + switch val := p.Value.(type) { + case *modelv1.TagValue_Str, *modelv1.TagValue_Int, *modelv1.TagValue_Null: + return bindScalarValue(v, p) + case *modelv1.TagValue_StrArray: + if len(val.StrArray.GetValue()) == 0 { + return fmt.Errorf("array parameter must not be empty") + } + expanded := make([]*GrammarValue, 0, len(val.StrArray.GetValue())) + for _, s := range val.StrArray.GetValue() { + expanded = append(expanded, &GrammarValue{String: &s}) + } + b.recordExpansion(v, expanded) + case *modelv1.TagValue_IntArray: + if len(val.IntArray.GetValue()) == 0 { + return fmt.Errorf("array parameter must not be empty") + } + expanded := make([]*GrammarValue, 0, len(val.IntArray.GetValue())) + for _, i := range val.IntArray.GetValue() { + expanded = append(expanded, &GrammarValue{Integer: &i}) + } + b.recordExpansion(v, expanded) + default: + return fmt.Errorf("value list only accepts str, int, null, str_array, or int_array parameters, got %T", p.Value) + } + return nil +} + +func (b *binder) recordExpansion(v *GrammarValue, expanded []*GrammarValue) { + if b.expansions == nil { + b.expansions = make(map[*GrammarValue][]*GrammarValue) + } + b.expansions[v] = expanded +} + +func (b *binder) expandLists() { + for _, list := range b.lists { + rebuilt := make([]*GrammarValue, 0, len(list.values)) + for _, v := range list.values { + if expanded, ok := b.expansions[v]; ok { + rebuilt = append(rebuilt, expanded...) + continue + } + rebuilt = append(rebuilt, v) + } + list.assign(rebuilt) + } +} + +func bindTimeValue(v *GrammarTimeValue, p *modelv1.TagValue) error { + switch val := p.Value.(type) { + case *modelv1.TagValue_Str: + strVal := val.Str.GetValue() + v.String = &strVal + case *modelv1.TagValue_Int: + intVal := val.Int.GetValue() + v.Integer = &intVal + case *modelv1.TagValue_Timestamp: + // A nil inner timestamp would silently decode as the Unix epoch and an + // out-of-range one would format as a nonsense year; reject both here. + if err := val.Timestamp.CheckValid(); err != nil { + return fmt.Errorf("invalid timestamp parameter: %w", err) + } + strVal := val.Timestamp.AsTime().Format(time.RFC3339Nano) + v.String = &strVal + default: + return fmt.Errorf("time clause only accepts str, int, or timestamp parameters, got %T", p.Value) + } + v.Param = false + return nil +} + +func bindScalarValue(v *GrammarValue, p *modelv1.TagValue) error { + switch val := p.Value.(type) { + case *modelv1.TagValue_Str: + strVal := val.Str.GetValue() + v.String = &strVal + case *modelv1.TagValue_Int: + intVal := val.Int.GetValue() + v.Integer = &intVal + case *modelv1.TagValue_Null: + v.Null = true + default: + return fmt.Errorf("this position only accepts str, int, or null parameters, got %T", p.Value) + } + v.Param = false + return nil +} diff --git a/pkg/bydbql/binder_equivalence_test.go b/pkg/bydbql/binder_equivalence_test.go new file mode 100644 index 000000000..bc63fc683 --- /dev/null +++ b/pkg/bydbql/binder_equivalence_test.go @@ -0,0 +1,413 @@ +// Licensed to Apache Software Foundation (ASF) under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Apache Software Foundation (ASF) licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package bydbql_test + +import ( + "context" + "time" + + "github.com/alecthomas/participle/v2/lexer" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "go.uber.org/mock/gomock" + "google.golang.org/protobuf/testing/protocmp" + + commonv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/common/v1" + databasev1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/database/v1" + measurev1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/measure/v1" + modelv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/model/v1" + streamv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/stream/v1" + "github.com/apache/skywalking-banyandb/banyand/metadata" + "github.com/apache/skywalking-banyandb/banyand/metadata/schema" + . "github.com/apache/skywalking-banyandb/pkg/bydbql" +) + +// equivalenceCase describes a parameterized query that must be indistinguishable, +// both at the AST level and at the native query request level, from the same +// query written with literal values. +type equivalenceCase struct { + name string + parameterized string + literal string + params []*modelv1.TagValue + // ignoreTimeRange skips the time_range comparison for queries whose range + // depends on the wall clock at transform time (relative times, open-ended comparators). + ignoreTimeRange bool + // expectError marks cases whose transform must fail on BOTH sides with the + // same message; without it a case reaching the error branch fails the test, + // so no case can pass vacuously. + expectError bool +} + +func equivalenceStream() *databasev1.Stream { + return &databasev1.Stream{ + Metadata: &commonv1.Metadata{Name: "sw", Group: "default"}, + TagFamilies: []*databasev1.TagFamilySpec{{ + Name: "searchable", + Tags: []*databasev1.TagSpec{ + {Name: "service_id", Type: databasev1.TagType_TAG_TYPE_STRING}, + {Name: "message", Type: databasev1.TagType_TAG_TYPE_STRING}, + {Name: "tags", Type: databasev1.TagType_TAG_TYPE_STRING_ARRAY}, + {Name: "duration", Type: databasev1.TagType_TAG_TYPE_INT}, + {Name: "codes", Type: databasev1.TagType_TAG_TYPE_INT_ARRAY}, + {Name: "created_at", Type: databasev1.TagType_TAG_TYPE_TIMESTAMP}, + }, + }}, + } +} + +func equivalenceCases() []equivalenceCase { + base := "SELECT * FROM STREAM sw IN default " + return []equivalenceCase{ + // TIME clause: every accepted parameter type + { + name: "str relative time in TIME >", + parameterized: base + "TIME > ?", + literal: base + "TIME > '-30m'", + params: []*modelv1.TagValue{strParam("-30m")}, + ignoreTimeRange: true, + }, + { + name: "str absolute times in TIME BETWEEN", + parameterized: base + "TIME BETWEEN ? AND ?", + literal: base + "TIME BETWEEN '2026-07-06T10:00:00Z' AND '2026-07-06T11:00:00Z'", + params: []*modelv1.TagValue{strParam("2026-07-06T10:00:00Z"), strParam("2026-07-06T11:00:00Z")}, + }, + { + name: "timestamp params in TIME BETWEEN", + parameterized: base + "TIME BETWEEN ? AND ?", + literal: base + "TIME BETWEEN '2026-07-06T10:00:00Z' AND '2026-07-06T11:00:00Z'", + params: []*modelv1.TagValue{ + timestampParam(time.Date(2026, 7, 6, 10, 0, 0, 0, time.UTC)), + timestampParam(time.Date(2026, 7, 6, 11, 0, 0, 0, time.UTC)), + }, + }, + { + // An int TIME value is grammatically valid but the transformer cannot + // parse a bare integer as a timestamp; both sides must fail identically. + name: "int param in TIME >", + parameterized: base + "TIME > ?", + literal: base + "TIME > 1720000000", + params: []*modelv1.TagValue{intParam(1720000000)}, + expectError: true, + }, + // Scalar comparisons: every operator, every accepted parameter type + { + name: "str param in =", + parameterized: base + "WHERE service_id = ?", + literal: base + "WHERE service_id = 'webapp'", + params: []*modelv1.TagValue{strParam("webapp")}, + }, + { + name: "str param in !=", + parameterized: base + "WHERE service_id != ?", + literal: base + "WHERE service_id != 'webapp'", + params: []*modelv1.TagValue{strParam("webapp")}, + }, + { + name: "int param in >", + parameterized: base + "WHERE duration > ?", + literal: base + "WHERE duration > 100", + params: []*modelv1.TagValue{intParam(100)}, + }, + { + name: "int param in >=", + parameterized: base + "WHERE duration >= ?", + literal: base + "WHERE duration >= 100", + params: []*modelv1.TagValue{intParam(100)}, + }, + { + name: "int param in <", + parameterized: base + "WHERE duration < ?", + literal: base + "WHERE duration < 100", + params: []*modelv1.TagValue{intParam(100)}, + }, + { + name: "int param in <=", + parameterized: base + "WHERE duration <= ?", + literal: base + "WHERE duration <= 100", + params: []*modelv1.TagValue{intParam(100)}, + }, + { + name: "null param in =", + parameterized: base + "WHERE service_id = ?", + literal: base + "WHERE service_id = NULL", + params: []*modelv1.TagValue{nullParam()}, + }, + // IN predicate + { + name: "two str params in IN", + parameterized: base + "WHERE service_id IN (?, ?)", + literal: base + "WHERE service_id IN ('a', 'b')", + params: []*modelv1.TagValue{strParam("a"), strParam("b")}, + }, + { + name: "str_array param expands in IN", + parameterized: base + "WHERE service_id IN (?)", + literal: base + "WHERE service_id IN ('a', 'b')", + params: []*modelv1.TagValue{strArrayParam("a", "b")}, + }, + { + name: "int_array param expands in IN", + parameterized: base + "WHERE duration IN (?)", + literal: base + "WHERE duration IN (100, 200)", + params: []*modelv1.TagValue{intArrayParam(100, 200)}, + }, + { + name: "str_array param expands between literals in IN", + parameterized: base + "WHERE service_id IN ('a', ?, 'd')", + literal: base + "WHERE service_id IN ('a', 'b', 'c', 'd')", + params: []*modelv1.TagValue{strArrayParam("b", "c")}, + }, + { + name: "two array params and a scalar param expand in one IN list", + parameterized: base + "WHERE service_id IN (?, ?, ?)", + literal: base + "WHERE service_id IN ('a', 'b', 'c', 'd')", + params: []*modelv1.TagValue{strArrayParam("a", "b"), strParam("c"), strArrayParam("d")}, + }, + { + name: "array params expand in two lists of one query", + parameterized: base + "WHERE service_id IN (?) AND codes HAVING (?)", + literal: base + "WHERE service_id IN ('a', 'b') AND codes HAVING (200, 500)", + params: []*modelv1.TagValue{strArrayParam("a", "b"), intArrayParam(200, 500)}, + }, + { + name: "str param in NOT IN", + parameterized: base + "WHERE service_id NOT IN (?)", + literal: base + "WHERE service_id NOT IN ('a')", + params: []*modelv1.TagValue{strParam("a")}, + }, + // MATCH predicate + { + name: "str param in MATCH", + parameterized: base + "WHERE message MATCH(?)", + literal: base + "WHERE message MATCH('error')", + params: []*modelv1.TagValue{strParam("error")}, + }, + { + name: "str_array param expands in MATCH", + parameterized: base + "WHERE message MATCH(?)", + literal: base + "WHERE message MATCH(('error', 'warn'))", + params: []*modelv1.TagValue{strArrayParam("error", "warn")}, + }, + { + name: "str param in MATCH keeps literal analyzer", + parameterized: base + "WHERE message MATCH(?, 'simple')", + literal: base + "WHERE message MATCH('error', 'simple')", + params: []*modelv1.TagValue{strParam("error")}, + }, + // HAVING predicate + { + name: "str param in HAVING", + parameterized: base + "WHERE tags HAVING ?", + literal: base + "WHERE tags HAVING 'blue'", + params: []*modelv1.TagValue{strParam("blue")}, + }, + { + name: "int_array param expands in HAVING", + parameterized: base + "WHERE codes HAVING (?)", + literal: base + "WHERE codes HAVING (200, 500)", + params: []*modelv1.TagValue{intArrayParam(200, 500)}, + }, + { + name: "str_array param expands in NOT HAVING", + parameterized: base + "WHERE tags NOT HAVING (?)", + literal: base + "WHERE tags NOT HAVING ('red', 'green')", + params: []*modelv1.TagValue{strArrayParam("red", "green")}, + }, + // Single value vs list form: the bound container must keep the exact + // form a literal query would have, because the transformer emits a + // scalar condition for the single form and an array condition for the + // list form. + { + name: "one-element array in a parenthesized HAVING list stays a list", + parameterized: base + "WHERE codes HAVING (?)", + literal: base + "WHERE codes HAVING (200)", + params: []*modelv1.TagValue{intArrayParam(200)}, + }, + { + name: "one-element array in a single HAVING value stays a single value", + parameterized: base + "WHERE tags HAVING ?", + literal: base + "WHERE tags HAVING 'blue'", + params: []*modelv1.TagValue{strArrayParam("blue")}, + }, + { + name: "multi-element array in a single HAVING value becomes a list", + parameterized: base + "WHERE tags HAVING ?", + literal: base + "WHERE tags HAVING ('a', 'b')", + params: []*modelv1.TagValue{strArrayParam("a", "b")}, + }, + { + name: "scalar in a parenthesized MATCH list stays a list", + parameterized: base + "WHERE message MATCH((?))", + literal: base + "WHERE message MATCH(('error'))", + params: []*modelv1.TagValue{strParam("error")}, + }, + // Parameter type vs tag schema type: mismatches must behave exactly like + // the same mismatch written as a literal — lenient conversion where the + // transformer converts, identical errors where it rejects. + { + name: "int param on a STRING tag converts like an int literal", + parameterized: base + "WHERE service_id = ?", + literal: base + "WHERE service_id = 123", + params: []*modelv1.TagValue{intParam(123)}, + }, + { + name: "numeric str param on an INT tag parses like a str literal", + parameterized: base + "WHERE duration > ?", + literal: base + "WHERE duration > '100'", + params: []*modelv1.TagValue{strParam("100")}, + }, + { + name: "non-numeric str param on an INT tag fails like a str literal", + parameterized: base + "WHERE duration > ?", + literal: base + "WHERE duration > 'abc'", + params: []*modelv1.TagValue{strParam("abc")}, + expectError: true, + }, + { + name: "non-numeric str_array param on an INT_ARRAY tag fails like literals", + parameterized: base + "WHERE codes HAVING (?)", + literal: base + "WHERE codes HAVING ('a', 'b')", + params: []*modelv1.TagValue{strArrayParam("a", "b")}, + expectError: true, + }, + { + name: "int_array param on a STRING_ARRAY tag converts like int literals", + parameterized: base + "WHERE tags HAVING (?)", + literal: base + "WHERE tags HAVING (1, 2)", + params: []*modelv1.TagValue{intArrayParam(1, 2)}, + }, + { + name: "param on a TIMESTAMP tag is rejected like a literal", + parameterized: base + "WHERE created_at = ?", + literal: base + "WHERE created_at = '2026-07-06T10:00:00Z'", + params: []*modelv1.TagValue{strParam("2026-07-06T10:00:00Z")}, + expectError: true, + }, + { + name: "param on an unknown tag fails like a literal", + parameterized: base + "WHERE nonexistent = ?", + literal: base + "WHERE nonexistent = 'x'", + params: []*modelv1.TagValue{strParam("x")}, + expectError: true, + }, + // Combined clauses and nested expressions + { + name: "params across TIME and nested OR/AND expressions", + parameterized: base + "TIME > ? WHERE (service_id = ? OR duration > ?) AND message MATCH(?)", + literal: base + "TIME > '-15m' WHERE (service_id = 'svc' OR duration > 42) AND message MATCH('boom')", + params: []*modelv1.TagValue{strParam("-15m"), strParam("svc"), intParam(42), strParam("boom")}, + ignoreTimeRange: true, + }, + // Integer-only positions: LIMIT / OFFSET / TOP N counts + { + name: "int params in LIMIT and OFFSET", + parameterized: base + "WHERE service_id = 'svc' LIMIT ? OFFSET ?", + literal: base + "WHERE service_id = 'svc' LIMIT 10 OFFSET 5", + params: []*modelv1.TagValue{intParam(10), intParam(5)}, + }, + { + name: "int param in SELECT TOP N projection count", + parameterized: "SELECT TOP ? value, service FROM MEASURE svc_metrics IN default", + literal: "SELECT TOP 3 value, service FROM MEASURE svc_metrics IN default", + params: []*modelv1.TagValue{intParam(3)}, + }, + // SHOW TOP N statement + { + name: "params in SHOW TOP N count, TIME, and WHERE", + parameterized: "SHOW TOP ? FROM MEASURE svc_topn IN default TIME > ? WHERE service = ?", + literal: "SHOW TOP 5 FROM MEASURE svc_topn IN default TIME > '-1h' WHERE service = 'svc'", + params: []*modelv1.TagValue{intParam(5), strParam("-1h"), strParam("svc")}, + ignoreTimeRange: true, + }, + } +} + +var _ = Describe("BindParams equivalence with literal queries", func() { + var transformer *Transformer + + BeforeEach(func() { + ctrl := gomock.NewController(GinkgoT()) + streamRegistry := schema.NewMockStream(ctrl) + streamRegistry.EXPECT().GetStream(gomock.Any(), gomock.Any()).Return(equivalenceStream(), nil).AnyTimes() + topNRegistry := schema.NewMockTopNAggregation(ctrl) + topNRegistry.EXPECT().GetTopNAggregation(gomock.Any(), gomock.Any()).Return(&databasev1.TopNAggregation{ + Metadata: &commonv1.Metadata{Name: "svc_topn", Group: "default"}, + SourceMeasure: &commonv1.Metadata{Name: "svc_metrics", Group: "default"}, + }, nil).AnyTimes() + measureRegistry := schema.NewMockMeasure(ctrl) + measureRegistry.EXPECT().GetMeasure(gomock.Any(), gomock.Any()).Return(&databasev1.Measure{ + Metadata: &commonv1.Metadata{Name: "svc_metrics", Group: "default"}, + TagFamilies: []*databasev1.TagFamilySpec{{ + Name: "default", + Tags: []*databasev1.TagSpec{{Name: "service", Type: databasev1.TagType_TAG_TYPE_STRING}}, + }}, + Fields: []*databasev1.FieldSpec{{Name: "value", FieldType: databasev1.FieldType_FIELD_TYPE_INT}}, + }, nil).AnyTimes() + mockRepo := metadata.NewMockRepo(ctrl) + mockRepo.EXPECT().StreamRegistry().AnyTimes().Return(streamRegistry) + mockRepo.EXPECT().TopNAggregationRegistry().AnyTimes().Return(topNRegistry) + mockRepo.EXPECT().MeasureRegistry().AnyTimes().Return(measureRegistry) + transformer = NewTransformer(mockRepo) + }) + + for _, testCase := range equivalenceCases() { + It(testCase.name, func() { + boundGrammar, err := ParseQuery(testCase.parameterized) + Expect(err).To(BeNil()) + Expect(BindParams(boundGrammar, testCase.params)).To(Succeed()) + literalGrammar, err := ParseQuery(testCase.literal) + Expect(err).To(BeNil()) + Expect(BindParams(literalGrammar, nil)).To(Succeed()) + + // The bound AST must be indistinguishable from the literal AST + astDiff := cmp.Diff(literalGrammar, boundGrammar, cmpopts.IgnoreTypes(lexer.Position{})) + Expect(astDiff).To(BeEmpty(), "AST mismatch:\n%s", astDiff) + + // Both must transform to the same native query request + ctx := context.Background() + literalResult, literalErr := transformer.Transform(ctx, literalGrammar) + boundResult, boundErr := transformer.Transform(ctx, boundGrammar) + if literalErr != nil { + Expect(testCase.expectError).To(BeTrue(), "unexpected transform error: %v", literalErr) + Expect(boundErr).To(HaveOccurred()) + Expect(boundErr.Error()).To(Equal(literalErr.Error())) + return + } + Expect(testCase.expectError).To(BeFalse(), "expected a transform error but both sides succeeded") + Expect(boundErr).To(BeNil()) + Expect(boundResult.Type).To(Equal(literalResult.Type)) + // SELECT * projections are built from a map, so their tag order is + // nondeterministic per Transform call; sort them before comparing. + opts := []cmp.Option{ + protocmp.Transform(), + protocmp.SortRepeatedFields(&modelv1.TagProjection_TagFamily{}, "tags"), + } + if testCase.ignoreTimeRange { + opts = append(opts, + protocmp.IgnoreFields(&streamv1.QueryRequest{}, "time_range"), + protocmp.IgnoreFields(&measurev1.TopNRequest{}, "time_range")) + } + requestDiff := cmp.Diff(literalResult.QueryRequest, boundResult.QueryRequest, opts...) + Expect(requestDiff).To(BeEmpty(), "query request mismatch:\n%s", requestDiff) + }) + } +}) diff --git a/pkg/bydbql/binder_test.go b/pkg/bydbql/binder_test.go new file mode 100644 index 000000000..ed2fed8dc --- /dev/null +++ b/pkg/bydbql/binder_test.go @@ -0,0 +1,573 @@ +// Licensed to Apache Software Foundation (ASF) under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Apache Software Foundation (ASF) licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package bydbql_test + +import ( + "math" + "reflect" + "strings" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "google.golang.org/protobuf/types/known/timestamppb" + + modelv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/model/v1" + . "github.com/apache/skywalking-banyandb/pkg/bydbql" +) + +func strParam(v string) *modelv1.TagValue { + return &modelv1.TagValue{Value: &modelv1.TagValue_Str{Str: &modelv1.Str{Value: v}}} +} + +func intParam(v int64) *modelv1.TagValue { + return &modelv1.TagValue{Value: &modelv1.TagValue_Int{Int: &modelv1.Int{Value: v}}} +} + +func strArrayParam(vs ...string) *modelv1.TagValue { + return &modelv1.TagValue{Value: &modelv1.TagValue_StrArray{StrArray: &modelv1.StrArray{Value: vs}}} +} + +func intArrayParam(vs ...int64) *modelv1.TagValue { + return &modelv1.TagValue{Value: &modelv1.TagValue_IntArray{IntArray: &modelv1.IntArray{Value: vs}}} +} + +func nullParam() *modelv1.TagValue { + return &modelv1.TagValue{Value: &modelv1.TagValue_Null{}} +} + +func timestampParam(t time.Time) *modelv1.TagValue { + return &modelv1.TagValue{Value: &modelv1.TagValue_Timestamp{Timestamp: timestamppb.New(t)}} +} + +func binaryParam(data []byte) *modelv1.TagValue { + return &modelv1.TagValue{Value: &modelv1.TagValue_BinaryData{BinaryData: data}} +} + +var _ = Describe("BindParams", func() { + parse := func(query string) *Grammar { + grammar, err := ParseQuery(query) + Expect(err).To(BeNil()) + return grammar + } + + Describe("TIME clause binding", func() { + It("binds a str parameter as a relative time", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ?") + Expect(BindParams(grammar, []*modelv1.TagValue{strParam("-30m")})).To(Succeed()) + Expect(grammar.Select.Time.Value.String).NotTo(BeNil()) + Expect(*grammar.Select.Time.Value.String).To(Equal("-30m")) + Expect(grammar.Select.Time.Value.Param).To(BeFalse()) + }) + + It("binds a timestamp parameter as an RFC3339 string", func() { + ts := time.Date(2026, 7, 6, 10, 0, 0, 0, time.UTC) + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ?") + Expect(BindParams(grammar, []*modelv1.TagValue{timestampParam(ts)})).To(Succeed()) + Expect(grammar.Select.Time.Value.String).NotTo(BeNil()) + parsed, err := time.Parse(time.RFC3339, *grammar.Select.Time.Value.String) + Expect(err).To(BeNil()) + Expect(parsed.Equal(ts)).To(BeTrue()) + }) + + It("binds both boundaries of TIME BETWEEN", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME BETWEEN ? AND ?") + Expect(BindParams(grammar, []*modelv1.TagValue{strParam("-1h"), strParam("now")})).To(Succeed()) + Expect(*grammar.Select.Time.Between.Begin.String).To(Equal("-1h")) + Expect(*grammar.Select.Time.Between.End.String).To(Equal("now")) + }) + + It("rejects an array parameter in the TIME clause", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ?") + err := BindParams(grammar, []*modelv1.TagValue{strArrayParam("a", "b")}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("time clause")) + }) + }) + + Describe("WHERE clause binding", func() { + It("binds a str parameter in a comparison", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + Expect(BindParams(grammar, []*modelv1.TagValue{strParam("webapp")})).To(Succeed()) + value := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Compare.Value + Expect(value.String).NotTo(BeNil()) + Expect(*value.String).To(Equal("webapp")) + Expect(value.Param).To(BeFalse()) + }) + + It("binds an int parameter in a comparison", func() { + grammar := parse("SELECT * FROM MEASURE metrics IN default WHERE value > ?") + Expect(BindParams(grammar, []*modelv1.TagValue{intParam(100)})).To(Succeed()) + value := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Compare.Value + Expect(value.Integer).NotTo(BeNil()) + Expect(*value.Integer).To(Equal(int64(100))) + }) + + It("binds a null parameter in a comparison", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + Expect(BindParams(grammar, []*modelv1.TagValue{nullParam()})).To(Succeed()) + value := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Compare.Value + Expect(value.Null).To(BeTrue()) + }) + + It("binds a str parameter in MATCH", func() { + grammar := parse("SELECT trace_id, message FROM STREAM logs IN group1 TIME > ? WHERE message MATCH(?)") + Expect(BindParams(grammar, []*modelv1.TagValue{strParam("-30m"), strParam("error")})).To(Succeed()) + match := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Match + Expect(match.Values.Single).NotTo(BeNil()) + Expect(*match.Values.Single.String).To(Equal("error")) + }) + + It("expands a str_array parameter in MATCH into multiple values", func() { + grammar := parse("SELECT * FROM STREAM logs IN group1 WHERE message MATCH(?)") + Expect(BindParams(grammar, []*modelv1.TagValue{strArrayParam("error", "warn")})).To(Succeed()) + match := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Match + Expect(match.Values.Single).To(BeNil()) + Expect(match.Values.Array).To(HaveLen(2)) + Expect(*match.Values.Array[0].String).To(Equal("error")) + Expect(*match.Values.Array[1].String).To(Equal("warn")) + }) + + It("expands multiple array parameters and a scalar in one list in order", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id IN (?, ?, ?)") + Expect(BindParams(grammar, []*modelv1.TagValue{strArrayParam("a", "b"), strParam("c"), strArrayParam("d", "e")})).To(Succeed()) + values := grammar.Select.Where.Expr.Left.Left.In.Values + Expect(values).To(HaveLen(5)) + for idx, expected := range []string{"a", "b", "c", "d", "e"} { + Expect(*values[idx].String).To(Equal(expected)) + } + }) + + It("expands an array parameter in IN alongside literal values", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id IN ('a', ?, 'b')") + Expect(BindParams(grammar, []*modelv1.TagValue{strArrayParam("x", "y")})).To(Succeed()) + values := grammar.Select.Where.Expr.Left.Left.In.Values + Expect(values).To(HaveLen(4)) + Expect(*values[0].String).To(Equal("a")) + Expect(*values[1].String).To(Equal("x")) + Expect(*values[2].String).To(Equal("y")) + Expect(*values[3].String).To(Equal("b")) + }) + + It("expands an int_array parameter in HAVING", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE codes HAVING (?)") + Expect(BindParams(grammar, []*modelv1.TagValue{intArrayParam(200, 500)})).To(Succeed()) + having := grammar.Select.Where.Expr.Left.Left.Having + Expect(having.Values.Array).To(HaveLen(2)) + Expect(*having.Values.Array[0].Integer).To(Equal(int64(200))) + Expect(*having.Values.Array[1].Integer).To(Equal(int64(500))) + }) + + It("expands array parameters in multiple lists within one query", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id IN (?) AND codes HAVING (?)") + Expect(BindParams(grammar, []*modelv1.TagValue{strArrayParam("a", "b"), intArrayParam(200, 500)})).To(Succeed()) + andExpr := grammar.Select.Where.Expr.Left + inValues := andExpr.Left.In.Values + Expect(inValues).To(HaveLen(2)) + Expect(*inValues[0].String).To(Equal("a")) + Expect(*inValues[1].String).To(Equal("b")) + havingValues := andExpr.Right[0].Right.Having.Values + Expect(havingValues.Array).To(HaveLen(2)) + Expect(*havingValues.Array[0].Integer).To(Equal(int64(200))) + Expect(*havingValues.Array[1].Integer).To(Equal(int64(500))) + }) + + It("expands an array parameter in MATCH while keeping the literal analyzer", func() { + grammar := parse("SELECT * FROM STREAM logs IN group1 WHERE message MATCH(?, 'simple')") + Expect(BindParams(grammar, []*modelv1.TagValue{strArrayParam("error", "warn")})).To(Succeed()) + match := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Match + Expect(match.Values.Array).To(HaveLen(2)) + Expect(match.Analyzer).NotTo(BeNil()) + Expect(*match.Analyzer).To(Equal("simple")) + }) + + It("rejects an empty array parameter", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id IN (?)") + err := BindParams(grammar, []*modelv1.TagValue{strArrayParam()}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must not be empty")) + }) + + It("rejects an array parameter in a scalar comparison", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + err := BindParams(grammar, []*modelv1.TagValue{strArrayParam("a", "b")}) + Expect(err).To(HaveOccurred()) + }) + + It("rejects a binary parameter anywhere", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + err := BindParams(grammar, []*modelv1.TagValue{binaryParam([]byte{1, 2})}) + Expect(err).To(HaveOccurred()) + }) + }) + + Describe("integer-only positions", func() { + It("binds int parameters to LIMIT and OFFSET", func() { + grammar := parse("SELECT * FROM STREAM sw IN default LIMIT ? OFFSET ?") + Expect(BindParams(grammar, []*modelv1.TagValue{intParam(10), intParam(5)})).To(Succeed()) + Expect(grammar.Select.Limit.Value).To(Equal(10)) + Expect(grammar.Select.Limit.Param).To(BeFalse()) + Expect(grammar.Select.Offset.Value).To(Equal(5)) + Expect(grammar.Select.Offset.Param).To(BeFalse()) + }) + + It("binds an int parameter to the SHOW TOP N count", func() { + grammar := parse("SHOW TOP ? FROM MEASURE metrics IN default") + Expect(BindParams(grammar, []*modelv1.TagValue{intParam(7)})).To(Succeed()) + Expect(grammar.TopN.N).To(Equal(7)) + Expect(grammar.TopN.NParam).To(BeFalse()) + }) + + It("binds an int parameter to the SELECT TOP N projection count", func() { + grammar := parse("SELECT TOP ? value FROM MEASURE metrics IN default") + Expect(BindParams(grammar, []*modelv1.TagValue{intParam(3)})).To(Succeed()) + Expect(grammar.Select.Projection.TopN.N).To(Equal(3)) + Expect(grammar.Select.Projection.TopN.NParam).To(BeFalse()) + }) + + It("rejects a str parameter in LIMIT", func() { + grammar := parse("SELECT * FROM STREAM sw IN default LIMIT ?") + err := BindParams(grammar, []*modelv1.TagValue{strParam("10")}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("only accepts int")) + }) + + It("rejects a negative int parameter in LIMIT", func() { + grammar := parse("SELECT * FROM STREAM sw IN default LIMIT ?") + err := BindParams(grammar, []*modelv1.TagValue{intParam(-1)}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("out of range")) + }) + + It("rejects an int parameter beyond int32 in OFFSET", func() { + grammar := parse("SELECT * FROM STREAM sw IN default OFFSET ?") + err := BindParams(grammar, []*modelv1.TagValue{intParam(math.MaxInt32 + 1)}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("out of range")) + }) + }) + + Describe("re-binding semantics", func() { + It("rejects binding the same grammar twice with the same parameters", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + params := []*modelv1.TagValue{strParam("svc")} + Expect(BindParams(grammar, params)).To(Succeed()) + err := BindParams(grammar, params) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("0 placeholder(s) but 1 parameter(s)")) + }) + + It("treats a second bind with no parameters as a no-op", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + Expect(BindParams(grammar, []*modelv1.TagValue{strParam("svc")})).To(Succeed()) + Expect(BindParams(grammar, nil)).To(Succeed()) + Expect(*grammar.Select.Where.Expr.Left.Left.Binary.Tail.Compare.Value.String).To(Equal("svc")) + }) + }) + + Describe("ordering and counting", func() { + It("binds parameters in order of appearance across clauses", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ? WHERE service_id = ? AND instance_id = ?") + Expect(BindParams(grammar, []*modelv1.TagValue{strParam("-15m"), strParam("svc"), strParam("inst")})).To(Succeed()) + Expect(*grammar.Select.Time.Value.String).To(Equal("-15m")) + andExpr := grammar.Select.Where.Expr.Left + Expect(*andExpr.Left.Binary.Tail.Compare.Value.String).To(Equal("svc")) + Expect(*andExpr.Right[0].Right.Binary.Tail.Compare.Value.String).To(Equal("inst")) + }) + + It("binds parameters in a SHOW TOP statement", func() { + grammar := parse("SHOW TOP 5 FROM MEASURE metrics IN default TIME > ? WHERE service = ?") + Expect(BindParams(grammar, []*modelv1.TagValue{strParam("-1h"), strParam("svc")})).To(Succeed()) + Expect(*grammar.TopN.Time.Value.String).To(Equal("-1h")) + Expect(*grammar.TopN.Where.Expr.Left.Binary.Tail.Compare.Value.String).To(Equal("svc")) + }) + + It("rejects fewer parameters than placeholders", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ? WHERE service_id = ?") + err := BindParams(grammar, []*modelv1.TagValue{strParam("-30m")}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("2 placeholder(s) but 1 parameter(s)")) + }) + + It("rejects more parameters than placeholders", func() { + grammar := parse("SELECT * FROM STREAM sw IN default") + err := BindParams(grammar, []*modelv1.TagValue{strParam("extra")}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("0 placeholder(s) but 1 parameter(s)")) + }) + + It("accepts a query without placeholders and no parameters", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = 'webapp'") + Expect(BindParams(grammar, nil)).To(Succeed()) + }) + }) + + Describe("binder coverage of grammar placeholder positions", func() { + // Tripwire: binder.collect is the single registry of placeholder positions, + // shared by BindParams and the Transform-entry guard. If a new @Param field + // is added to the grammar without wiring it into the binder, the placeholder + // would silently transform as a zero value. This test fails on any new + // @Param field until the binder (and the test matrices) are updated. + It("accounts for every @Param field in the grammar", func() { + expected := map[string]bool{ + "GrammarValue.Param": true, + "GrammarTimeValue.Param": true, + "GrammarTopNStatement.NParam": true, + "GrammarTopNProjection.NParam": true, + "GrammarLimitClause.Param": true, + "GrammarOffsetClause.Param": true, + } + found := make(map[string]bool) + visited := make(map[reflect.Type]bool) + var walk func(t reflect.Type) + walk = func(t reflect.Type) { + for t.Kind() == reflect.Pointer || t.Kind() == reflect.Slice { + t = t.Elem() + } + if t.Kind() != reflect.Struct || visited[t] { + return + } + visited[t] = true + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + if strings.Contains(field.Tag.Get("parser"), "@Param") { + found[t.Name()+"."+field.Name] = true + } + walk(field.Type) + } + } + walk(reflect.TypeOf(Grammar{})) + Expect(found).To(Equal(expected), + "grammar @Param positions changed: wire any new position into binder.collect, the acceptance matrix, and this list") + }) + }) + + Describe("placeholder syntax restrictions", func() { + // Placeholders are only legal in value positions. Every structural or + // identifier position must stay a syntax error so a bound parameter can + // never influence the query shape. + forbidden := map[string]string{ + "resource name": "SELECT * FROM STREAM ? IN default", + "group name": "SELECT * FROM STREAM sw IN ?", + "stage name": "SELECT * FROM STREAM sw IN default ON ? STAGES", + "projection column": "SELECT ? FROM STREAM sw IN default", + "condition left side": "SELECT * FROM STREAM sw IN default WHERE ? = 'a'", + "ORDER BY column": "SELECT * FROM STREAM sw IN default ORDER BY ?", + "GROUP BY column": "SELECT * FROM MEASURE m IN default GROUP BY ?", + "MATCH analyzer": "SELECT * FROM STREAM sw IN default WHERE message MATCH('x', ?)", + "doubled placeholder": "SELECT * FROM STREAM sw IN default WHERE service_id = ??", + } + for position, query := range forbidden { + It("rejects a placeholder in "+position+" at parse time", func() { + _, err := ParseQuery(query) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("syntax error")) + }) + } + }) + + Describe("parameter type acceptance matrix across all placeholder paths", func() { + timeAccepted := map[string]bool{"str": true, "int": true, "timestamp": true} + scalarAccepted := map[string]bool{"str": true, "int": true, "null": true} + listAccepted := map[string]bool{"str": true, "int": true, "null": true, "str_array": true, "int_array": true} + intAccepted := map[string]bool{"int": true} + paths := []struct { + accepted map[string]bool + name string + query string + }{ + {timeAccepted, "TIME comparison value", "SELECT * FROM STREAM sw IN default TIME > ?"}, + {timeAccepted, "TIME BETWEEN boundary", "SELECT * FROM STREAM sw IN default TIME BETWEEN ? AND '2026-07-06T11:00:00Z'"}, + {intAccepted, "LIMIT count", "SELECT * FROM STREAM sw IN default LIMIT ?"}, + {intAccepted, "OFFSET count", "SELECT * FROM STREAM sw IN default OFFSET ?"}, + {intAccepted, "SHOW TOP N count", "SHOW TOP ? FROM MEASURE m IN default"}, + {intAccepted, "SELECT TOP N projection count", "SELECT TOP ? value FROM MEASURE m IN default"}, + {scalarAccepted, "scalar equality value", "SELECT * FROM STREAM sw IN default WHERE service_id = ?"}, + {scalarAccepted, "scalar ordered comparison value", "SELECT * FROM STREAM sw IN default WHERE duration > ?"}, + {listAccepted, "IN value list", "SELECT * FROM STREAM sw IN default WHERE service_id IN (?)"}, + {listAccepted, "MATCH value list", "SELECT * FROM STREAM sw IN default WHERE message MATCH(?)"}, + {listAccepted, "HAVING single value", "SELECT * FROM STREAM sw IN default WHERE tags HAVING ?"}, + {listAccepted, "HAVING value list", "SELECT * FROM STREAM sw IN default WHERE codes HAVING (?)"}, + } + paramTypes := []struct { + param *modelv1.TagValue + name string + }{ + {strParam("value"), "str"}, + {intParam(42), "int"}, + {nullParam(), "null"}, + {strArrayParam("a", "b"), "str_array"}, + {intArrayParam(1, 2), "int_array"}, + {timestampParam(time.Date(2026, 7, 6, 10, 0, 0, 0, time.UTC)), "timestamp"}, + {binaryParam([]byte{1, 2}), "binary"}, + {&modelv1.TagValue{}, "empty"}, + {nil, "nil"}, + } + for _, path := range paths { + for _, paramType := range paramTypes { + expectAccept := path.accepted[paramType.name] + verdict := "rejects" + if expectAccept { + verdict = "accepts" + } + It(verdict+" a "+paramType.name+" parameter in "+path.name, func() { + grammar := parse(path.query) + err := BindParams(grammar, []*modelv1.TagValue{paramType.param}) + if expectAccept { + Expect(err).To(BeNil()) + } else { + Expect(err).To(HaveOccurred()) + } + }) + } + } + }) + + Describe("single value and list form preservation", func() { + It("keeps the single form when a one-element array binds a single MATCH value", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE message MATCH(?)") + Expect(BindParams(grammar, []*modelv1.TagValue{strArrayParam("error")})).To(Succeed()) + values := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Match.Values + Expect(values.Single).NotTo(BeNil()) + Expect(*values.Single.String).To(Equal("error")) + Expect(values.Array).To(BeNil()) + }) + + It("switches to the list form when a multi-element array binds a single HAVING value", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE tags HAVING ?") + Expect(BindParams(grammar, []*modelv1.TagValue{strArrayParam("a", "b")})).To(Succeed()) + values := grammar.Select.Where.Expr.Left.Left.Having.Values + Expect(values.Single).To(BeNil()) + Expect(values.Array).To(HaveLen(2)) + }) + + It("keeps the list form when a one-element array binds a parenthesized HAVING list", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE codes HAVING (?)") + Expect(BindParams(grammar, []*modelv1.TagValue{intArrayParam(200)})).To(Succeed()) + values := grammar.Select.Where.Expr.Left.Left.Having.Values + Expect(values.Single).To(BeNil()) + Expect(values.Array).To(HaveLen(1)) + Expect(*values.Array[0].Integer).To(Equal(int64(200))) + }) + + It("keeps the list form when a scalar binds a parenthesized MATCH list", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE message MATCH((?))") + Expect(BindParams(grammar, []*modelv1.TagValue{strParam("error")})).To(Succeed()) + values := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Match.Values + Expect(values.Single).To(BeNil()) + Expect(values.Array).To(HaveLen(1)) + }) + }) + + Describe("parameter type rejections", func() { + It("rejects a timestamp parameter in a scalar comparison", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + err := BindParams(grammar, []*modelv1.TagValue{timestampParam(time.Now())}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("str, int, or null")) + }) + + It("rejects a timestamp parameter in a value list", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id IN (?)") + err := BindParams(grammar, []*modelv1.TagValue{timestampParam(time.Now())}) + Expect(err).To(HaveOccurred()) + }) + + It("rejects a null parameter in the TIME clause", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ?") + err := BindParams(grammar, []*modelv1.TagValue{nullParam()}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("time clause")) + }) + + It("rejects a binary parameter in the TIME clause", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ?") + err := BindParams(grammar, []*modelv1.TagValue{binaryParam([]byte{1})}) + Expect(err).To(HaveOccurred()) + }) + + It("rejects a binary parameter in a value list", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id IN (?)") + err := BindParams(grammar, []*modelv1.TagValue{binaryParam([]byte{1})}) + Expect(err).To(HaveOccurred()) + }) + + It("rejects an int_array parameter in a scalar comparison", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE duration > ?") + err := BindParams(grammar, []*modelv1.TagValue{intArrayParam(1, 2)}) + Expect(err).To(HaveOccurred()) + }) + + It("rejects a nil inner timestamp in the TIME clause instead of decoding it as the epoch", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ?") + err := BindParams(grammar, []*modelv1.TagValue{{Value: &modelv1.TagValue_Timestamp{}}}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid timestamp")) + }) + + It("rejects an out-of-range timestamp in the TIME clause", func() { + grammar := parse("SELECT * FROM STREAM sw IN default TIME > ?") + err := BindParams(grammar, []*modelv1.TagValue{ + {Value: &modelv1.TagValue_Timestamp{Timestamp: ×tamppb.Timestamp{Seconds: math.MaxInt64}}}, + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("invalid timestamp")) + }) + + It("rejects a parameter without a value", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + err := BindParams(grammar, []*modelv1.TagValue{{}}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("has no value")) + }) + + It("rejects a nil parameter entry", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + err := BindParams(grammar, []*modelv1.TagValue{nil}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("has no value")) + }) + + It("reports the 1-based position of the failing parameter", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ? AND duration > ?") + err := BindParams(grammar, []*modelv1.TagValue{strParam("ok"), strArrayParam("bad")}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("parameter #2")) + }) + }) + + Describe("injection safety", func() { + It("keeps an injection payload as a pure string value", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE service_id = ?") + payload := "x' OR '1'='1" + Expect(BindParams(grammar, []*modelv1.TagValue{strParam(payload)})).To(Succeed()) + value := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Compare.Value + Expect(*value.String).To(Equal(payload)) + // The bound value never becomes additional predicates + Expect(grammar.Select.Where.Expr.Left.Right).To(BeEmpty()) + Expect(grammar.Select.Where.Expr.Right).To(BeEmpty()) + }) + + It("does not treat a question mark inside a string literal as a placeholder", func() { + grammar := parse("SELECT * FROM STREAM sw IN default WHERE message = 'why?'") + Expect(BindParams(grammar, nil)).To(Succeed()) + value := grammar.Select.Where.Expr.Left.Left.Binary.Tail.Compare.Value + Expect(*value.String).To(Equal("why?")) + }) + }) +}) diff --git a/pkg/bydbql/grammar.go b/pkg/bydbql/grammar.go index bf3e5acb5..64cf42b1b 100644 --- a/pkg/bydbql/grammar.go +++ b/pkg/bydbql/grammar.go @@ -55,7 +55,8 @@ type GrammarTopNStatement struct { Pos lexer.Position Show string `parser:"@'SHOW'"` Top string `parser:"@'TOP'"` - N int `parser:"@Int"` + N int `parser:"( @Int"` + NParam bool `parser:"| @Param )"` From *GrammarFromClause `parser:"@@"` Time *GrammarTimeClause `parser:"@@?"` Where *GrammarTopNWhereClause `parser:"@@?"` @@ -74,7 +75,8 @@ type GrammarProjection struct { // GrammarTopNProjection represents TOP N projection. type GrammarTopNProjection struct { - N int `parser:"@Int"` + N int `parser:"( @Int"` + NParam bool `parser:"| @Param )"` OrderField *GrammarIdentifierPath `parser:"@@"` Direction *string `parser:"@('ASC'|'DESC')?"` OtherColumns []*GrammarColumn `parser:" ( ',' @@ ( ',' @@ )* )?"` @@ -140,10 +142,11 @@ type GrammarTimeBetween struct { End *GrammarTimeValue `parser:"@@"` } -// GrammarTimeValue represents a time value (string or integer). +// GrammarTimeValue represents a time value (string, integer, or a `?` placeholder). type GrammarTimeValue struct { String *string `parser:" @String"` Integer *int64 `parser:"| @Int"` + Param bool `parser:"| @Param"` } // GrammarSelectWhereClause represents WHERE clause. @@ -251,6 +254,7 @@ type GrammarValue struct { String *string `parser:" @String"` Integer *int64 `parser:"| @Int"` Null bool `parser:"| @'NULL'"` + Param bool `parser:"| @Param"` } // GrammarIdentifierPart Can be either an Ident or a Keyword (keywords are allowed in paths, but not as standalone identifiers). @@ -319,13 +323,15 @@ type GrammarOrderByWithIdent struct { // GrammarLimitClause represents LIMIT clause. type GrammarLimitClause struct { Limit string `parser:"@'LIMIT'"` - Value int `parser:"@Int"` + Value int `parser:"( @Int"` + Param bool `parser:"| @Param )"` } // GrammarOffsetClause represents OFFSET clause. type GrammarOffsetClause struct { Offset string `parser:"@'OFFSET'"` - Value int `parser:"@Int"` + Value int `parser:"( @Int"` + Param bool `parser:"| @Param )"` } // GrammarWithTraceClause represents WITH QUERY_TRACE clause. diff --git a/pkg/bydbql/parser.go b/pkg/bydbql/parser.go index d58cb7c1c..35cbc8e15 100644 --- a/pkg/bydbql/parser.go +++ b/pkg/bydbql/parser.go @@ -52,6 +52,7 @@ func init() { {Name: "Int", Pattern: `[-+]?\d+`}, {Name: "String", Pattern: `'(?:[^'\\]|\\.)*'|"(?:[^"\\]|\\.)*"`}, {Name: "QuotedIdent", Pattern: `"[a-zA-Z_][a-zA-Z0-9_.]*"|'[a-zA-Z_][a-zA-Z0-9_.]*'`}, + {Name: "Param", Pattern: `\?`}, {Name: "Operators", Pattern: `!=|>=|<=|::|[=><,.()*]`}, {Name: "whitespace", Pattern: `\s+`}, }) diff --git a/pkg/bydbql/transformer.go b/pkg/bydbql/transformer.go index ac6d6bc40..f31e5bbd7 100644 --- a/pkg/bydbql/transformer.go +++ b/pkg/bydbql/transformer.go @@ -101,6 +101,11 @@ func NewTransformer(registry metadata.Repo) *Transformer { // Transform transforms a Grammar into a native query request. func (t *Transformer) Transform(ctx context.Context, grammar *Grammar) (*TransformResult, error) { + // Defense in depth: an unbound placeholder would otherwise transform into an + // empty string or a zero count silently instead of failing loudly. + if unbound := countUnboundParams(grammar); unbound > 0 { + return nil, fmt.Errorf("query contains %d unbound placeholder(s); BindParams must be called before Transform", unbound) + } if grammar.Select != nil { // Extract resource type from SELECT statement resourceType := grammar.Select.From.ResourceType diff --git a/pkg/test/helpers/bydbql.go b/pkg/test/helpers/bydbql.go new file mode 100644 index 000000000..0fe090524 --- /dev/null +++ b/pkg/test/helpers/bydbql.go @@ -0,0 +1,63 @@ +// Licensed to Apache Software Foundation (ASF) under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Apache Software Foundation (ASF) licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package helpers + +import ( + "encoding/json" + "fmt" + "strings" + + "google.golang.org/protobuf/encoding/protojson" + + modelv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/model/v1" +) + +// ExtractQL assembles a BydbQL query string from a .ql file's content, skipping +// blank and comment lines, and decodes the optional `#!params:` directive into +// TagValues bound to the query's `?` placeholders. The directive value is a JSON +// array of protojson-encoded model.v1.TagValue objects, e.g. +// `#!params: [{"str":{"value":"-15m"}},{"int":{"value":"10"}}]`. +func ExtractQL(content string) (string, []*modelv1.TagValue, error) { + var query strings.Builder + var params []*modelv1.TagValue + for _, line := range strings.Split(content, "\n") { + trimmed := strings.TrimSpace(line) + if paramsJSON, found := strings.CutPrefix(trimmed, "#!params:"); found { + var rawParams []json.RawMessage + if err := json.Unmarshal([]byte(paramsJSON), &rawParams); err != nil { + return "", nil, fmt.Errorf("failed to decode #!params directive: %w", err) + } + for _, rawParam := range rawParams { + param := &modelv1.TagValue{} + if err := protojson.Unmarshal(rawParam, param); err != nil { + return "", nil, fmt.Errorf("failed to decode #!params entry %s: %w", rawParam, err) + } + params = append(params, param) + } + continue + } + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + if query.Len() > 0 { + query.WriteByte(' ') + } + query.WriteString(trimmed) + } + return query.String(), params, nil +} diff --git a/test/cases/measure/data/data.go b/test/cases/measure/data/data.go index ae6e582c3..a343acd86 100644 --- a/test/cases/measure/data/data.go +++ b/test/cases/measure/data/data.go @@ -157,18 +157,8 @@ func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Ar } qlContent, err := qlFS.ReadFile("input/" + args.Input + ".ql") innerGm.Expect(err).NotTo(gm.HaveOccurred()) - - var qlQueryStr string - for _, line := range strings.Split(string(qlContent), "\n") { - trimmed := strings.TrimSpace(line) - if trimmed == "" || strings.HasPrefix(trimmed, "#") { - continue - } - if qlQueryStr != "" { - qlQueryStr += " " - } - qlQueryStr += trimmed - } + qlQueryStr, qlParams, err := helpers.ExtractQL(string(qlContent)) + innerGm.Expect(err).NotTo(gm.HaveOccurred()) // Auto-inject stages clause if args.Stages is not empty if len(args.Stages) > 0 { @@ -196,6 +186,7 @@ func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Ar parsed, errStrs := bydbql.ParseQuery(qlQueryStr) innerGm.Expect(errStrs).To(gm.BeNil()) + innerGm.Expect(bydbql.BindParams(parsed, qlParams)).To(gm.Succeed()) transformer := bydbql.NewTransformer(mockRepo) result, err := transformer.Transform(ctx, parsed) @@ -215,7 +206,8 @@ func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Ar bydbqlClient := bydbqlv1.NewBydbQLServiceClient(conn) bydbqlResp, err := bydbqlClient.Query(ctx, &bydbqlv1.QueryRequest{ - Query: qlQueryStr, + Query: qlQueryStr, + Params: qlParams, }) innerGm.Expect(err).NotTo(gm.HaveOccurred()) innerGm.Expect(bydbqlResp).NotTo(gm.BeNil()) diff --git a/test/cases/measure/data/input/params_bind.ql b/test/cases/measure/data/input/params_bind.ql new file mode 100644 index 000000000..842e284c3 --- /dev/null +++ b/test/cases/measure/data/input/params_bind.ql @@ -0,0 +1,22 @@ +# Licensed to Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Apache Software Foundation (ASF) licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +SELECT name, short_name FROM MEASURE service_traffic IN index_mode +TIME > ? +WHERE id IN (?) +#!params: [{"str":{"value":"-15m"}},{"strArray":{"value":["1","2"]}}] diff --git a/test/cases/measure/data/input/params_bind.yaml b/test/cases/measure/data/input/params_bind.yaml new file mode 100644 index 000000000..f002dd2eb --- /dev/null +++ b/test/cases/measure/data/input/params_bind.yaml @@ -0,0 +1,30 @@ +# Licensed to Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Apache Software Foundation (ASF) licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: "service_traffic" +groups: ["index_mode"] +tagProjection: + tagFamilies: + - name: "default" + tags: ["name", "short_name"] +criteria: + condition: + name: "id" + op: "BINARY_OP_IN" + value: + str_array: + value: ["1", "2"] diff --git a/test/cases/measure/measure.go b/test/cases/measure/measure.go index 4594dfeb6..ae5c8b0cf 100644 --- a/test/cases/measure/measure.go +++ b/test/cases/measure/measure.go @@ -70,6 +70,7 @@ var measureEntries = []any{ g.Entry("match nodes", helpers.Args{Input: "match_nodes", Duration: 25 * time.Minute, Offset: -20 * time.Minute}), g.Entry("filter by entity id", helpers.Args{Input: "entity", Duration: 25 * time.Minute, Offset: -20 * time.Minute, DisOrder: true}), g.Entry("filter by several entity ids", helpers.Args{Input: "entity_in", Duration: 25 * time.Minute, Offset: -20 * time.Minute, DisOrder: true}), + g.Entry("filter with bound parameters", helpers.Args{Input: "params_bind", Want: "entity_in", Duration: 25 * time.Minute, Offset: -20 * time.Minute, DisOrder: true}), g.Entry("filter by entity id and service id", helpers.Args{Input: "entity_service", Duration: 25 * time.Minute, Offset: -20 * time.Minute, DisOrder: true}), g.Entry("without field", helpers.Args{Input: "no_field", Duration: 25 * time.Minute, Offset: -20 * time.Minute, DisOrder: true}), g.Entry("invalid logical expression", helpers.Args{Input: "err_invalid_le", Duration: 25 * time.Minute, Offset: -20 * time.Minute, WantErr: true}), diff --git a/test/cases/property/data/data.go b/test/cases/property/data/data.go index 865f075e8..9fbb4ce15 100644 --- a/test/cases/property/data/data.go +++ b/test/cases/property/data/data.go @@ -135,18 +135,8 @@ func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Ar } qlContent, err := qlFS.ReadFile("input/" + args.Input + ".ql") innerGm.Expect(err).NotTo(gm.HaveOccurred()) - - var qlQueryStr string - for _, line := range strings.Split(string(qlContent), "\n") { - trimmed := strings.TrimSpace(line) - if trimmed == "" || strings.HasPrefix(trimmed, "#") { - continue - } - if qlQueryStr != "" { - qlQueryStr += " " - } - qlQueryStr += trimmed - } + qlQueryStr, qlParams, err := helpers.ExtractQL(string(qlContent)) + innerGm.Expect(err).NotTo(gm.HaveOccurred()) ctrl := gomock.NewController(g.GinkgoT()) defer ctrl.Finish() @@ -165,6 +155,7 @@ func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Ar parsed, errStrs := bydbql.ParseQuery(qlQueryStr) innerGm.Expect(errStrs).To(gm.BeNil()) + innerGm.Expect(bydbql.BindParams(parsed, qlParams)).To(gm.Succeed()) transformer := bydbql.NewTransformer(mockRepo) result, err := transformer.Transform(ctx, parsed) @@ -180,7 +171,8 @@ func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Ar bydbqlClient := bydbqlv1.NewBydbQLServiceClient(conn) bydbqlResp, err := bydbqlClient.Query(ctx, &bydbqlv1.QueryRequest{ - Query: qlQueryStr, + Query: qlQueryStr, + Params: qlParams, }) innerGm.Expect(err).NotTo(gm.HaveOccurred()) innerGm.Expect(bydbqlResp).NotTo(gm.BeNil()) diff --git a/test/cases/property/data/input/params_bind.ql b/test/cases/property/data/input/params_bind.ql new file mode 100644 index 000000000..d03313ffc --- /dev/null +++ b/test/cases/property/data/input/params_bind.ql @@ -0,0 +1,21 @@ +# Licensed to Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Apache Software Foundation (ASF) licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +SELECT menu_name, configuration, update_time FROM PROPERTY ui_menu IN sw +WHERE menu_name = ? +#!params: [{"str":{"value":"test1"}}] diff --git a/test/cases/property/data/input/params_bind.yaml b/test/cases/property/data/input/params_bind.yaml new file mode 100644 index 000000000..757cf5c96 --- /dev/null +++ b/test/cases/property/data/input/params_bind.yaml @@ -0,0 +1,30 @@ +# Licensed to Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Apache Software Foundation (ASF) licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: "ui_menu" +groups: ["sw"] +tagProjection: + - menu_name + - configuration + - update_time +criteria: + condition: + name: "menu_name" + op: "BINARY_OP_EQ" + value: + str: + value: "test1" \ No newline at end of file diff --git a/test/cases/property/property.go b/test/cases/property/property.go index 36cb20c87..985247591 100644 --- a/test/cases/property/property.go +++ b/test/cases/property/property.go @@ -42,6 +42,7 @@ var propertyEntries = []any{ g.Entry("all", helpers.Args{Input: "all"}), g.Entry("limit", helpers.Args{Input: "limit"}), g.Entry("query by criteria", helpers.Args{Input: "query_by_criteria"}), + g.Entry("query by criteria with bound parameters", helpers.Args{Input: "params_bind", Want: "query_by_criteria"}), g.Entry("query by ids", helpers.Args{Input: "query_by_ids"}), g.Entry("order by asc", helpers.Args{Input: "order_by_asc"}), g.Entry("order by desc", helpers.Args{Input: "order_by_desc"}), diff --git a/test/cases/stream/data/data.go b/test/cases/stream/data/data.go index 059598727..e8118dbff 100644 --- a/test/cases/stream/data/data.go +++ b/test/cases/stream/data/data.go @@ -210,16 +210,8 @@ func loadDataWithSpec(stream streamv1.StreamService_WriteClient, md *commonv1.Me func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *streamv1.QueryRequest, conn *grpclib.ClientConn) { qlContent, err := qlFS.ReadFile("input/" + args.Input + ".ql") innerGm.Expect(err).NotTo(gm.HaveOccurred()) - var qlQueryStr string - for _, line := range strings.Split(string(qlContent), "\n") { - trimmed := strings.TrimSpace(line) - if trimmed != "" && !strings.HasPrefix(trimmed, "#") { - if qlQueryStr != "" { - qlQueryStr += " " - } - qlQueryStr += trimmed - } - } + qlQueryStr, qlParams, err := helpers.ExtractQL(string(qlContent)) + innerGm.Expect(err).NotTo(gm.HaveOccurred()) // Auto-inject stages clause if args.Stages is not empty if len(args.Stages) > 0 { @@ -248,6 +240,7 @@ func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *stream // parse QL to QueryRequest query, errStrs := bydbql.ParseQuery(qlQueryStr) innerGm.Expect(errStrs).To(gm.BeNil()) + innerGm.Expect(bydbql.BindParams(query, qlParams)).To(gm.Succeed()) transformer := bydbql.NewTransformer(mockRepo) transformCtx, transformCancel := context.WithTimeout(context.Background(), 10*time.Second) defer transformCancel() @@ -274,7 +267,8 @@ func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *stream qlCtx, qlCancel := context.WithTimeout(context.Background(), 10*time.Second) defer qlCancel() bydbqlResp, err := client.Query(qlCtx, &bydbqlv1.QueryRequest{ - Query: qlQueryStr, + Query: qlQueryStr, + Params: qlParams, }) if args.WantErr { innerGm.Expect(err).To(gm.HaveOccurred()) diff --git a/test/cases/stream/data/input/params_bind.ql b/test/cases/stream/data/input/params_bind.ql new file mode 100644 index 000000000..71b5fc86c --- /dev/null +++ b/test/cases/stream/data/input/params_bind.ql @@ -0,0 +1,21 @@ +# Licensed to Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Apache Software Foundation (ASF) licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +SELECT trace_id, span_id FROM STREAM sw IN default +TIME > ? +WHERE trace_id = ? AND span_id = ? +#!params: [{"str":{"value":"-15m"}},{"str":{"value":"1"}},{"str":{"value":"1"}}] diff --git a/test/cases/stream/data/input/params_bind.yaml b/test/cases/stream/data/input/params_bind.yaml new file mode 100644 index 000000000..ffb8b211f --- /dev/null +++ b/test/cases/stream/data/input/params_bind.yaml @@ -0,0 +1,40 @@ +# Licensed to Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Apache Software Foundation (ASF) licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: "sw" +groups: ["default"] +projection: + tagFamilies: + - name: "searchable" + tags: ["trace_id", "span_id"] +criteria: + le: + op: "LOGICAL_OP_AND" + right: + condition: + name: "span_id" + op: "BINARY_OP_EQ" + value: + str: + value: "1" + left: + condition: + name: "trace_id" + op: "BINARY_OP_EQ" + value: + str: + value: "1" diff --git a/test/cases/stream/stream.go b/test/cases/stream/stream.go index a5520e420..31891cd93 100644 --- a/test/cases/stream/stream.go +++ b/test/cases/stream/stream.go @@ -70,6 +70,7 @@ var streamEntries = []any{ g.Entry("global index", helpers.Args{Input: "global_index", Duration: 1 * time.Hour}), g.Entry("multi-global index", helpers.Args{Input: "global_indices", Duration: 1 * time.Hour}), g.Entry("filter by non-indexed tag", helpers.Args{Input: "filter_tag", Duration: 1 * time.Hour}), + g.Entry("filter with bound parameters", helpers.Args{Input: "params_bind", Want: "filter_tag", Duration: 1 * time.Hour}), g.Entry("filter hidden tag projection", helpers.Args{Input: "filter_hidden_tag", Duration: 1 * time.Hour}), g.Entry("filter by non-indexed tag order by duration desc with limit 3", helpers.Args{Input: "sort_duration_no_index_limit", Duration: 1 * time.Hour}), g.Entry("get empty result by non-indexed tag", helpers.Args{Input: "filter_tag_empty", Duration: 1 * time.Hour, WantEmpty: true}), diff --git a/test/cases/topn/data/data.go b/test/cases/topn/data/data.go index 4f9c175a5..1fd44b33d 100644 --- a/test/cases/topn/data/data.go +++ b/test/cases/topn/data/data.go @@ -229,16 +229,8 @@ func tagValOrder(v *modelv1.TagValue) int { func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Args, yamlQuery *measurev1.TopNRequest, conn *grpclib.ClientConn) { qlContent, err := qlFS.ReadFile("input/" + args.Input + ".ql") innerGm.Expect(err).NotTo(gm.HaveOccurred()) - var qlQueryStr string - for _, line := range strings.Split(string(qlContent), "\n") { - trimmed := strings.TrimSpace(line) - if trimmed != "" && !strings.HasPrefix(trimmed, "#") { - if qlQueryStr != "" { - qlQueryStr += " " - } - qlQueryStr += trimmed - } - } + qlQueryStr, qlParams, err := helpers.ExtractQL(string(qlContent)) + innerGm.Expect(err).NotTo(gm.HaveOccurred()) // Auto-inject stages clause if args.Stages is not empty if len(args.Stages) > 0 { @@ -277,6 +269,7 @@ func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Ar // parse QL to QueryRequest query, errStrs := bydbql.ParseQuery(qlQueryStr) innerGm.Expect(errStrs).To(gm.BeNil()) + innerGm.Expect(bydbql.BindParams(query, qlParams)).To(gm.Succeed()) transformer := bydbql.NewTransformer(mockRepo) transform, err := transformer.Transform(ctx, query) innerGm.Expect(err).NotTo(gm.HaveOccurred()) @@ -296,7 +289,8 @@ func verifyQLWithRequest(ctx context.Context, innerGm gm.Gomega, args helpers.Ar // simple check the QL can be executed client := bydbqlv1.NewBydbQLServiceClient(conn) bydbqlResp, err := client.Query(ctx, &bydbqlv1.QueryRequest{ - Query: qlQueryStr, + Query: qlQueryStr, + Params: qlParams, }) if args.WantErr { innerGm.Expect(err).To(gm.HaveOccurred()) diff --git a/test/cases/topn/topn.go b/test/cases/topn/topn.go index 40b858881..5dcb3fe94 100644 --- a/test/cases/topn/topn.go +++ b/test/cases/topn/topn.go @@ -45,6 +45,7 @@ var ( var topnEntries = []any{ g.Entry("max top3 order by desc", helpers.Args{Input: "aggr_desc", Duration: 25 * time.Minute, Offset: -20 * time.Minute}), g.Entry("max top3 with condition order by desc", helpers.Args{Input: "condition_aggr_desc", Duration: 25 * time.Minute, Offset: -20 * time.Minute}), + g.Entry("condition with bound parameters", helpers.Args{Input: "params_bind", Want: "condition_aggr_desc", Duration: 25 * time.Minute, Offset: -20 * time.Minute}), g.Entry("max top3 for null group order by desc", helpers.Args{Input: "null_group", Duration: 25 * time.Minute, Offset: -20 * time.Minute}), g.Entry("multi-group: max top3 order by desc", helpers.Args{Input: "multi_group_aggr_desc", Want: "aggr_desc", Duration: 25 * time.Minute, Offset: -20 * time.Minute}), g.Entry("using equal in aggregation", helpers.Args{Input: "eq", Duration: 25 * time.Minute, Offset: -20 * time.Minute}), diff --git a/test/cases/trace/data/data.go b/test/cases/trace/data/data.go index 816812346..2db1c9b99 100644 --- a/test/cases/trace/data/data.go +++ b/test/cases/trace/data/data.go @@ -184,18 +184,8 @@ var VerifyFn = func(innerGm gm.Gomega, sharedContext helpers.SharedContext, args func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *tracev1.QueryRequest, conn *grpclib.ClientConn) { qlContent, err := qlFS.ReadFile("input/" + args.Input + ".ql") innerGm.Expect(err).NotTo(gm.HaveOccurred()) - - var qlQueryStr string - for _, line := range strings.Split(string(qlContent), "\n") { - trimmed := strings.TrimSpace(line) - if trimmed == "" || strings.HasPrefix(trimmed, "#") { - continue - } - if qlQueryStr != "" { - qlQueryStr += " " - } - qlQueryStr += trimmed - } + qlQueryStr, qlParams, err := helpers.ExtractQL(string(qlContent)) + innerGm.Expect(err).NotTo(gm.HaveOccurred()) // Auto-inject stages clause if args.Stages is not empty if len(args.Stages) > 0 { @@ -223,6 +213,7 @@ func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *tracev parsed, errStrs := bydbql.ParseQuery(qlQueryStr) innerGm.Expect(errStrs).To(gm.BeNil()) + innerGm.Expect(bydbql.BindParams(parsed, qlParams)).To(gm.Succeed()) transformer := bydbql.NewTransformer(mockRepo) transformCtx, transformCancel := context.WithTimeout(context.Background(), 10*time.Second) @@ -246,7 +237,8 @@ func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *tracev qlCtx, qlCancel := context.WithTimeout(context.Background(), 10*time.Second) defer qlCancel() bydbqlResp, err := bydbqlClient.Query(qlCtx, &bydbqlv1.QueryRequest{ - Query: qlQueryStr, + Query: qlQueryStr, + Params: qlParams, }) innerGm.Expect(err).NotTo(gm.HaveOccurred()) innerGm.Expect(bydbqlResp).NotTo(gm.BeNil()) diff --git a/test/cases/trace/data/input/params_bind.ql b/test/cases/trace/data/input/params_bind.ql new file mode 100644 index 000000000..0da14e4a2 --- /dev/null +++ b/test/cases/trace/data/input/params_bind.ql @@ -0,0 +1,24 @@ +# Licensed to Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Apache Software Foundation (ASF) licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + + +SELECT () FROM TRACE zipkin IN zipkinTrace +TIME > ? +WHERE duration >= ? AND duration <= ? +ORDER BY zipkin-timestamp DESC +LIMIT ? +#!params: [{"str":{"value":"-1h"}},{"int":{"value":"10"}},{"int":{"value":"1000"}},{"int":{"value":"10"}}] diff --git a/test/cases/trace/data/input/params_bind.yml b/test/cases/trace/data/input/params_bind.yml new file mode 100644 index 000000000..a37f54044 --- /dev/null +++ b/test/cases/trace/data/input/params_bind.yml @@ -0,0 +1,40 @@ +# Licensed to Apache Software Foundation (ASF) under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Apache Software Foundation (ASF) licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +name: "zipkin" +groups: ["zipkinTrace"] +limit: 10 +order_by: + index_rule_name: "zipkin-timestamp" + sort: "SORT_DESC" +criteria: + le: + op: "LOGICAL_OP_AND" + left: + condition: + name: "duration" + op: "BINARY_OP_GE" + value: + int: + value: 10 + right: + condition: + name: "duration" + op: "BINARY_OP_LE" + value: + int: + value: 1000 diff --git a/test/cases/trace/trace.go b/test/cases/trace/trace.go index e3afe26d5..75c79ee68 100644 --- a/test/cases/trace/trace.go +++ b/test/cases/trace/trace.go @@ -51,6 +51,8 @@ var traceEntries = []any{ g.Entry("order by duration", helpers.Args{Input: "order_duration_desc", Duration: 1 * time.Hour}), g.Entry("duration range 10-1000 order by timestamp", helpers.Args{Input: "duration_range_order_timestamp", Duration: 1 * time.Hour}), + g.Entry("duration range with bound parameters", + helpers.Args{Input: "params_bind", Want: "duration_range_order_timestamp", Duration: 1 * time.Hour}), g.Entry("duration range and ipv4 filter order by timestamp", helpers.Args{Input: "duration_range_and_ipv4_order_timestamp", Duration: 1 * time.Hour}), g.Entry("filter by service id", helpers.Args{Input: "eq_service_order_timestamp_desc", Duration: 1 * time.Hour}), diff --git a/test/cases/tracepipeline/data/data.go b/test/cases/tracepipeline/data/data.go index 017712e39..47082f227 100644 --- a/test/cases/tracepipeline/data/data.go +++ b/test/cases/tracepipeline/data/data.go @@ -154,18 +154,8 @@ var VerifyFn = func(innerGm gm.Gomega, sharedContext helpers.SharedContext, args func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *tracev1.QueryRequest, conn *grpclib.ClientConn) { qlContent, err := qlFS.ReadFile("input/" + args.Input + ".ql") innerGm.Expect(err).NotTo(gm.HaveOccurred()) - - var qlQueryStr string - for _, line := range strings.Split(string(qlContent), "\n") { - trimmed := strings.TrimSpace(line) - if trimmed == "" || strings.HasPrefix(trimmed, "#") { - continue - } - if qlQueryStr != "" { - qlQueryStr += " " - } - qlQueryStr += trimmed - } + qlQueryStr, qlParams, err := helpers.ExtractQL(string(qlContent)) + innerGm.Expect(err).NotTo(gm.HaveOccurred()) // Auto-inject stages clause if args.Stages is not empty. if len(args.Stages) > 0 { @@ -191,6 +181,7 @@ func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *tracev parsed, errStrs := bydbql.ParseQuery(qlQueryStr) innerGm.Expect(errStrs).To(gm.BeNil()) + innerGm.Expect(bydbql.BindParams(parsed, qlParams)).To(gm.Succeed()) transformer := bydbql.NewTransformer(mockRepo) transformCtx, transformCancel := context.WithTimeout(context.Background(), 10*time.Second) @@ -214,7 +205,8 @@ func verifyQLWithRequest(innerGm gm.Gomega, args helpers.Args, yamlQuery *tracev qlCtx, qlCancel := context.WithTimeout(context.Background(), 10*time.Second) defer qlCancel() bydbqlResp, qlErr := bydbqlClient.Query(qlCtx, &bydbqlv1.QueryRequest{ - Query: qlQueryStr, + Query: qlQueryStr, + Params: qlParams, }) innerGm.Expect(qlErr).NotTo(gm.HaveOccurred()) innerGm.Expect(bydbqlResp).NotTo(gm.BeNil()) diff --git a/ui/src/components/BydbQL/Index.vue b/ui/src/components/BydbQL/Index.vue index 419da41e0..b6539aa51 100644 --- a/ui/src/components/BydbQL/Index.vue +++ b/ui/src/components/BydbQL/Index.vue @@ -49,6 +49,10 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`); const executionTime = ref(0); const codeMirrorInstance = ref(null); const editorLoading = ref(true); + // Positional parameters bound to `?` placeholders in the query, in order of appearance + const queryParams = ref([]); + const paramTypes = ['str', 'int', 'str_array', 'int_array', 'null']; + const integerPattern = /^-?\d+$/; const hasResult = computed(() => queryResult.value !== null); const resultType = computed(() => { @@ -262,7 +266,21 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`); .join('\n') .trim(); - const response = await executeBydbQLQuery({ query: cleanQuery }); + let boundParams; + try { + boundParams = buildBoundParams(); + } catch (paramError) { + loading.value = false; + error.value = paramError.message; + ElMessage.error(paramError.message); + return; + } + + const requestBody = { query: cleanQuery }; + if (boundParams.length > 0) { + requestBody.params = boundParams; + } + const response = await executeBydbQLQuery(requestBody); const endTime = performance.now(); loading.value = false; executionTime.value = Math.round(endTime - startTime); @@ -281,6 +299,62 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`); queryResult.value = null; error.value = null; executionTime.value = 0; + queryParams.value = []; + } + + function addParam() { + queryParams.value.push({ type: 'str', value: '' }); + } + + function removeParam(index) { + queryParams.value.splice(index, 1); + } + + function splitArrayValue(raw, position) { + const entries = raw + .split(',') + .map((entry) => entry.trim()) + .filter((entry) => entry !== ''); + if (entries.length === 0) { + throw new Error(`${position} must contain at least one value`); + } + return entries; + } + + // Convert the parameter rows to protojson TagValue form accepted by the HTTP API. + // Keep in sync with toTagValueParams in mcp/src/query/validation.ts, which builds + // the same shapes independently. The server-side timestamp variant is deliberately + // not exposed here: an RFC3339 str is equivalent for TIME positions. + function buildBoundParams() { + return queryParams.value.map((param, index) => { + const position = `Parameter #${index + 1}`; + switch (param.type) { + case 'str': + return { str: { value: param.value } }; + case 'int': { + const trimmed = param.value.trim(); + if (!integerPattern.test(trimmed)) { + throw new Error(`${position} must be an integer`); + } + return { int: { value: trimmed } }; + } + case 'str_array': + return { strArray: { value: splitArrayValue(param.value, position) } }; + case 'int_array': { + const entries = splitArrayValue(param.value, position); + for (const entry of entries) { + if (!integerPattern.test(entry)) { + throw new Error(`${position} must be a comma-separated list of integers`); + } + } + return { intArray: { value: entries } }; + } + case 'null': + return { null: null }; + default: + throw new Error(`${position} has an unknown type`); + } + }); } function onCodeMirrorReady(cm) { @@ -584,6 +658,26 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`); @ready="onCodeMirrorReady" /> </div> + <div class="query-params-container"> + <div class="params-header"> + <span class="params-title">Parameters (bound to ? placeholders in order)</span> + <el-button size="small" @click="addParam" :disabled="loading"> Add Parameter </el-button> + </div> + <div v-for="(param, index) in queryParams" :key="index" class="param-row"> + <span class="param-index">?{{ index + 1 }}</span> + <el-select v-model="param.type" size="small" class="param-type"> + <el-option v-for="paramType in paramTypes" :key="paramType" :label="paramType" :value="paramType" /> + </el-select> + <el-input + v-model="param.value" + size="small" + class="param-value" + :disabled="param.type === 'null'" + :placeholder="param.type.endsWith('_array') ? 'Comma-separated values' : 'Value'" + /> + <el-button size="small" type="danger" text @click="removeParam(index)"> Remove </el-button> + </div> + </div> </el-card> <el-card shadow="always" class="result-card"> <template #header> @@ -675,6 +769,45 @@ SELECT * FROM STREAM log in sw_recordsLog TIME > '-30m'`); gap: 10px; } + .query-params-container { + margin-top: 10px; + display: flex; + flex-direction: column; + gap: 8px; + } + + .params-header { + display: flex; + justify-content: space-between; + align-items: center; + } + + .params-title { + font-size: 13px; + color: var(--el-text-color-secondary); + } + + .param-row { + display: flex; + align-items: center; + gap: 8px; + } + + .param-index { + font-family: monospace; + font-size: 12px; + color: var(--el-text-color-secondary); + min-width: 24px; + } + + .param-type { + width: 120px; + } + + .param-value { + flex: 1; + } + .header-right { display: flex; align-items: center;
