Merge remote-tracking branch 'origin/main' into issue_5601_spanlist

.claude/rules/comments.md

@@ -1,11 +0,0 @@
-# Comments
-
-Applies to everything in the repo — code, config, workflows.
-
-- **No unnecessary comments.** Do not comment where the code is self-explanatory; never restate what the code already says.
-- **Document only** non-obvious behavior, constraints, formats, and edge cases.
-- **Rationale goes in prose, not source.** Why a version is pinned, why a job exists, how a subsystem fits together — that belongs in the README or the PR.
-- **Never remove pre-existing comments** when editing code. The bar above applies to comments you write, not comments already there.
-- **Never talk to the reviewer.** No comments about where a change came from, what was changed, or why the change is correct — that belongs in the PR description and is noise the moment it merges.
-
-Language rules build on this one: [`go-comments`](go-comments.md), [`py-comments`](py-comments.md).

.claude/rules/go-comments.md

@@ -1,12 +0,0 @@
----
-paths:
-  - "**/*.go"
----
-
-# Go comments
-
-The bar is the [`comments`](comments.md) rule: nothing where the code is self-explanatory.
-
-- **Names carry the meaning.** Make function, type, and variable names self-explanatory so the comment is unnecessary in the first place. If a comment is needed to explain what a function does, fix the name, not the comment.
-- **Godoc**: Skip comments that merely restate the identifier. Document only non-obvious behavior, constraints, formats, and edge cases.
-- **Generated code**: If the comment is emitted by an external codegen tool, leave it as-is — do not add or trim comments in generated files.

.claude/rules/pull-requests.md

@@ -1,7 +0,0 @@
-# Pull requests
-
-- **Follow the template** (`.github/pull_request_template.md`): fill in its headings (Description / Issues closed by this PR / Screenshots / Additional Information). Don't add sections the template doesn't have.
-- **Keep only the headings that apply.** Delete every heading that has nothing under it, along with its `<!--...-->` placeholder comment. The body must never contain an empty heading — if only Description applies, the body has exactly that one heading.
-- **Keep the description concise and human-readable.** A few plain bullets saying what changed and why, for a reviewer skimming it — not a wall of text, not a restatement of the diff, not generated boilerplate.
-- **Reference issues with `Closes #issue-number`** under "Issues closed by this PR" so they auto-close on merge. This goes in the PR description only — never in commit messages.
-- **AI assistance in commits may optionally be disclosed with an `Assisted-by:` trailer** naming the model (e.g. `Assisted-by: Claude Opus 4.5`) — do NOT use a `Co-authored-by:` trailer for this.

.claude/rules/py-comments.md

@@ -1,13 +0,0 @@
----
-paths:
-  - "**/*.py"
----
-
-# Python comments
-
-The bar is the [`comments`](comments.md) rule: nothing where the code is self-explanatory.
-
-- **Names carry the meaning.** Make function and variable names self-explanatory so the comment or docstring is unnecessary in the first place. If a docstring is needed to explain what a function does, fix the name, not the docstring.
-- **No file-level docstring.** The filename says what the module is for — `tool_bin.py` gets the tool binary. A module docstring restating that is noise, and a paragraph of design prose at the top of a file goes stale where nobody is looking. A constraint belongs next to the code it constrains, not in a preamble.
-- **Docstrings**: only when they say something the name and signature don't — drop them otherwise. Keep them short. A contract that genuinely needs a few lines (interacting flags, retry semantics, an edge case) is fine; a narrative is not.
-- **No song and dance.** Comment the constraint or the edge case. Not the narrative, not the rationale, not what the next line does.

.github/pull_request_template.md

@@ -1,13 +1,85 @@
-<!--A few plain bullets saying what changed and why, for a reviewer skimming it - not a wall of text, not a restatement of the diff, not generated boilerplate.-->
-#### Description
+## Pull Request
+
+---
+
+### 📄 Summary
+> Why does this change exist?  
+> What problem does it solve, and why is this the right approach?
+
+
+
+#### Screenshots / Screen Recordings (if applicable)
+> Include screenshots or screen recordings that clearly show the behavior before the change and the result after the change. This helps reviewers quickly understand the impact and verify the update.
+
 
-<!--Reference issues using `Closes #issue-number` to enable automatic closure on merge. -->
 #### Issues closed by this PR
+> Reference issues using `Closes #issue-number` to enable automatic closure on merge.
 
-<!--If applicable, include screenshots or screen recordings that clearly show the behavior before the change and the result after the change. -->
-#### Screenshots / Screen Recordings
+---
 
-<!--Anything reviewers should keep in mind while reviewing -->
-#### Additional Information
+### ✅ Change Type
+_Select all that apply_
 
-<!--Please delete paragraphs that you did not use before submitting.-->
+- [ ] ✨ Feature
+- [ ] 🐛 Bug fix
+- [ ] ♻️ Refactor
+- [ ] 🛠️ Infra / Tooling
+- [ ] 🧪 Test-only
+
+---
+
+### 🐛 Bug Context
+> Required if this PR fixes a bug
+
+#### Root Cause
+> What caused the issue?  
+> Regression, faulty assumption, edge case, refactor, etc.
+
+#### Fix Strategy
+> How does this PR address the root cause?
+
+---
+
+### 🧪 Testing Strategy
+> How was this change validated?
+
+- Tests added/updated:
+- Manual verification:
+- Edge cases covered:
+
+---
+
+### ⚠️ Risk & Impact Assessment
+> What could break? How do we recover?
+
+- Blast radius:
+- Potential regressions:
+- Rollback plan:
+
+---
+
+### 📝 Changelog
+> Fill only if this affects users, APIs, UI, or documented behavior  
+> Use **N/A** for internal or non-user-facing changes
+
+| Field | Value |
+|------|-------|
+| Deployment Type | Cloud / OSS / Enterprise |
+| Change Type | Feature / Bug Fix / Maintenance |
+| Description | User-facing summary |
+
+---
+
+### 📋 Checklist
+- [ ] Tests added or explicitly not required
+- [ ] Manually tested
+- [ ] Breaking changes documented
+- [ ] Backward compatibility considered
+
+---
+
+## 👀 Notes for Reviewers
+
+<!-- Anything reviewers should keep in mind while reviewing -->
+
+---

.gitignore

