Copilot commented on code in PR #1032:
URL:
https://github.com/apache/skywalking-banyandb/pull/1032#discussion_r3008438629
##########
pkg/query/logical/trace/trace_plan_tag_filter.go:
##########
@@ -96,6 +96,11 @@ func (uis *unresolvedTraceTagFilter) Analyze(s
logical.Schema) (logical.Plan, er
}
}
+ // Add entity tags to projection for row-level filtering
+ // Entity tags are not in SIDX blocks (handled by series routing) but
need to be
+ // projected so they can be validated at the row level during span
filtering
+ ctx.projectionTags.Names = append(ctx.projectionTags.Names,
entityList...)
Review Comment:
`ctx.projectionTags.Names = append(ctx.projectionTags.Names, entityList...)`
adds *all* entity tags to the read projection for every trace query (including
queries with no criteria), which increases tag IO and memory for results that
will later be discarded unless the client explicitly requested those tags.
Consider only adding `entityList` when it is actually required for correctness
(e.g., when `len(traceIDs) > 0` so entity-tag conditions are evaluated via
`tagFilter`), or when the client’s `TagProjection` explicitly includes entity
tags.
```suggestion
// Add entity tags to projection for row-level filtering when needed
// Entity tags are not in SIDX blocks (handled by series routing) but
need to be
// projected so they can be validated at the row level during span
filtering.
// To avoid unnecessary IO and memory usage, only include them when
traceID-based
// filtering is active (i.e., when entity-tag conditions are evaluated
via tagFilter).
if len(traceIDs) > 0 {
ctx.projectionTags.Names = append(ctx.projectionTags.Names,
entityList...)
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
