This is an automated email from the ASF dual-hosted git repository. hanahmily pushed a commit to branch query-expr in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
commit b6ae669d9c811bfe11660267e69a79fcda46aaa0 Author: Gao Hongtao <[email protected]> AuthorDate: Mon Oct 10 10:55:29 2022 +0000 Fix having senmatic inconsistency Signed-off-by: Gao Hongtao <[email protected]> --- pkg/query/logical/common.go | 11 +- pkg/query/logical/expr_literal.go | 115 ++++++++++++++++++++- pkg/query/logical/index_filter.go | 4 +- pkg/query/logical/interface.go | 1 + pkg/query/logical/stream/stream_plan_tag_filter.go | 2 +- pkg/query/logical/tag_filter.go | 4 +- pkg/test/stream/testdata/stream.json | 4 + .../stream/data/input/having_non_indexed.yaml | 33 ++++++ .../stream/data/input/having_non_indexed_arr.yaml | 33 ++++++ test/cases/stream/data/testdata/data.json | 7 +- .../cases/stream/data/want/having_non_indexed.yaml | 75 ++++++++++++++ .../stream/data/want/having_non_indexed_arr.yaml | 57 ++++++++++ test/cases/stream/stream.go | 2 + 13 files changed, 338 insertions(+), 10 deletions(-) diff --git a/pkg/query/logical/common.go b/pkg/query/logical/common.go index e236b16..a80b0fc 100644 --- a/pkg/query/logical/common.go +++ b/pkg/query/logical/common.go @@ -35,9 +35,10 @@ var ( ErrIncompatibleQueryCondition = errors.New("incompatible query condition type") ErrIndexNotDefined = errors.New("index is not define for the tag") ErrMultipleGlobalIndexes = errors.New("multiple global indexes are not supported") -) + ErrInvalidData = errors.New("data is invalid") -var ErrInvalidData = errors.New("data is invalid") + nullTag = &modelv1.TagValue{Value: &modelv1.TagValue_Null{}} +) type ( SeekerBuilder func(builder tsdb.SeekerBuilder) @@ -76,7 +77,11 @@ func ProjectItem(ec executor.ExecutionContext, item tsdb.Item, projectionFieldRe len(parsedTagFamily.Tags), familyName, len(refs)) } for j, ref := range refs { - tags[j] = parsedTagFamily.GetTags()[ref.Spec.TagIdx] + if len(parsedTagFamily.GetTags()) > ref.Spec.TagIdx { + tags[j] = parsedTagFamily.GetTags()[ref.Spec.TagIdx] + } else { + tags[j] = &modelv1.Tag{Key: ref.Tag.name, Value: nullTag} + } } tagFamily[i] = &modelv1.TagFamily{ diff --git a/pkg/query/logical/expr_literal.go b/pkg/query/logical/expr_literal.go index 9624552..0b8bc89 100644 --- a/pkg/query/logical/expr_literal.go +++ b/pkg/query/logical/expr_literal.go @@ -46,6 +46,18 @@ func (i *int64Literal) Compare(other LiteralExpr) (int, bool) { return 0, false } +func (i *int64Literal) Contains(other LiteralExpr) bool { + if o, ok := other.(*int64Literal); ok { + return i == o + } + if o, ok := other.(*int64ArrLiteral); ok { + if len(o.arr) == 1 && o.arr[0] == i.int64 { + return true + } + } + return false +} + func (i *int64Literal) BelongTo(other LiteralExpr) bool { if o, ok := other.(*int64Literal); ok { return i == o @@ -96,7 +108,7 @@ func (i *int64ArrLiteral) Compare(other LiteralExpr) (int, bool) { return 0, false } -func (i *int64ArrLiteral) BelongTo(other LiteralExpr) bool { +func (i *int64ArrLiteral) Contains(other LiteralExpr) bool { if o, ok := other.(*int64Literal); ok { return slices.Contains(i.arr, o.int64) } @@ -111,6 +123,24 @@ func (i *int64ArrLiteral) BelongTo(other LiteralExpr) bool { return false } +func (i *int64ArrLiteral) BelongTo(other LiteralExpr) bool { + if o, ok := other.(*int64Literal); ok { + if len(i.arr) == 1 && i.arr[0] == o.int64 { + return true + } + return false + } + if o, ok := other.(*int64ArrLiteral); ok { + for _, v := range i.arr { + if !slices.Contains(o.arr, v) { + return false + } + } + return true + } + return false +} + func (i *int64ArrLiteral) Bytes() [][]byte { b := make([][]byte, 0, len(i.arr)) for _, i := range i.arr { @@ -157,6 +187,18 @@ func (s *strLiteral) Compare(other LiteralExpr) (int, bool) { return 0, false } +func (s *strLiteral) Contains(other LiteralExpr) bool { + if o, ok := other.(*strLiteral); ok { + return s == o + } + if o, ok := other.(*strArrLiteral); ok { + if len(o.arr) == 1 && o.arr[0] == s.string { + return true + } + } + return false +} + func (s *strLiteral) BelongTo(other LiteralExpr) bool { if o, ok := other.(*strLiteral); ok { return s == o @@ -207,7 +249,7 @@ func (s *strArrLiteral) Compare(other LiteralExpr) (int, bool) { return 0, false } -func (s *strArrLiteral) BelongTo(other LiteralExpr) bool { +func (s *strArrLiteral) Contains(other LiteralExpr) bool { if o, ok := other.(*strLiteral); ok { return slices.Contains(s.arr, o.string) } @@ -222,6 +264,24 @@ func (s *strArrLiteral) BelongTo(other LiteralExpr) bool { return false } +func (s *strArrLiteral) BelongTo(other LiteralExpr) bool { + if o, ok := other.(*strLiteral); ok { + if len(s.arr) == 1 && s.arr[0] == o.string { + return true + } + return false + } + if o, ok := other.(*strArrLiteral); ok { + for _, v := range s.arr { + if !slices.Contains(o.arr, v) { + return false + } + } + return true + } + return false +} + func (s *strArrLiteral) Bytes() [][]byte { b := make([][]byte, 0, len(s.arr)) for _, str := range s.arr { @@ -269,6 +329,21 @@ func (s *idLiteral) Compare(other LiteralExpr) (int, bool) { return 0, false } +func (s *idLiteral) Contains(other LiteralExpr) bool { + if o, ok := other.(*idLiteral); ok { + return s == o + } + if o, ok := other.(*strLiteral); ok { + return s.string == o.string + } + if o, ok := other.(*strArrLiteral); ok { + if len(o.arr) == 1 && o.arr[0] == s.string { + return true + } + } + return false +} + func (s *idLiteral) BelongTo(other LiteralExpr) bool { if o, ok := other.(*idLiteral); ok { return s == o @@ -331,3 +406,39 @@ func (b *bytesLiteral) DataType() int32 { func (b *bytesLiteral) String() string { return hex.EncodeToString(b.bb) } + +var ( + _ LiteralExpr = (*nullLiteral)(nil) + _ ComparableExpr = (*nullLiteral)(nil) + nullLiteralExpr = &nullLiteral{} +) + +type nullLiteral struct{} + +func (s nullLiteral) Compare(other LiteralExpr) (int, bool) { + return 0, false +} + +func (s nullLiteral) BelongTo(other LiteralExpr) bool { + return false +} + +func (s nullLiteral) Contains(other LiteralExpr) bool { + return false +} + +func (s nullLiteral) Bytes() [][]byte { + return nil +} + +func (s nullLiteral) Equal(expr Expr) bool { + return false +} + +func (s nullLiteral) DataType() int32 { + return int32(databasev1.TagType_TAG_TYPE_UNSPECIFIED) +} + +func (s nullLiteral) String() string { + return "null" +} diff --git a/pkg/query/logical/index_filter.go b/pkg/query/logical/index_filter.go index 847dbd3..059ef34 100644 --- a/pkg/query/logical/index_filter.go +++ b/pkg/query/logical/index_filter.go @@ -180,6 +180,8 @@ func parseExprOrEntity(entityDict map[string]int, entity tsdb.Entity, cond *mode return &int64ArrLiteral{ arr: v.IntArray.GetValue(), }, nil, nil + case *model_v1.TagValue_Null: + return nullLiteralExpr, nil, nil } return nil, nil, ErrInvalidConditionType } @@ -580,7 +582,7 @@ func (bl bypassList) Max() (common.ItemID, error) { } func (bl bypassList) Len() int { - panic("not invoked") + return 0 } func (bl bypassList) Iterator() posting.Iterator { diff --git a/pkg/query/logical/interface.go b/pkg/query/logical/interface.go index 2fce58b..6be21ac 100644 --- a/pkg/query/logical/interface.go +++ b/pkg/query/logical/interface.go @@ -51,4 +51,5 @@ type ComparableExpr interface { LiteralExpr Compare(LiteralExpr) (int, bool) BelongTo(LiteralExpr) bool + Contains(LiteralExpr) bool } diff --git a/pkg/query/logical/stream/stream_plan_tag_filter.go b/pkg/query/logical/stream/stream_plan_tag_filter.go index b7ab756..c71e48b 100644 --- a/pkg/query/logical/stream/stream_plan_tag_filter.go +++ b/pkg/query/logical/stream/stream_plan_tag_filter.go @@ -67,7 +67,7 @@ func (uis *unresolvedTagFilter) Analyze(s logical.Schema) (logical.Plan, error) var errProject error ctx.projTagsRefs, errProject = s.CreateTagRef(uis.projectionTags...) if errProject != nil { - return nil, err + return nil, errProject } } plan, err := uis.selectIndexScanner(ctx) diff --git a/pkg/query/logical/tag_filter.go b/pkg/query/logical/tag_filter.go index 08718b4..fd0c58a 100644 --- a/pkg/query/logical/tag_filter.go +++ b/pkg/query/logical/tag_filter.go @@ -130,6 +130,8 @@ func parseExpr(value *model_v1.TagValue) (ComparableExpr, error) { return &int64ArrLiteral{ arr: v.IntArray.GetValue(), }, nil + case *model_v1.TagValue_Null: + return nullLiteralExpr, nil } return nil, ErrInvalidConditionType } @@ -438,7 +440,7 @@ func (h *havingTag) Match(tagFamilies []*model_v1.TagFamily) (bool, error) { if err != nil { return false, err } - return expr.BelongTo(h.Expr), nil + return expr.Contains(h.Expr), nil } func (h *havingTag) MarshalJSON() ([]byte, error) { diff --git a/pkg/test/stream/testdata/stream.json b/pkg/test/stream/testdata/stream.json index bb996a0..453f461 100644 --- a/pkg/test/stream/testdata/stream.json +++ b/pkg/test/stream/testdata/stream.json @@ -80,6 +80,10 @@ { "name": "extended_tags", "type": "TAG_TYPE_STRING_ARRAY" + }, + { + "name": "non_indexed_tags", + "type": "TAG_TYPE_STRING_ARRAY" } ] } diff --git a/test/cases/stream/data/input/having_non_indexed.yaml b/test/cases/stream/data/input/having_non_indexed.yaml new file mode 100644 index 0000000..4cba244 --- /dev/null +++ b/test/cases/stream/data/input/having_non_indexed.yaml @@ -0,0 +1,33 @@ +# 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. + +metadata: + group: "default" + name: "sw" +projection: + tagFamilies: + - name: "searchable" + tags: ["trace_id", "non_indexed_tags"] + - name: "data" + tags: ["data_binary"] +criteria: + condition: + name: "non_indexed_tags" + op: "BINARY_OP_HAVING" + value: + str: + value: "c" \ No newline at end of file diff --git a/test/cases/stream/data/input/having_non_indexed_arr.yaml b/test/cases/stream/data/input/having_non_indexed_arr.yaml new file mode 100644 index 0000000..c23b82c --- /dev/null +++ b/test/cases/stream/data/input/having_non_indexed_arr.yaml @@ -0,0 +1,33 @@ +# 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. + +metadata: + group: "default" + name: "sw" +projection: + tagFamilies: + - name: "searchable" + tags: ["trace_id", "non_indexed_tags"] + - name: "data" + tags: ["data_binary"] +criteria: + condition: + name: "non_indexed_tags" + op: "BINARY_OP_HAVING" + value: + strArray: + value: ["b","c"] \ No newline at end of file diff --git a/test/cases/stream/data/testdata/data.json b/test/cases/stream/data/testdata/data.json index a13ff12..e12eb30 100644 --- a/test/cases/stream/data/testdata/data.json +++ b/test/cases/stream/data/testdata/data.json @@ -46,6 +46,7 @@ {"null":0}, {"null":0}, {"null":0}, + {"str_array":{"value": ["c"]}}, {"str_array":{"value": ["c"]}} ] }, @@ -66,7 +67,8 @@ {"null":0}, {"null":0}, {"null":0}, - {"str_array":{"value": ["b", "c"]}} + {"str_array":{"value": ["b", "c"]}}, + {"str_array":{"value": ["b", "c"]}} ] }, { @@ -86,7 +88,8 @@ {"null":0}, {"null":0}, {"null":0}, - {"str_array":{"value": ["a", "b", "c"]}} + {"str_array":{"value": ["a", "b", "c"]}}, + {"str_array":{"value": ["a", "b", "c"]}} ] } ] \ No newline at end of file diff --git a/test/cases/stream/data/want/having_non_indexed.yaml b/test/cases/stream/data/want/having_non_indexed.yaml new file mode 100644 index 0000000..f63057e --- /dev/null +++ b/test/cases/stream/data/want/having_non_indexed.yaml @@ -0,0 +1,75 @@ +# 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. + +elements: + - elementId: "2" + tagFamilies: + - name: searchable + tags: + - key: trace_id + value: + str: + value: "3" + - key: non_indexed_tags + value: + strArray: + value: + - c + - name: data + tags: + - key: data_binary + value: + binaryData: YWJjMTIzIT8kKiYoKSctPUB+ + - elementId: "3" + tagFamilies: + - name: searchable + tags: + - key: trace_id + value: + str: + value: "4" + - key: non_indexed_tags + value: + strArray: + value: + - b + - c + - name: data + tags: + - key: data_binary + value: + binaryData: YWJjMTIzIT8kKiYoKSctPUB+ + - elementId: "4" + tagFamilies: + - name: searchable + tags: + - key: trace_id + value: + str: + value: "5" + - key: non_indexed_tags + value: + strArray: + value: + - a + - b + - c + - name: data + tags: + - key: data_binary + value: + binaryData: YWJjMTIzIT8kKiYoKSctPUB+ \ No newline at end of file diff --git a/test/cases/stream/data/want/having_non_indexed_arr.yaml b/test/cases/stream/data/want/having_non_indexed_arr.yaml new file mode 100644 index 0000000..df924ec --- /dev/null +++ b/test/cases/stream/data/want/having_non_indexed_arr.yaml @@ -0,0 +1,57 @@ +# 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. + +elements: + - elementId: "3" + tagFamilies: + - name: searchable + tags: + - key: trace_id + value: + str: + value: "4" + - key: non_indexed_tags + value: + strArray: + value: + - b + - c + - name: data + tags: + - key: data_binary + value: + binaryData: YWJjMTIzIT8kKiYoKSctPUB+ + - elementId: "4" + tagFamilies: + - name: searchable + tags: + - key: trace_id + value: + str: + value: "5" + - key: non_indexed_tags + value: + strArray: + value: + - a + - b + - c + - name: data + tags: + - key: data_binary + value: + binaryData: YWJjMTIzIT8kKiYoKSctPUB+ \ No newline at end of file diff --git a/test/cases/stream/stream.go b/test/cases/stream/stream.go index 88f7ab8..210b973 100644 --- a/test/cases/stream/stream.go +++ b/test/cases/stream/stream.go @@ -55,6 +55,8 @@ var _ = g.DescribeTable("Scanning Streams", verify, g.Entry("numeric local index: less and eq", helpers.Args{Input: "less_eq", Duration: 1 * time.Hour}), g.Entry("logical expression", helpers.Args{Input: "logical", Duration: 1 * time.Hour}), g.Entry("having", helpers.Args{Input: "having", Duration: 1 * time.Hour}), + g.Entry("having non indexed", helpers.Args{Input: "having_non_indexed", Duration: 1 * time.Hour}), + g.Entry("having non indexed array", helpers.Args{Input: "having_non_indexed_arr", Duration: 1 * time.Hour}), g.Entry("full text searching", helpers.Args{Input: "search", Duration: 1 * time.Hour}), g.Entry("indexed only tags", helpers.Args{Input: "indexed_only", Duration: 1 * time.Hour}), )