@@ -90,6 +90,8 @@ queries.active
 .devenv/**/tmp/**
 .qodo
 
+.dev
+
 ### Python ###
 # Byte-compiled / optimized / DLL files
 __pycache__/
@@ -229,6 +231,4 @@ cython_debug/
 # LSP config files
 pyrightconfig.json
 
-# dev
-.dev/
-.claude/worktrees/
+

go.mod

@@ -4,7 +4,7 @@ go 1.25.7
 
 require (
 	dario.cat/mergo v1.0.2
-	github.com/AfterShip/clickhouse-sql-parser v0.5.5
+	github.com/AfterShip/clickhouse-sql-parser v0.5.4
 	github.com/ClickHouse/clickhouse-go/v2 v2.44.0
 	github.com/DATA-DOG/go-sqlmock v1.5.2
 	github.com/SigNoz/clickhouse-go-mock v0.14.0

go.sum

@@ -66,8 +66,8 @@ dario.cat/mergo v1.0.2/go.mod h1:E/hbnu0NxMFBjpMIE34DRGLWqDy0g5FuKDhCb31ngxA=
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
 filippo.io/edwards25519 v1.2.0 h1:crnVqOiS4jqYleHd9vaKZ+HKtHfllngJIiOpNpoJsjo=
 filippo.io/edwards25519 v1.2.0/go.mod h1:xzAOLCNug/yB62zG1bQ8uziwrIqIuxhctzJT18Q77mc=
-github.com/AfterShip/clickhouse-sql-parser v0.5.5 h1:LCA23yAA4GgF73PoYXb67yzCdC4sXsj4geQz1Oij3U8=
-github.com/AfterShip/clickhouse-sql-parser v0.5.5/go.mod h1:Qi3qvPTfZb/aFwI5V4WFOahgjsLJa4MzVijIAfwOhDw=
+github.com/AfterShip/clickhouse-sql-parser v0.5.4 h1:yiCQaMq8EO+dpKdnpP9YYd/ne6MSuOXgsMsNL33NiTI=
+github.com/AfterShip/clickhouse-sql-parser v0.5.4/go.mod h1:Qi3qvPTfZb/aFwI5V4WFOahgjsLJa4MzVijIAfwOhDw=
 github.com/Azure/azure-sdk-for-go v68.0.0+incompatible h1:fcYLmCpyNYRnvJbPerq7U0hS+6+I79yEDJBqVNcqUzU=
 github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.0 h1:fou+2+WFTib47nS+nz/ozhEBnvU96bKHy6LjRsY4E28=
 github.com/Azure/azure-sdk-for-go/sdk/azcore v1.21.0/go.mod h1:t76Ruy8AHvUAC8GfMWJMa0ElSbuIcO03NLpynfbgsPA=

pkg/querybuilder/clickhouse_sql.go

@@ -17,7 +17,6 @@ var (
 	CodeClickHouseSQLNotSingleStatement = errors.MustNewCode("clickhouse_sql_not_single_statement")
 	CodeClickHouseSQLNotSelect          = errors.MustNewCode("clickhouse_sql_not_select")
 	CodeClickHouseSQLTableFunction      = errors.MustNewCode("clickhouse_sql_table_function")
-	CodeClickHouseSQLReadingFunction    = errors.MustNewCode("clickhouse_sql_reading_function")
 	CodeClickHouseSQLInternalDatabase   = errors.MustNewCode("clickhouse_sql_internal_database")
 	CodeClickHouseSQLReadonlyOverride   = errors.MustNewCode("clickhouse_sql_readonly_override")
 )
@@ -44,25 +43,6 @@ var generatorTableFunctions = map[string]string{
 
 var generatorTableFunctionsMessage = "allowed table functions are " + strings.Join(slices.Sorted(maps.Values(generatorTableFunctions)), ", ")
 
-// readingFunctions reach a file, a model or the server binary while looking like ordinary
-// scalar functions. They name no table and no database, so neither of the rules above sees
-// them, and a wrapper that returns a number leaks what they read through the row count alone:
-// numbers(length(file(x))) yields one row per byte.
-//
-// Keyed by the lowercased name, since ClickHouse resolves function names case-insensitively.
-var readingFunctions = map[string]struct{}{
-	"file":                     {},
-	"catboostevaluate":         {},
-	"demangle":                 {},
-	"addresstoline":            {},
-	"addresstolinewithinlines": {},
-	"addresstosymbol":          {},
-}
-
-// A dictionary can be backed by HTTP, ODBC or another database, and every one of the 42
-// accessors carries this prefix.
-const dictionaryFunctionPrefix = "dict"
-
 // The parser's grammar has gaps against SQL that ClickHouse itself accepts.
 func ErrIfStatementIsNotValid(query string) (err error) {
 	defer func() {
@@ -89,23 +69,11 @@ func ErrIfStatementIsNotValid(query string) (err error) {
 
 	visitor := &chparser.DefaultASTVisitor{Visit: func(node chparser.Expr) error {
 		switch expr := node.(type) {
-		case *chparser.TableExpr:
-			// Source table functions remain usable in ClickHouse read-only mode, and only a
-			// table position can be one. The parser also types a call inside a table function's
-			// argument list as a TableFunctionExpr, so asking every one of those refuses the
-			// numbers(intDiv(...)) that every dashboard writes. What can read from an argument
-			// is caught by name below instead.
-			source := expr.Expr
-			if alias, ok := source.(*chparser.AliasExpr); ok {
-				source = alias.Expr
-			}
-
-			tableFunction, ok := source.(*chparser.TableFunctionExpr)
-			if !ok {
-				return nil
-			}
-
-			name := functionName(tableFunction.Name)
+		case *chparser.TableFunctionExpr:
+			// Source table functions remain usable in ClickHouse read-only mode. Arguments are
+			// visited before this, so a read smuggled into one is already refused by the time
+			// an allowed generator gets here.
+			name := chparser.Format(expr.Name)
 			if _, ok := generatorTableFunctions[strings.ToLower(name)]; ok {
 				return nil
 			}
@@ -114,25 +82,6 @@ func ErrIfStatementIsNotValid(query string) (err error) {
 				NewInvalidInputf(CodeClickHouseSQLTableFunction, "ClickHouse table functions are not allowed in SQL queries: %s", name).
 				WithAdditional(generatorTableFunctionsMessage)
 
-		case *chparser.FunctionExpr:
-			return errIfFunctionReads(expr.Name.Name)
-
-		case *chparser.TableFunctionExpr:
-			// Reached for a call in an argument list, and for a table position ahead of the
-			// TableExpr above, since a node is visited after its children.
-			return errIfFunctionReads(functionName(expr.Name))
-
-		case *chparser.Path:
-			// ClickHouse reads `x IN db.table` as a select from that table, and a qualified name
-			// on the right of IN is a Path rather than a TableIdentifier.
-			if len(expr.Fields) < 2 {
-				return nil
-			}
-
-			if _, ok := internalDatabases[strings.ToLower(expr.Fields[0].Name)]; ok {
-				return errors.NewInvalidInputf(CodeClickHouseSQLInternalDatabase, "the ClickHouse %s database is not allowed in SQL queries", expr.Fields[0].Name)
-			}
-
 		case *chparser.TableIdentifier:
 			// Reading these is unaffected by ClickHouse read-only mode.
 			if expr.Database == nil {
@@ -162,22 +111,3 @@ func LogIfStatementIsNotValid(ctx context.Context, logger *slog.Logger, query st
 		logger.WarnContext(ctx, "clickhouse sql is not valid", errors.Attr(err), slog.String("query", query))
 	}
 }
-
-func errIfFunctionReads(name string) error {
-	lowered := strings.ToLower(name)
-	if _, ok := readingFunctions[lowered]; !ok && !strings.HasPrefix(lowered, dictionaryFunctionPrefix) {
-		return nil
-	}
-
-	return errors.NewInvalidInputf(CodeClickHouseSQLReadingFunction, "ClickHouse functions that read outside the telemetry tables are not allowed in SQL queries: %s", name)
-}
-
-// The parser spells a call's name as an Ident everywhere it can. Reading the field rather than
-// formatting the node keeps the quoting out, so `numbers`(1) matches numbers.
-func functionName(expr chparser.Expr) string {
-	if ident, ok := expr.(*chparser.Ident); ok {
-		return ident.Name
-	}
-
-	return chparser.Format(expr)
-}

pkg/querybuilder/clickhouse_sql_test.go

@@ -7,7 +7,6 @@ import (
 	"github.com/SigNoz/signoz/pkg/errors"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestErrIfStatementIsNotValid_Pass(t *testing.T) {
@@ -15,12 +14,13 @@ func TestErrIfStatementIsNotValid_Pass(t *testing.T) {
 		name  string
 		query string
 	}{
+		// Shapes a telemetry read is allowed to take.
 		{"Select", "SELECT region AS r, zone FROM metrics WHERE metric_name = 'cpu' GROUP BY region, zone"},
 		{"TrailingSemicolon", "SELECT count() FROM signoz_logs.distributed_logs_v2;"},
 		{"CommonTableExpression", "WITH t AS (SELECT fingerprint FROM signoz_metrics.time_series_v4) SELECT * FROM t"},
 		{"Join", "SELECT * FROM t1 LEFT JOIN t2 ON t1.a = t2.b"},
 		{"GlobalIn", "SELECT a FROM t WHERE a GLOBAL IN (SELECT b FROM t2)"},
-		// https://github.com/AfterShip/clickhouse-sql-parser/pull/293
+		// GLOBAL parsed only when the join type was omitted, and only before IN. https://github.com/AfterShip/clickhouse-sql-parser/pull/293
 		{"GlobalLeftJoin", "SELECT * FROM t1 GLOBAL LEFT JOIN t2 ON t1.a = t2.a"},
 		{"GlobalNotIn", "SELECT a FROM t WHERE a GLOBAL NOT IN (SELECT b FROM t2)"},
 		{"Union", "SELECT * FROM t UNION ALL SELECT * FROM t2"},
@@ -29,34 +29,32 @@ func TestErrIfStatementIsNotValid_Pass(t *testing.T) {
 		{"UnrelatedSetting", "SELECT * FROM t SETTINGS max_threads = 4"},
 		{"TerminatedBlockComment", "SELECT /* keep me */ count() FROM t"},
 		{"BlockCommentMarkerInsideStringLiteral", "SELECT count() FROM t WHERE body = '/* not a comment'"},
