This is an automated email from the ASF dual-hosted git repository. hanahmily pushed a commit to branch trace/sidx in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
commit fc032492c878ebaf89869f4762cdef3b815a1b05 Author: Gao Hongtao <[email protected]> AuthorDate: Sun Aug 31 09:02:18 2025 +0800 Refactor trace query handling by removing redundant validation and optimizing entity management in trace filters. Updated test cases to include ordering by timestamp. --- banyand/internal/sidx/interfaces.go | 6 ------ banyand/trace/trace.go | 12 ++++++------ pkg/query/logical/trace/schema.go | 16 ++++------------ pkg/query/logical/trace/trace_plan_tag_filter.go | 12 ++++++++---- test/cases/trace/data/data.go | 5 +---- test/cases/trace/trace.go | 1 + 6 files changed, 20 insertions(+), 32 deletions(-) diff --git a/banyand/internal/sidx/interfaces.go b/banyand/internal/sidx/interfaces.go index 6a8b11ad..074bc3e5 100644 --- a/banyand/internal/sidx/interfaces.go +++ b/banyand/internal/sidx/interfaces.go @@ -390,12 +390,6 @@ func (qr QueryRequest) Validate() error { if qr.MinKey != nil && qr.MaxKey != nil && *qr.MinKey > *qr.MaxKey { return fmt.Errorf("MinKey cannot be greater than MaxKey") } - // Validate tag projection names - for i, projection := range qr.TagProjection { - if projection.Family == "" { - return fmt.Errorf("tagProjection[%d] family cannot be empty", i) - } - } return nil } diff --git a/banyand/trace/trace.go b/banyand/trace/trace.go index dfa552d9..fa9da38c 100644 --- a/banyand/trace/trace.go +++ b/banyand/trace/trace.go @@ -151,12 +151,12 @@ func (t *trace) querySidxForTraceIDs(ctx context.Context, sidxInstances []sidx.S } // Set key range based on time range - if tqo.TimeRange != nil { - minKey := tqo.TimeRange.Start.UnixNano() - maxKey := tqo.TimeRange.End.UnixNano() - req.MinKey = &minKey - req.MaxKey = &maxKey - } + // if tqo.TimeRange != nil { + // minKey := tqo.TimeRange.Start.UnixNano() + // maxKey := tqo.TimeRange.End.UnixNano() + // req.MinKey = &minKey + // req.MaxKey = &maxKey + // } // Use the provided series IDs for targeted querying if len(seriesIDs) > 0 { diff --git a/pkg/query/logical/trace/schema.go b/pkg/query/logical/trace/schema.go index 624686c8..69057712 100644 --- a/pkg/query/logical/trace/schema.go +++ b/pkg/query/logical/trace/schema.go @@ -74,24 +74,16 @@ func (s *schema) CreateFieldRef(_ ...*logical.Field) ([]*logical.FieldRef, error panic("no field for trace") } -func (s *schema) IndexRuleDefined(_ string) (bool, *databasev1.IndexRule) { - return false, &databasev1.IndexRule{} +func (s *schema) IndexRuleDefined(name string) (bool, *databasev1.IndexRule) { + return s.common.IndexRuleDefined(name) } func (s *schema) EntityList() []string { return s.common.EntityList } -// IndexDefined checks whether the field given is indexed. -// For trace, we check if tagName matches the last tag in any index rule. -func (s *schema) IndexDefined(tagName string) (bool, *databasev1.IndexRule) { - for _, rule := range s.common.IndexRules { - tags := rule.GetTags() - if len(tags) > 0 && tags[len(tags)-1] == tagName { - return true, rule - } - } - return false, nil +func (s *schema) IndexDefined(_ string) (bool, *databasev1.IndexRule) { + panic("trace does not support finding index by tag name") } // CreateTagRef create TagRef to the given tags. diff --git a/pkg/query/logical/trace/trace_plan_tag_filter.go b/pkg/query/logical/trace/trace_plan_tag_filter.go index 8bb3aeca..b6086044 100644 --- a/pkg/query/logical/trace/trace_plan_tag_filter.go +++ b/pkg/query/logical/trace/trace_plan_tag_filter.go @@ -27,6 +27,7 @@ import ( "github.com/apache/skywalking-banyandb/pkg/index" "github.com/apache/skywalking-banyandb/pkg/iter" "github.com/apache/skywalking-banyandb/pkg/logger" + pbv1 "github.com/apache/skywalking-banyandb/pkg/pb/v1" "github.com/apache/skywalking-banyandb/pkg/query/executor" "github.com/apache/skywalking-banyandb/pkg/query/logical" "github.com/apache/skywalking-banyandb/pkg/query/model" @@ -49,15 +50,18 @@ func (uis *unresolvedTraceTagFilter) Analyze(s logical.Schema) (logical.Plan, er ctx := newTraceAnalyzerContext(s) entityList := s.EntityList() entityDict := make(map[string]int) + entity := make([]*modelv1.TagValue, len(entityList)) for idx, e := range entityList { entityDict[e] = idx + // fill AnyEntry by default + entity[idx] = pbv1.AnyTagValue } var err error var conditionTagNames []string var traceIDs []string var entities [][]*modelv1.TagValue // For trace, we use skipping filter and capture entities for query optimization - ctx.skippingFilter, entities, conditionTagNames, traceIDs, err = buildTraceFilter(uis.criteria, s, entityDict, uis.traceIDTagName) + ctx.skippingFilter, entities, conditionTagNames, traceIDs, err = buildTraceFilter(uis.criteria, s, entityDict, entity, uis.traceIDTagName) if err != nil { return nil, err } @@ -156,10 +160,10 @@ func deduplicateStrings(strings []string) []string { // buildTraceFilter builds a filter for trace queries and returns both the filter and collected tag names. // Unlike stream, trace only needs skipping filter. func buildTraceFilter(criteria *modelv1.Criteria, s logical.Schema, entityDict map[string]int, - traceIDTagName string, + entity []*modelv1.TagValue, traceIDTagName string, ) (index.Filter, [][]*modelv1.TagValue, []string, []string, error) { if criteria == nil { - return nil, nil, nil, nil, nil + return nil, [][]*modelv1.TagValue{entity}, nil, nil, nil } // Create a map of valid tag names from the schema tagNames := make(map[string]bool) @@ -168,7 +172,7 @@ func buildTraceFilter(criteria *modelv1.Criteria, s logical.Schema, entityDict m tagNames[tagName] = true } - filter, entities, collectedTagNames, traceIDs, err := buildFilter(criteria, tagNames, entityDict, nil, traceIDTagName) + filter, entities, collectedTagNames, traceIDs, err := buildFilter(criteria, tagNames, entityDict, entity, traceIDTagName) return filter, entities, collectedTagNames, traceIDs, err } diff --git a/test/cases/trace/data/data.go b/test/cases/trace/data/data.go index 71a6c6a5..90de5657 100644 --- a/test/cases/trace/data/data.go +++ b/test/cases/trace/data/data.go @@ -21,7 +21,6 @@ package data import ( "context" "embed" - "encoding/base64" "encoding/json" "fmt" "io" @@ -141,8 +140,6 @@ func loadData(stream tracev1.TraceService_WriteClient, metadata *commonv1.Metada // Get span data spanData, ok := templateMap["span"].(string) gm.Expect(ok).To(gm.BeTrue()) - spanBytes, err := base64.StdEncoding.DecodeString(spanData) - gm.Expect(err).ShouldNot(gm.HaveOccurred()) // Get tags data tagsData, ok := templateMap["tags"].([]interface{}) @@ -170,7 +167,7 @@ func loadData(stream tracev1.TraceService_WriteClient, metadata *commonv1.Metada errInner := stream.Send(&tracev1.WriteRequest{ Metadata: metadata, Tags: tagValues, - Span: spanBytes, + Span: []byte(spanData), Version: uint64(i + 1), }) gm.Expect(errInner).ShouldNot(gm.HaveOccurred()) diff --git a/test/cases/trace/trace.go b/test/cases/trace/trace.go index 824b2c35..4ade3006 100644 --- a/test/cases/trace/trace.go +++ b/test/cases/trace/trace.go @@ -43,4 +43,5 @@ var _ = g.DescribeTable("Scanning Traces", func(args helpers.Args) { }, flags.EventuallyTimeout).Should(gm.Succeed()) }, g.Entry("query by trace id", helpers.Args{Input: "eq_trace_id", Duration: 1 * time.Hour}), + g.FEntry("order by timestamp", helpers.Args{Input: "order_timestamp_desc", Duration: 1 * time.Hour}), )
