You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ha...@apache.org on 2022/10/10 10:58:57 UTC
[skywalking-banyandb] 01/01: Fix having senmatic inconsistency
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 <ha...@gmail.com>
AuthorDate: Mon Oct 10 10:55:29 2022 +0000
Fix having senmatic inconsistency
Signed-off-by: Gao Hongtao <ha...@gmail.com>
---
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}),
)