-		// Looped forever before v0.5.2.
+		// The parser used to loop forever on this; it now reads the comment to the end of
+		// the input, so this doubles as a canary for that regression.
 		{"TrailingUnterminatedBlockComment", "SELECT count() FROM t /* unterminated"},
-		// Keyed on the database, not on the table name.
+		// The rule keys on the database, not on the table name.
 		{"TableNamedSystemInTelemetryDatabase", "SELECT * FROM signoz_logs.system"},
 		{"SignedLiteralAfterClosingParenSpaced", "SELECT (toUnixTimestamp(now()) - 3600)*1000000000"},
+		// order by interval
 		{"OrderByInterval", "SELECT toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS interval ORDER BY interval"},
 		{"OrderByIntervalAndDirection", "SELECT toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS `interval` ORDER BY `interval` ASC"},
-		// https://github.com/AfterShip/clickhouse-sql-parser/pull/296
+		// `interval` is a unit keyword, so unquoting it was rejected everywhere the parser
+		// expected a plain identifier. https://github.com/AfterShip/clickhouse-sql-parser/pull/296
 		{"OrderByUnquotedIntervalAsc", "SELECT toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS interval FROM t GROUP BY interval ORDER BY interval ASC"},
 		{"OrderByUnquotedIntervalDesc", "SELECT toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS interval FROM t GROUP BY interval ORDER BY interval DESC"},
 		{"UnquotedIntervalInGroupByTuple", "SELECT a FROM t GROUP BY (`service.name`, `service.version`, interval)"},
 		{"UnquotedIntervalProductionQuery", "SELECT toStartOfInterval(timestamp, INTERVAL 1 MINUTE) AS interval, resource_string_service$$name AS `service.name`, attributes_string['http.route'] AS `http.route`, quantile(0.95)(duration_nano) / 1000000000 AS value FROM signoz_traces.distributed_signoz_index_v3 WHERE resource_string_service$$name = 'svc-a' AND resources_string['deployment.environment'] = 'dev' AND attributes_string['http.route'] = '/v1' AND http_method = 'POST' AND timestamp BETWEEN toDateTime(1784601720) AND toDateTime(1784602620) AND ts_bucket_start BETWEEN 1784601720 - 1800 AND 1784602620 GROUP BY `service.name`, `http.route`, interval ORDER BY interval ASC"},
-		// The fix backtracks, so this bounds the cost. https://github.com/AfterShip/clickhouse-sql-parser/pull/296#issuecomment-5150316367
+		// Separating the two readings of INTERVAL needs backtracking as per the current implementation which could have performance regressions.
+		// https://github.com/AfterShip/clickhouse-sql-parser/pull/296#issuecomment-5150316367
 		{"UnquotedIntervalRepeatedThirtyTimes", "SELECT interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval + interval AS total FROM t WHERE interval > 0 ORDER BY interval ASC"},
-		// `interval` was one of 37 such keywords. https://github.com/AfterShip/clickhouse-sql-parser/pull/305
-		{"UnquotedLimitInFunctionArgument", "SELECT sum(limit) FROM t"},
-		{"UnquotedLimitInArithmetic", "SELECT limit + 1 FROM t"},
-		{"UnquotedLimitInNegation", "SELECT abs(-limit) FROM t"},
-		{"UnquotedKeywordOperands", "SELECT sum(offset) + sum(format) + sum(settings) FROM t"},
-		{"UnquotedLimitProductionQuery", "WITH limit_value AS (SELECT cluster, region, value AS limit FROM t) SELECT region AS `Region`, sum(limit) AS `Capacity` FROM limit_value GROUP BY Region"},
 		{"SignedLiteralAfterClosingParenUnspaced", "SELECT now() AS ts, toFloat64(count()) AS value FROM ( SELECT attributes_string['TableName'] AS T, attributes_string['MissingId'] AS M, max(fromUnixTimestamp64Nano(timestamp)) AS last_seen, dateDiff('minute', min(fromUnixTimestamp64Nano(timestamp)), max(fromUnixTimestamp64Nano(timestamp))) AS age_min FROM signoz_logs.distributed_logs_v2 WHERE body='missing_map_record' AND timestamp >= (toUnixTimestamp(now())-3600)*1000000000 GROUP BY T, M ) WHERE age_min >= 20 AND last_seen >= now() - toIntervalMinute(8)"},
 		{"SignedLiteralAfterClosingParenMinimal", "SELECT (1)-1"},
 		{"TrimFunction", "SELECT trimBoth('/api/endpoint/', '/');"},
-		// https://github.com/AfterShip/clickhouse-sql-parser/pull/290
+		// The SQL-standard keyword-separated argument forms, which took commas only. https://github.com/AfterShip/clickhouse-sql-parser/pull/290
 		{"StandardTrimSyntax", "SELECT trim(BOTH ' ' FROM body) FROM t"},
 		{"StandardSubstringSyntax", "SELECT substring(body FROM 2 FOR 3) FROM t"},
 		{"StandardOverlaySyntax", "SELECT overlay(body PLACING 'x' FROM 2) FROM t"},
-		// The shape row generators get used for: a dense interval axis to CROSS JOIN a sparse series against.
+		// Row generators compute their rows from their arguments, so they read through nothing. This is the shape they get used for: a dense interval axis to CROSS JOIN a sparse series against.
 		{"NumbersTableFunction", "SELECT intervals.interval AS interval, active.cluster AS cluster, toFloat64(if(ts_data.has_data = 0, 0, 1)) AS value FROM ( SELECT DISTINCT JSONExtractString(labels, 'k8s.cluster.name') AS cluster FROM signoz_metrics.distributed_time_series_v4 WHERE metric_name = 'my_metric' AND unix_milli >= toUnixTimestamp(now() - INTERVAL 30 DAY) * 1000 HAVING cluster != '' ) AS active CROSS JOIN ( SELECT toStartOfInterval( toDateTime(toUnixTimestamp(now() - INTERVAL 30 MINUTE) + number * 60), INTERVAL 1 MINUTE ) AS interval FROM numbers(31) ) AS intervals LEFT JOIN ( SELECT toStartOfInterval( toDateTime(intDiv(s.unix_milli, 1000)), INTERVAL 1 MINUTE ) AS interval, JSONExtractString(ts.labels, 'k8s.cluster.name') AS cluster, 1 AS has_data FROM signoz_metrics.distributed_samples_v4 s INNER JOIN ( SELECT DISTINCT fingerprint, labels FROM signoz_metrics.distributed_time_series_v4 WHERE metric_name = 'my_metric' ) AS ts ON s.fingerprint = ts.fingerprint WHERE s.metric_name = 'my_metric' AND s.unix_milli >= toUnixTimestamp(now() - INTERVAL 30 MINUTE) * 1000 GROUP BY interval, cluster ) AS ts_data ON active.cluster = ts_data.cluster AND intervals.interval = ts_data.interval ORDER BY interval ASC"},
 		{"NumbersMtTableFunction", "SELECT * FROM numbers_mt(31)"},
 		{"ZerosTableFunction", "SELECT * FROM zeros(31)"},
@@ -65,16 +63,6 @@ func TestErrIfStatementIsNotValid_Pass(t *testing.T) {
 		{"GenerateSeriesSnakeCaseTableFunction", "SELECT * FROM generate_series(1, 10)"},
 		{"GeneratorTableFunctionUppercase", "SELECT * FROM NUMBERS(31)"},
 		{"GeneratorTableFunctionParenthesisedArgument", "SELECT * FROM NUMBERS((31))"},
-		// CAST in an argument was itself read as a table function. https://github.com/AfterShip/clickhouse-sql-parser/pull/307
-		{"CastInGeneratorTableFunctionArgument", "SELECT * FROM numbers(CAST(10 AS UInt64))"},
-		{"ScalarCallInGeneratorTableFunctionArgument", "SELECT * FROM numbers(intDiv(100, 2))"},
-		{"NestedScalarCallInGeneratorTableFunctionArgument", "SELECT * FROM numbers(greatest(1, intDiv(100, 2) + 1))"},
-		{"GeneratorTableFunctionProductionQuery", "WITH toInt64(1786029960000000000) AS start_ns, toInt64(1786031760000000000) AS end_ns, 300000000000 AS step_ns SELECT ts, toFloat64(sum(value)) AS value FROM (SELECT fromUnixTimestamp64Nano(start_ns + toInt64(number) * step_ns) AS ts, 0 AS value FROM numbers(greatest(1, intDiv(end_ns - start_ns, step_ns) + 1)) UNION ALL SELECT toStartOfInterval(fromUnixTimestamp64Nano(timestamp), INTERVAL 5 minute) AS ts, count() AS value FROM signoz_logs.distributed_logs_v2 WHERE timestamp >= 1786029960000000000 AND timestamp <= 1786031760000000000 GROUP BY ts) GROUP BY ts ORDER BY ts"},
-		// The allow list keys on the bare name, so quoting must not hide a generator from it.
-		{"BacktickQuotedGeneratorTableFunction", "SELECT * FROM `numbers`(31)"},
-		{"DoubleQuotedGeneratorTableFunction", "SELECT * FROM \"numbers\"(31)"},
-		// Reads nothing: format builds a string, and shares its name with a table function.
-		{"ScalarFunctionNamedAfterATableFunction", "SELECT format('{} {}', a, b) FROM t"},
 		{"GeneratorTableFunctionInJoin", "SELECT * FROM signoz_logs.distributed_logs_v2 AS l CROSS JOIN numbers(31) AS n"},
 		{"GeneratorTableFunctionInCommonTableExpression", "WITH axis AS (SELECT number FROM numbers(31)) SELECT * FROM axis"},
 		{"GeneratorTableFunctionInWhereSubquery", "SELECT * FROM t WHERE a IN (SELECT number FROM numbers(31))"},
@@ -83,7 +71,8 @@ func TestErrIfStatementIsNotValid_Pass(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			// Bounded because a parser that backtracks without memoising hangs rather than returning.
+			// Bounded rather than called directly: a parser that backtracks without memoising
+			// hangs instead of returning. Every case here parses in well under a millisecond.
 			errC := make(chan error, 1)
 			go func() { errC <- ErrIfStatementIsNotValid(testCase.query) }()
 
@@ -103,57 +92,46 @@ func TestErrIfStatementIsNotValid_Fail(t *testing.T) {
 		query        string
 		expectedCode errors.Code
 	}{
+		// Not a single statement, or not a statement at all.
 		{"Empty", "", CodeClickHouseSQLNotSingleStatement},
 		{"UnterminatedBlockCommentOnly", "/* x", CodeClickHouseSQLUnparseable},
 		{"Unparseable", "SELECT FROM WHERE", CodeClickHouseSQLUnparseable},
 		{"MultipleStatements", "SELECT 1; DROP TABLE signoz_logs.logs_v2", CodeClickHouseSQLNotSingleStatement},
+		// Parses, but is not a SELECT.
 		{"Drop", "DROP TABLE signoz_logs.logs_v2", CodeClickHouseSQLNotSelect},
 		{"Insert", "INSERT INTO signoz_logs.logs_v2 SELECT * FROM signoz_logs.logs_v2", CodeClickHouseSQLNotSelect},
 		{"AlterDelete", "ALTER TABLE signoz_logs.logs_v2 DELETE WHERE 1 = 1", CodeClickHouseSQLNotSelect},
 		{"CreateTable", "CREATE TABLE evil (a Int) ENGINE = Memory", CodeClickHouseSQLNotSelect},
 		{"Grant", "GRANT ALL ON *.* TO admin", CodeClickHouseSQLNotSelect},
 		{"Set", "SET readonly = 0", CodeClickHouseSQLNotSelect},
-		// Both panicked before v0.5.5. https://github.com/AfterShip/clickhouse-sql-parser/pull/306
-		{"UnparseableDefaultExpression", "CREATE TABLE t (a String DEFAULT foo(b FROM 2)) ENGINE = Memory", CodeClickHouseSQLUnparseable},
-		{"TrailingOperatorInDefaultExpression", "CREATE TABLE t (a String DEFAULT 1 +) ENGINE = Memory", CodeClickHouseSQLUnparseable},
-		// Rejected outright rather than classified.
+		// The parser still dereferences nil on a DEFAULT expression it cannot read, so the recover is what turns this into a rejection rather than a crash.
+		{"UnparseableDefaultExpression", "CREATE TABLE t (a String DEFAULT foo(b FROM 2)) ENGINE = Memory", CodeClickHouseSQLParserPanic},
+		// These the parser rejects outright rather than classifying.
 		{"ShowGrants", "SHOW GRANTS", CodeClickHouseSQLUnparseable},
 		{"IntoOutfile", "SELECT * FROM t INTO OUTFILE '/tmp/x.csv'", CodeClickHouseSQLUnparseable},
+		// Table functions, which read through something other than a telemetry table.
 		{"UrlTableFunction", "SELECT * FROM url('http://attacker.example/x', CSV, 'a String')", CodeClickHouseSQLTableFunction},
-		// file is also a scalar function, so the reading rule reaches it before the table rule does.
-		{"FileTableFunction", "SELECT * FROM file('/etc/passwd', CSV, 'a String')", CodeClickHouseSQLReadingFunction},
+		{"FileTableFunction", "SELECT * FROM file('/etc/passwd', CSV, 'a String')", CodeClickHouseSQLTableFunction},
 		{"ExecutableTableFunction", "SELECT * FROM executable('script.sh', CSV, 'a String')", CodeClickHouseSQLTableFunction},
 		{"TableFunctionInJoin", "SELECT * FROM t1 JOIN url('http://x', CSV, 'a String') u ON 1 = 1", CodeClickHouseSQLTableFunction},
 		{"TableFunctionInCommonTableExpression", "WITH c AS (SELECT * FROM url('http://x', CSV, 'a String')) SELECT * FROM c", CodeClickHouseSQLTableFunction},
-		{"TableFunctionInWhereSubquery", "SELECT * FROM t WHERE a IN (SELECT * FROM url('http://x', CSV, 'a String'))", CodeClickHouseSQLTableFunction},
+		{"TableFunctionInWhereSubquery", "SELECT * FROM t WHERE a IN (SELECT * FROM file('/etc/passwd', CSV, 'a String'))", CodeClickHouseSQLTableFunction},
 		{"TableFunctionInUnion", "SELECT * FROM t UNION ALL SELECT * FROM url('http://x', CSV, 'a String')", CodeClickHouseSQLTableFunction},
-		// Reach an internal database without naming one, so only the table-function rule sees them.
+		// These reach the internal databases without ever naming one, so the table-function rule is the only thing that sees them.
 		{"MergeTableFunction", "SELECT * FROM merge('system', '.*')", CodeClickHouseSQLTableFunction},
 		{"RemoteTableFunction", "SELECT * FROM remote('other-host', 'system.users')", CodeClickHouseSQLTableFunction},
 		{"ClusterTableFunction", "SELECT * FROM cluster('c', 'system.users')", CodeClickHouseSQLTableFunction},
-		// Pure, but excluded: generateRandom is unbounded, and values adds nothing over an array literal.
+		// Pure, but excluded: generateRandom streams rows the arguments do not bound, and values has no use here that an array literal does not already cover.
 		{"GenerateRandomTableFunction", "SELECT * FROM generateRandom('a UInt64')", CodeClickHouseSQLTableFunction},
 		{"ValuesTableFunction", "SELECT * FROM values('a UInt64', 1, 2)", CodeClickHouseSQLTableFunction},
-		// Arguments are visited first, so an allowed generator is not a wrapper to smuggle a read through.
+		// Arguments are visited before the table function itself, so allowing a generator does not give anyone a wrapper to smuggle a read through.
 		{"InternalDatabaseInsideAllowedTableFunction", "SELECT * FROM numbers((SELECT count() FROM system.users))", CodeClickHouseSQLInternalDatabase},
 		{"InternalDatabaseJoinedOntoAllowedTableFunction", "SELECT * FROM numbers(31) AS n JOIN system.users AS u ON 1 = 1", CodeClickHouseSQLInternalDatabase},
 		{"InternalDatabaseUnionedWithAllowedTableFunction", "SELECT number FROM numbers(31) UNION ALL SELECT name FROM system.users", CodeClickHouseSQLInternalDatabase},
 		{"RefusedTableFunctionJoinedOntoAllowedTableFunction", "SELECT * FROM numbers(31) AS n JOIN url('http://x', CSV, 'a String') AS u ON 1 = 1", CodeClickHouseSQLTableFunction},
-		{"RefusedTableFunctionInsideAllowedTableFunction", "SELECT * FROM numbers((SELECT count() FROM url('http://x', CSV, 'a String')))", CodeClickHouseSQLTableFunction},
+		{"RefusedTableFunctionInsideAllowedTableFunction", "SELECT * FROM numbers((SELECT count() FROM file('/etc/passwd', CSV, 'a String')))", CodeClickHouseSQLTableFunction},
 		{"InternalDatabaseInsideAllowedTableFunctionCommonTableExpression", "WITH axis AS (SELECT * FROM numbers((SELECT count() FROM system.users))) SELECT * FROM axis", CodeClickHouseSQLInternalDatabase},
-		// Read a file, a dictionary or the server binary without naming a table, so neither the table rule nor the database rule sees them. The row count alone is an oracle: numbers(length(file(x))) returns one row per byte.
-		{"ScalarFileFunction", "SELECT file('/etc/passwd')", CodeClickHouseSQLReadingFunction},
-		{"ScalarFileFunctionInWhere", "SELECT * FROM t WHERE length(file('/etc/passwd')) > 0", CodeClickHouseSQLReadingFunction},
-		{"ScalarFileFunctionInGeneratorTableFunctionArgument", "SELECT * FROM numbers(length(file('/etc/passwd')))", CodeClickHouseSQLReadingFunction},
-		{"DictionaryFunction", "SELECT dictGetUInt64('d', 'k', toUInt64(1))", CodeClickHouseSQLReadingFunction},
-		{"DictionaryFunctionUppercase", "SELECT DICTGETSTRING('d', 'k', toUInt64(1))", CodeClickHouseSQLReadingFunction},
-		{"DictionaryFunctionInGeneratorTableFunctionArgument", "SELECT * FROM numbers(dictGetUInt64('d', 'k', toUInt64(1)))", CodeClickHouseSQLReadingFunction},
-		{"IntrospectionFunction", "SELECT demangle(addressToSymbol(toUInt64(1)))", CodeClickHouseSQLReadingFunction},
-		{"ModelEvaluationFunction", "SELECT catboostEvaluate('/model.bin', 1)", CodeClickHouseSQLReadingFunction},
-		// ClickHouse reads `x IN table` as `x IN (SELECT * FROM table)`, and a qualified name there is a Path rather than a TableIdentifier.
-		{"InternalDatabaseInInOperator", "SELECT * FROM t WHERE a IN system.users", CodeClickHouseSQLInternalDatabase},
-		{"InternalDatabaseInGlobalInOperator", "SELECT * FROM t WHERE a GLOBAL IN system.users", CodeClickHouseSQLInternalDatabase},
-		{"InternalDatabaseInNotInOperator", "SELECT * FROM t WHERE a NOT IN system.users", CodeClickHouseSQLInternalDatabase},
+		// Internal databases, which hold grants and server metadata rather than telemetry.
 		{"SystemUsers", "SELECT * FROM system.users", CodeClickHouseSQLInternalDatabase},
 		{"SystemUppercase", "SELECT * FROM SYSTEM.USERS", CodeClickHouseSQLInternalDatabase},
 		{"SystemQuoted", "SELECT count() FROM `system`.`tables`", CodeClickHouseSQLInternalDatabase},
@@ -161,7 +139,7 @@ func TestErrIfStatementIsNotValid_Fail(t *testing.T) {
 		{"SystemInJoin", "SELECT * FROM signoz_logs.distributed_logs_v2 AS l JOIN system.users AS u ON 1 = 1", CodeClickHouseSQLInternalDatabase},
 		{"SystemInIntersect", "SELECT * FROM t INTERSECT SELECT * FROM system.users", CodeClickHouseSQLInternalDatabase},
 		{"InformationSchema", "SELECT * FROM information_schema.tables", CodeClickHouseSQLInternalDatabase},
-		// Takes precedence over the setting the caller applies.
+		// A query-level setting takes precedence over the one the caller applies.
 		{"ReadonlySettingOverride", "SELECT * FROM t SETTINGS readonly = 0", CodeClickHouseSQLReadonlyOverride},
 		{"ReadonlySettingOverrideAmongOthers", "SELECT * FROM t SETTINGS max_threads = 4, readonly = 0", CodeClickHouseSQLReadonlyOverride},
 	}
@@ -170,33 +148,7 @@ func TestErrIfStatementIsNotValid_Fail(t *testing.T) {
 		t.Run(testCase.name, func(t *testing.T) {
 			err := ErrIfStatementIsNotValid(testCase.query)
 
-			// Required rather than asserted: errors.Asc dereferences the error it is given.
-			require.Error(t, err)
-			assert.True(t, errors.Asc(err, testCase.expectedCode), "expected code %s, got %v", testCase.expectedCode, err)
-		})
-	}
-}
-
-func TestErrIfStatementIsNotValid_ShouldPassButFails(t *testing.T) {
-	testCases := []struct {
-		name         string
-		query        string
-		expectedCode errors.Code
-	}{
-		// The left operand commits the parser to a subquery, leaving the operator nowhere to bind. Parenthesising only the right operand is fine.
-		{"ParenthesisedUnionLeftOperand", "SELECT a FROM ((SELECT 1 AS a) UNION ALL (SELECT 2 AS a))", CodeClickHouseSQLUnparseable},
-		{"ParenthesisedExceptLeftOperand", "SELECT a FROM ((SELECT 1 AS a) EXCEPT (SELECT 2 AS a))", CodeClickHouseSQLUnparseable},
-		{"ParenthesisedUnionLeftOperandAtStatementLevel", "(SELECT 1 AS a) UNION ALL (SELECT 2 AS a)", CodeClickHouseSQLUnparseable},
-		// The one keyword PR 305 left behind, because ON also opens a join condition.
-		{"UnquotedOnAsColumnName", "SELECT on + 1 FROM t", CodeClickHouseSQLUnparseable},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(testCase.name, func(t *testing.T) {
-			err := ErrIfStatementIsNotValid(testCase.query)
-
-			// Required rather than asserted: errors.Asc dereferences the error it is given.
-			require.Error(t, err)
+			assert.Error(t, err)
 			assert.True(t, errors.Asc(err, testCase.expectedCode), "expected code %s, got %v", testCase.expectedCode, err)
 		})
 	}

pkg/statementbuilder/aistatementbuilder/span_list_test.go

@@ -0,0 +1,155 @@
+package aistatementbuilder
+
+import (
+	"context"
+	"testing"
+
+	qbtypes "github.com/SigNoz/signoz/pkg/types/querybuildertypes/querybuildertypesv5"
+	"github.com/SigNoz/signoz/pkg/types/telemetrytypes"
+	"github.com/SigNoz/signoz/pkg/valuer"
+	"github.com/stretchr/testify/require"
+)
+
+// Span list with a mixed filter: gen_ai spans matching the span-level part, in
+// traces whose window-clipped aggregates satisfy the trace-level part (the
+// __trace_scope qualification on the delegated path).
+func TestBuild_FullSQL_SpanList_TraceScoped(t *testing.T) {
+	b := newTestBuilder(t)
+	stmt, err := b.Build(context.Background(), valuer.UUID{}, testStartMs, testEndMs, qbtypes.RequestTypeRaw,
+		qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+			Signal: telemetrytypes.SignalTraces,
+			Filter: &qbtypes.Filter{Expression: "gen_ai.request.model = 'gpt-4o-mini' AND trace.output_tokens > 1000"},
+			Limit:  10,
+		}, nil)
+	require.NoError(t, err)
+
+	assertSQLEqual(t, `
+WITH __trace_scope AS (
+    SELECT trace_id,
+        sum(multiIf(mapContains(attributes_number, 'gen_ai.usage.output_tokens'), toFloat64(attributes_number['gen_ai.usage.output_tokens']), NULL)) AS output_tokens
+    FROM signoz_traces.distributed_signoz_index_v3
+    WHERE timestamp >= '1747947419000000000'
+      AND timestamp < '1747983448000000000'
+      AND ts_bucket_start >= 1747945619
+      AND ts_bucket_start <= 1747983448
+      AND ((mapContains(attributes_string, 'gen_ai.request.model') OR mapContains(attributes_string, 'gen_ai.tool.name') OR mapContains(attributes_string, 'gen_ai.agent.name')))
+    GROUP BY trace_id
+    HAVING output_tokens > 1000
+)
+SELECT timestamp AS __SELECT_KEY_0_timestamp, trace_id AS __SELECT_KEY_1_trace_id, span_id AS __SELECT_KEY_2_span_id,
+    trace_state AS __SELECT_KEY_3_trace_state, parent_span_id AS __SELECT_KEY_4_parent_span_id, flags AS __SELECT_KEY_5_flags,
+    name AS __SELECT_KEY_6_name, kind AS __SELECT_KEY_7_kind, kind_string AS __SELECT_KEY_8_kind_string, duration_nano AS __SELECT_KEY_9_duration_nano,
+    status_code AS __SELECT_KEY_10_status_code, status_message AS __SELECT_KEY_11_status_message,
+    status_code_string AS __SELECT_KEY_12_status_code_string, events AS __SELECT_KEY_13_events, links AS __SELECT_KEY_14_links,
+    response_status_code AS __SELECT_KEY_15_response_status_code, external_http_url AS __SELECT_KEY_16_external_http_url,
+    http_url AS __SELECT_KEY_17_http_url, external_http_method AS __SELECT_KEY_18_external_http_method,
+    http_method AS __SELECT_KEY_19_http_method, http_host AS __SELECT_KEY_20_http_host, db_name AS __SELECT_KEY_21_db_name,
+    db_operation AS __SELECT_KEY_22_db_operation, has_error AS __SELECT_KEY_23_has_error, is_remote AS __SELECT_KEY_24_is_remote,
+    attributes_string, attributes_number, attributes_bool, resources_string
+FROM signoz_traces.distributed_signoz_index_v3
+WHERE trace_id GLOBAL IN (SELECT trace_id FROM __trace_scope)
+  AND (((mapContains(attributes_string, 'gen_ai.request.model')
+        OR mapContains(attributes_string, 'gen_ai.tool.name')
+        OR mapContains(attributes_string, 'gen_ai.agent.name')))
+    AND ((attributes_string['gen_ai.request.model'] = 'gpt-4o-mini'
+        AND mapContains(attributes_string, 'gen_ai.request.model'))))
+  AND timestamp >= '1747947419000000000'
+  AND timestamp < '1747983448000000000'
+  AND ts_bucket_start >= 1747945619
+  AND ts_bucket_start <= 1747983448
+LIMIT 10
+`, stmt)
+}
+
+// A raw filter mixing a resource attribute with a trace-level condition: the resource
+// part flows through the delegate's fingerprint machinery (__resource_filter CTE),
+// the trace-level part becomes the __trace_scope qualification.
+func TestBuild_SpanList_ResourcePlusTraceFilter(t *testing.T) {
+	b := newTestBuilder(t)
+	stmt, err := b.Build(context.Background(), valuer.UUID{}, testStartMs, testEndMs, qbtypes.RequestTypeRaw,
+		qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+			Signal: telemetrytypes.SignalTraces,
+			Filter: &qbtypes.Filter{Expression: "resource.service.name = 'checkout' AND trace.output_tokens > 1000"},
+			Limit:  10,
+		}, nil)
+	require.NoError(t, err)
+
+	got := renderSQL(t, stmt)
+	require.Contains(t, got, "__resource_filter AS (")
+	require.Contains(t, got, "resource_fingerprint GLOBAL IN (SELECT fingerprint FROM __resource_filter)")
+	require.Contains(t, got, "__trace_scope AS (")
+	require.Contains(t, got, "trace_id GLOBAL IN (SELECT trace_id FROM __trace_scope)")
+	require.Contains(t, got, "HAVING output_tokens > 1000")
+}
+
+// Without a trace-level condition nothing changes: the span list stays a single
+// gated span scan (no __trace_scope CTE).
+func TestBuild_SpanList_NoTraceFilter_NoScope(t *testing.T) {
+	b := newTestBuilder(t)
+	stmt, err := b.Build(context.Background(), valuer.UUID{}, testStartMs, testEndMs, qbtypes.RequestTypeRaw,
+		qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+			Signal: telemetrytypes.SignalTraces,
+			Filter: &qbtypes.Filter{Expression: "gen_ai.request.model = 'gpt-4o-mini'"},
+			Limit:  10,
+		}, nil)
+	require.NoError(t, err)
+	require.NotContains(t, stmt.Query, "__trace_scope")
+}
+
+// The span-list trace-level filter shares the trace list's rules: output-only
+// aggregates are rejected, OR-mixing the two classes is rejected, and explicitly
+// trace-level order keys get a targeted error — while bare span columns that happen
+// to share a name with an aggregate alias (duration_nano) stay orderable.
+func TestBuild_SpanList_TraceFilter_Validation(t *testing.T) {
+	b := newTestBuilder(t)
+	build := func(q qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]) error {
+		q.Signal = telemetrytypes.SignalTraces
+		_, err := b.Build(context.Background(), valuer.UUID{}, testStartMs, testEndMs, qbtypes.RequestTypeRaw, q, nil)
+		return err
+	}
+
+	err := build(qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+		Filter: &qbtypes.Filter{Expression: "trace.span_count > 3"},
+	})
+	require.ErrorContains(t, err, `aggregate "span_count" cannot be used`)
+
+	err = build(qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+		Filter: &qbtypes.Filter{Expression: "trace.output_tokens > 1000 OR kind_string = 'Client'"},
+	})
+	require.ErrorContains(t, err, "cannot be combined")
+
+	err = build(qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+		Order: []qbtypes.OrderBy{{Key: qbtypes.OrderByKey{TelemetryFieldKey: telemetrytypes.TelemetryFieldKey{Name: "trace.output_tokens"}}}},
+	})
+	require.ErrorContains(t, err, `ordering the span list by trace-level aggregate "trace.output_tokens" is not supported`)
+
+	err = build(qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+		Order: []qbtypes.OrderBy{{Key: qbtypes.OrderByKey{TelemetryFieldKey: telemetrytypes.TelemetryFieldKey{Name: "duration_nano"}}, Direction: qbtypes.OrderDirectionDesc}},
+		Limit: 10,
+	})
+	require.NoError(t, err, "bare duration_nano is a span column, not a trace-level key")
+}
+
+// Variables in a trace-level condition on the span list get the trace list's
+// treatment: substituted as literals, __all__ drops the condition (no scope CTE).
+func TestBuild_SpanList_TraceFilter_Variables(t *testing.T) {
+	b := newTestBuilder(t)
+	build := func(expr string, vars map[string]qbtypes.VariableItem) (*qbtypes.Statement, error) {
+		return b.Build(context.Background(), valuer.UUID{}, testStartMs, testEndMs, qbtypes.RequestTypeRaw,
+			qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]{
+				Signal: telemetrytypes.SignalTraces,
+				Filter: &qbtypes.Filter{Expression: expr},
+				Limit:  10,
+			}, vars)
+	}
+
+	stmt, err := build("trace.output_tokens > $threshold",
+		map[string]qbtypes.VariableItem{"threshold": {Value: 700}})
+	require.NoError(t, err)
+	require.Contains(t, stmt.Query, "HAVING output_tokens > 700")
+
+	stmt, err = build("trace.output_tokens > $threshold",
+		map[string]qbtypes.VariableItem{"threshold": {Type: qbtypes.DynamicVariableType, Value: "__all__"}})
+	require.NoError(t, err)
+	require.NotContains(t, stmt.Query, "__trace_scope")
+}

pkg/statementbuilder/scopedtracesstatementbuilder/statement_builder.go

@@ -31,15 +31,24 @@ var (
 // (e.g. gen_ai spans); the TraceScope decides which spans are in scope and which
 // per-trace columns to compute.
 type scopedTraceStatementBuilder struct {
-	logger                    *slog.Logger
-	metadataStore             telemetrytypes.MetadataStore
-	fm                        qbtypes.FieldMapper
-	cb                        qbtypes.ConditionBuilder
-	scope                     TraceScope
-	traceStmtBuilder          qbtypes.StatementBuilder[qbtypes.TraceAggregation]
+	logger           *slog.Logger
+	metadataStore    telemetrytypes.MetadataStore
+	fm               qbtypes.FieldMapper
+	cb               qbtypes.ConditionBuilder
+	scope            TraceScope
+	traceStmtBuilder qbtypes.StatementBuilder[qbtypes.TraceAggregation]
+	// scopedDelegate is traceStmtBuilder's trace-scoping capability, resolved once at
+	// construction; nil when the delegate cannot constrain a query by trace ids.
+	scopedDelegate            traceScopedStatementBuilder
 	resourceFilterStmtBuilder qbtypes.StatementBuilder[qbtypes.TraceAggregation]
 }
 
+// traceScopedStatementBuilder is the delegate's optional capability of constraining a
+// query to a set of trace ids (implemented by the traces statement builder).
+type traceScopedStatementBuilder interface {
+	BuildTraceScoped(ctx context.Context, orgID valuer.UUID, start, end uint64, requestType qbtypes.RequestType, query qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation], variables map[string]qbtypes.VariableItem, traceScope *qbtypes.Statement) (*qbtypes.Statement, error)
+}
+
 var _ qbtypes.StatementBuilder[qbtypes.TraceAggregation] = (*scopedTraceStatementBuilder)(nil)
 
 // NewFactory returns a provider factory for a scoped trace statement builder. The
@@ -90,6 +99,7 @@ func NewScopedTraceStatementBuilder(
 		fl,
 	)
 
+	scopedDelegate, _ := traceStmtBuilder.(traceScopedStatementBuilder)
 	return &scopedTraceStatementBuilder{
 		logger:                    scopedSettings.Logger(),
 		metadataStore:             metadataStore,
@@ -97,6 +107,7 @@ func NewScopedTraceStatementBuilder(
 		cb:                        conditionBuilder,
 		scope:                     scope,
 		traceStmtBuilder:          traceStmtBuilder,
+		scopedDelegate:            scopedDelegate,
 		resourceFilterStmtBuilder: resourceFilterStmtBuilder,
 	}
 }
@@ -114,14 +125,18 @@ func (b *scopedTraceStatementBuilder) Build(
 	case qbtypes.RequestTypeTrace:
 		return b.buildTraceListQuery(ctx, orgID, querybuilder.ToNanoSecs(start), querybuilder.ToNanoSecs(end), query, variables)
 	case qbtypes.RequestTypeRaw:
+		if err := b.validateRawOrderKeys(query); err != nil {
+			return nil, err
+		}
 		return b.buildDelegated(ctx, orgID, start, end, requestType, query, variables)
 	default:
 		return nil, ErrUnsupportedRequestType
 	}
 }
 
-// buildDelegated ANDs the base gate into the user filter and delegates to the
-// standard trace builder (the span-list / raw path).
+// buildDelegated serves the span-list / raw path: the gate is ANDed into the filter's
+// span-level part and the query delegates to the standard trace builder; a trace-level
+// part becomes a qualification the delegate constrains trace_id by.
 func (b *scopedTraceStatementBuilder) buildDelegated(
 	ctx context.Context,
 	orgID valuer.UUID,
@@ -130,17 +145,127 @@ func (b *scopedTraceStatementBuilder) buildDelegated(
 	query qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation],
 	variables map[string]qbtypes.VariableItem,
 ) (*qbtypes.Statement, error) {
+	var spanExpr, traceExpr string
+	if query.Filter != nil && strings.TrimSpace(query.Filter.Expression) != "" {
+		var err error
+		spanExpr, traceExpr, err = querybuilder.SplitFilterForAggregates(query.Filter.Expression, b.aggregateAliasSet())
+		if err != nil {
+			return nil, err
+		}
+	}
+	traceExpr, err := substituteTraceLevelVariables(traceExpr, variables)
+	if err != nil {
+		return nil, err
+	}
+
 	gate := b.scope.FilterExpression
 	expr := gate
-	if query.Filter != nil && strings.TrimSpace(query.Filter.Expression) != "" {
-		expr = fmt.Sprintf("(%s) AND (%s)", gate, query.Filter.Expression)
+	if strings.TrimSpace(spanExpr) != "" {
+		expr = fmt.Sprintf("(%s) AND (%s)", gate, spanExpr)
 	}
 
 	// shallow copy; only Filter is replaced, caller's query untouched
 	gated := query
 	gated.Filter = &qbtypes.Filter{Expression: expr}
 
-	return b.traceStmtBuilder.Build(ctx, orgID, start, end, requestType, gated, variables)
+	if strings.TrimSpace(traceExpr) == "" {
+		return b.traceStmtBuilder.Build(ctx, orgID, start, end, requestType, gated, variables)
+	}
+
+	if b.scopedDelegate == nil {
+		return nil, errors.NewInternalf(errors.CodeInternal, "trace statement builder does not support trace-scoped queries")
+	}
+	scope, err := b.buildTraceScopeStatement(ctx, orgID, querybuilder.ToNanoSecs(start), querybuilder.ToNanoSecs(end), traceExpr)
+	if err != nil {
+		return nil, err
+	}
+	return b.scopedDelegate.BuildTraceScoped(ctx, orgID, start, end, requestType, gated, variables, scope)
+}
+
+// validateRawOrderKeys rejects ordering the span list by an explicitly trace-level
+// aggregate (trace. prefix or trace field context) — the per-trace value does not
+// exist on span rows. Bare names pass through: they may legitimately be span columns
+// (duration_nano, timestamp).
+func (b *scopedTraceStatementBuilder) validateRawOrderKeys(query qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation]) error {
+	aliases := b.aggregateAliasSet()
+	for _, o := range query.Order {
+		key := telemetrytypes.GetFieldKeyFromKeyText(o.Key.Name)
+		if _, ok := aliases[key.Name]; !ok {
+			continue
+		}
+		if key.FieldContext == telemetrytypes.FieldContextTrace || o.Key.FieldContext == telemetrytypes.FieldContextTrace {
+			return errors.NewInvalidInputf(errors.CodeInvalidInput,
+				"ordering the span list by trace-level aggregate %q is not supported; order by span columns instead (e.g. timestamp, duration_nano)", o.Key.Name)
+		}
+	}
+	return nil
+}
+
+// substituteTraceLevelVariables resolves query variables in a trace-level expression.
+// The span-level parts bind variables via PrepareWhereClause; trace-level parts are
+// text rewrites, so variables become literals (a dynamic __all__ drops its condition).
+func substituteTraceLevelVariables(expr string, variables map[string]qbtypes.VariableItem) (string, error) {
+	if strings.TrimSpace(expr) == "" || len(variables) == 0 {
+		return expr, nil
+	}
+	return qbvariables.ReplaceVariablesInExpression(expr, variables)
+}
+
+// buildTraceScopeStatement builds the __trace_scope statement: trace ids whose
+// window-clipped per-trace aggregates satisfy traceExpr. The same scan as the matched
+// CTE, minus its span-filter widening, resource prune, ordering and pagination.
+// start/end are ns.
+func (b *scopedTraceStatementBuilder) buildTraceScopeStatement(ctx context.Context, orgID valuer.UUID, start, end uint64, traceExpr string) (*qbtypes.Statement, error) {
+	keys, err := b.fetchKeys(ctx, orgID)
+	if err != nil {
+		return nil, err
+	}
+	sb := sqlbuilder.NewSelectBuilder()
+	maskExpr, resolved, err := b.resolveFor(ctx, orgID, start, end, keys, sb)
+	if err != nil {
+		return nil, err
+	}
+	orderableSet := orderableAliasSet(resolved)
+	if err := validateAggregateFilter(traceExpr, orderableSet); err != nil {
+		return nil, err
+	}
+
+	needed := neededMatchedAliases(nil, traceExpr, orderableSet)
+	selects := []string{"trace_id"}
+	for _, rc := range resolved {
+		if _, ok := needed[rc.alias]; !ok {
+			continue
+		}
+		selects = append(selects, rc.expr+" AS "+quoteAlias(rc.alias))
+	}
+	sb.Select(selects...)
+	sb.From(fmt.Sprintf("%s.%s", tracestelemetryschema.DBName, tracestelemetryschema.SpanIndexV3TableName))
+	sb.Where(
+		sb.GE("timestamp", fmt.Sprintf("%d", start)),
+		sb.L("timestamp", fmt.Sprintf("%d", end)),
+		sb.GE("ts_bucket_start", start/querybuilder.NsToSeconds-querybuilder.BucketAdjustment),
+		sb.LE("ts_bucket_start", end/querybuilder.NsToSeconds),
+		"("+maskExpr+")",
+	)
+	sb.GroupBy("trace_id")
+
+	// the rewriter matches raw key text, so map the trace. form alongside the bare name
+	columnMap := make(map[string]string, len(orderableSet)*2)
+	for a := range orderableSet {
+		columnMap[a] = quoteAlias(a)
+		columnMap[telemetrytypes.FieldContextTrace.StringValue()+"."+a] = quoteAlias(a)
+	}
+	hv, err := querybuilder.NewHavingExpressionRewriter().Rewrite(traceExpr, columnMap)
+	if err != nil {
+		return nil, err
+	}
+	if hv != "" {
+		// escape user text so a literal $ isn't read as an arg marker
+		sb.Having(sqlbuilder.Escape(hv))
+	}
+
+	sql, args := sb.BuildWithFlavor(sqlbuilder.ClickHouse)
+	return &qbtypes.Statement{Query: sql, Args: args}, nil
 }
 
 // buildTraceListQuery wires the CTE pipeline (start/end are nanoseconds):

pkg/statementbuilder/tracesstatementbuilder/statement_builder.go

@@ -32,6 +32,9 @@ type traceQueryStatementBuilder struct {
 	resourceFilterResolver         *resourcefilter.ResourceFingerprintResolver[qbtypes.TraceAggregation]
 	aggExprRewriter                qbtypes.AggExprRewriter
 	skipResourceFingerprintEnabled bool
+	// traceScope is set only on the per-call copy made by BuildTraceScoped; it
+	// constrains the query to trace ids selected by the __trace_scope CTE.
+	traceScope *qbtypes.Statement
 }
 
 var _ qbtypes.StatementBuilder[qbtypes.TraceAggregation] = (*traceQueryStatementBuilder)(nil)
@@ -95,6 +98,38 @@ func NewTraceQueryStatementBuilder(
 	}
 }
 
+// BuildTraceScoped is Build additionally constrained to spans whose trace_id is
+// selected by traceScope. The receiver is copied so the shared builder stays stateless.
+func (b *traceQueryStatementBuilder) BuildTraceScoped(
+	ctx context.Context,
+	orgID valuer.UUID,
+	start uint64,
+	end uint64,
+	requestType qbtypes.RequestType,
+	query qbtypes.QueryBuilderQuery[qbtypes.TraceAggregation],
+	variables map[string]qbtypes.VariableItem,
+	traceScope *qbtypes.Statement,
+) (*qbtypes.Statement, error) {
+	// The scope is wired into the list query only; reject other request types rather
+	// than silently dropping the constraint.
+	if requestType != qbtypes.RequestTypeRaw {
+		return nil, errors.NewInternalf(errors.CodeInternal, "trace-scoped build supports only the raw request type, got %s", requestType.StringValue())
+	}
+	scoped := *b
+	scoped.traceScope = traceScope
+	return scoped.Build(ctx, orgID, start, end, requestType, query, variables)
+}
+
+// attachTraceScope adds the trace-scope condition to sb and returns the CTE fragment
+// + args to prepend; both empty when no scope is set.
+func (b *traceQueryStatementBuilder) attachTraceScope(sb *sqlbuilder.SelectBuilder) (string, []any) {
+	if b.traceScope == nil {
+		return "", nil
+	}
+	sb.Where("trace_id GLOBAL IN (SELECT trace_id FROM __trace_scope)")
+	return fmt.Sprintf("__trace_scope AS (%s)", b.traceScope.Query), b.traceScope.Args
+}
+
 // Build builds a SQL query for traces based on the given parameters.
 func (b *traceQueryStatementBuilder) Build(
 	ctx context.Context,
@@ -318,6 +353,11 @@ func (b *traceQueryStatementBuilder) buildListQuery(
 		cteArgs = append(cteArgs, args)
 	}
 
+	if scopeFrag, scopeArgs := b.attachTraceScope(sb); scopeFrag != "" {
+		cteFragments = append(cteFragments, scopeFrag)
+		cteArgs = append(cteArgs, scopeArgs)
+	}
+
 	for i, field := range query.SelectFields {
 		expr, err := b.fm.ColumnExpressionFor(ctx, orgID, start, end, &field, telemetrytypes.FieldDataTypeUnspecified, keys)
 		if err != nil {

tests/integration/tests/querierai/01_ai_traces.py

@@ -214,6 +214,41 @@ def test_ai_span_list_excludes_non_gen_ai_spans(
     assert "POST /api/chat" not in names  # root span excluded
 
 
+def test_ai_span_list_trace_level_filter(
+    signoz: types.SigNoz,
+    create_user_admin: None,  # pylint: disable=unused-argument
+    get_token: Callable[[str, str], str],
+    insert_traces: Callable[[list[Traces]], None],
+) -> None:
+    """Span list (raw) with a trace-level condition returns only the gen_ai spans of
+    traces whose window-clipped aggregates qualify: of two traces with out-tokens
+    100/300, `trace.output_tokens > 100` keeps only the large one's LLM span."""
+    now = datetime.now(tz=UTC).replace(second=0, microsecond=0)
+    service = "ai-it-spanlist-tracefilter"
+    small = ai_trace(now=now, service=service, user="a", in_tokens=10, out_tokens=100)
+    large = ai_trace(now=now, service=service, user="b", in_tokens=30, out_tokens=300)
+    insert_traces(small + large)
+
+    token = get_token(USER_ADMIN_EMAIL, USER_ADMIN_PASSWORD)
+    start_ms, end_ms = query_window(now)
+
+    query = BuilderQuery(
+        signal="traces",
+        query_type="builder_ai_query",
+        name="A",
+        filter_expression=f"service.name = '{service}' AND trace.output_tokens > 100",
+        limit=10,
+    )
+    response = make_query_request(signoz, token, start_ms, end_ms, [query.to_dict()], request_type=RequestType.RAW)
+    assert response.status_code == HTTPStatus.OK, response.text
+
+    rows = response.json()["data"]["data"]["results"][0]["rows"]
+    assert len(rows) == 1, f"expected only the large trace's LLM span, got {len(rows)} rows"
+    body = json.dumps(rows)
+    assert large[0].trace_id in body
+    assert small[0].trace_id not in body
+
+
 def test_ai_list_having_or_aggregates(
     signoz: types.SigNoz,
     create_user_admin: None,  # pylint: disable=unused-argument

tests/integration/tests/querierai/conftest.py

@@ -0,0 +1,38 @@
+import pytest
+from testcontainers.core.container import Network
+
+from fixtures import types
+from fixtures.signoz import create_signoz
+
+
+@pytest.fixture(name="signoz", scope="package")
+def signoz_ai_observability(
+    network: Network,
+    migrator: types.Operation,  # pylint: disable=unused-argument
+    zeus: types.TestContainerDocker,
+    gateway: types.TestContainerDocker,
+    sqlstore: types.TestContainerSQL,
+    clickhouse: types.TestContainerClickhouse,
+    request: pytest.FixtureRequest,
+    pytestconfig: pytest.Config,
+) -> types.SigNoz:
+    """
+    Package-scoped SigNoz instance with AI observability enabled. builder_ai_query
+    relies on the metadata store surfacing the static gen_ai key definitions
+    (enrichWithGenAIKeys), which is gated on this flag — without it the gate keys
+    (gen_ai.tool.name, gen_ai.agent.name, ...) only resolve once a span carrying
+    them has been ingested.
+    """
+    return create_signoz(
+        network=network,
+        zeus=zeus,
+        gateway=gateway,
+        sqlstore=sqlstore,
+        clickhouse=clickhouse,
+        request=request,
+        pytestconfig=pytestconfig,
+        cache_key="signoz-ai-observability",
+        env_overrides={
+            "SIGNOZ_FLAGGER_CONFIG_BOOLEAN_ENABLE__AI__OBSERVABILITY": True,
+        },
+    )