You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/09/02 02:11:26 UTC
[GitHub] [skywalking-banyandb] hanahmily opened a new pull request, #170: [BREAKING CHANGES] Introduce OR logical operation
hanahmily opened a new pull request, #170:
URL: https://github.com/apache/skywalking-banyandb/pull/170
These commits will introduce:
* [x] Introduce OR logical operation to APIs
* [x] Add tree-based IndexFilter and TagFilter to filter items
* [x] Update relevant Stream query components
* [ ] Update relevant TSDB query components
* [ ] Update relevant Measure query components
The `Criteria` update brings a lot of changes to query testing of `Liaison`, `Stream`, `Measure`, and `Query` modules. This PR will merge them to `test/integration` to reduce the duplicated cases and accelerate the testing process.
* [ ] Move Stream query test cases
* [ ] Move Mesure query test cases
* [ ] Move Query execution cases
* [ ] Move Liaison test cases
We should tweak The analysis test cases of Query due to the tricky compare operations. I will skip them since we don't have the analysis result query APIs. The follow-up PRs will bring them back.
Signed-off-by: Gao Hongtao <ha...@gmail.com>
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-banyandb] hanahmily merged pull request #170: [BREAKING CHANGES] Introduce OR logical operation
Posted by GitBox <gi...@apache.org>.
hanahmily merged PR #170:
URL: https://github.com/apache/skywalking-banyandb/pull/170
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-banyandb] lujiajing1126 commented on a diff in pull request #170: [BREAKING CHANGES] Introduce OR logical operation
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #170:
URL: https://github.com/apache/skywalking-banyandb/pull/170#discussion_r973715445
##########
pkg/query/logical/index_filter.go:
##########
@@ -0,0 +1,647 @@
+// 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.
+
+package logical
+
+import (
+ "bytes"
+ "encoding/base64"
+ "encoding/json"
+ "strings"
+
+ "github.com/apache/skywalking-banyandb/api/common"
+ database_v1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/database/v1"
+ model_v1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/model/v1"
+ "github.com/apache/skywalking-banyandb/banyand/tsdb"
+ "github.com/apache/skywalking-banyandb/pkg/convert"
+ "github.com/apache/skywalking-banyandb/pkg/index"
+ "github.com/apache/skywalking-banyandb/pkg/index/posting"
+ "github.com/pkg/errors"
Review Comment:
A format issue
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-banyandb] codecov-commenter commented on pull request #170: [BREAKING CHANGES] Introduce OR logical operation
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #170:
URL: https://github.com/apache/skywalking-banyandb/pull/170#issuecomment-1247439440
# [Codecov](https://codecov.io/gh/apache/skywalking-banyandb/pull/170?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#170](https://codecov.io/gh/apache/skywalking-banyandb/pull/170?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cd97de4) into [main](https://codecov.io/gh/apache/skywalking-banyandb/commit/40adf38c044441e4fd568a3b18088f5674988786?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (40adf38) will **decrease** coverage by `13.44%`.
> The diff coverage is `1.43%`.
```diff
@@ Coverage Diff @@
## main #170 +/- ##
===========================================
- Coverage 53.52% 40.07% -13.45%
===========================================
Files 86 68 -18
Lines 7869 6255 -1614
===========================================
- Hits 4212 2507 -1705
- Misses 3262 3539 +277
+ Partials 395 209 -186
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking-banyandb/pull/170?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [banyand/tsdb/series\_seek.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmFueWFuZC90c2RiL3Nlcmllc19zZWVrLmdv) | `0.00% <0.00%> (ø)` | |
| [banyand/tsdb/series\_seek\_filter.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmFueWFuZC90c2RiL3Nlcmllc19zZWVrX2ZpbHRlci5nbw==) | `0.00% <0.00%> (ø)` | |
| [banyand/tsdb/series\_seek\_sort.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmFueWFuZC90c2RiL3Nlcmllc19zZWVrX3NvcnQuZ28=) | `0.00% <0.00%> (ø)` | |
| [banyand/tsdb/seriesdb.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YmFueWFuZC90c2RiL3Nlcmllc2RiLmdv) | `62.83% <0.00%> (-13.11%)` | :arrow_down: |
| [pkg/query/logical/measure/measure\_analyzer.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3F1ZXJ5L2xvZ2ljYWwvbWVhc3VyZS9tZWFzdXJlX2FuYWx5emVyLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/query/logical/measure/measure\_plan.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3F1ZXJ5L2xvZ2ljYWwvbWVhc3VyZS9tZWFzdXJlX3BsYW4uZ28=) | `0.00% <0.00%> (ø)` | |
| [.../query/logical/measure/measure\_plan\_aggregation.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3F1ZXJ5L2xvZ2ljYWwvbWVhc3VyZS9tZWFzdXJlX3BsYW5fYWdncmVnYXRpb24uZ28=) | `0.00% <0.00%> (ø)` | |
| [pkg/query/logical/measure/measure\_plan\_groupby.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3F1ZXJ5L2xvZ2ljYWwvbWVhc3VyZS9tZWFzdXJlX3BsYW5fZ3JvdXBieS5nbw==) | `0.00% <0.00%> (ø)` | |
| [...ry/logical/measure/measure\_plan\_indexscan\_local.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3F1ZXJ5L2xvZ2ljYWwvbWVhc3VyZS9tZWFzdXJlX3BsYW5faW5kZXhzY2FuX2xvY2FsLmdv) | `0.00% <0.00%> (ø)` | |
| [pkg/query/logical/measure/measure\_plan\_top.go](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL3F1ZXJ5L2xvZ2ljYWwvbWVhc3VyZS9tZWFzdXJlX3BsYW5fdG9wLmdv) | `0.00% <0.00%> (ø)` | |
| ... and [33 more](https://codecov.io/gh/apache/skywalking-banyandb/pull/170/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-banyandb] lujiajing1126 commented on a diff in pull request #170: [BREAKING CHANGES] Introduce OR logical operation
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #170:
URL: https://github.com/apache/skywalking-banyandb/pull/170#discussion_r973715210
##########
pkg/query/logical/expr_literal.go:
##########
@@ -90,32 +89,26 @@ type int64ArrLiteral struct {
arr []int64
}
-func (i *int64ArrLiteral) Compare(tagValue *modelv1.TagValue) (int, bool) {
- intArray := tagValue.GetIntArray()
- if intArray == nil {
- return 0, false
- }
- if slices.Equal(i.arr, intArray.Value) {
- return 0, true
+func (i *int64ArrLiteral) Compare(other LiteralExpr) (int, bool) {
+ if o, ok := other.(*int64ArrLiteral); ok {
+ return 0, slices.Equal(i.arr, o.arr)
}
return 0, false
}
-func (i *int64ArrLiteral) BelongTo(tagValue *modelv1.TagValue) bool {
- intValue := tagValue.GetInt()
- if intValue != nil {
- return slices.Contains(i.arr, intValue.Value)
+func (i *int64ArrLiteral) BelongTo(other LiteralExpr) bool {
Review Comment:
The semantics of `BelongTo` is not consistent for all `ComparableExpr`,
1. For singled values, it mean if `this` is an element of a slice, or it is equal to the element,
2. For slices, it means if the given element is a subset of `this` slice, which inverses the relationship.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-banyandb] hanahmily commented on a diff in pull request #170: [BREAKING CHANGES] Introduce OR logical operation
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #170:
URL: https://github.com/apache/skywalking-banyandb/pull/170#discussion_r973816108
##########
pkg/query/logical/expr_literal.go:
##########
@@ -90,32 +89,26 @@ type int64ArrLiteral struct {
arr []int64
}
-func (i *int64ArrLiteral) Compare(tagValue *modelv1.TagValue) (int, bool) {
- intArray := tagValue.GetIntArray()
- if intArray == nil {
- return 0, false
- }
- if slices.Equal(i.arr, intArray.Value) {
- return 0, true
+func (i *int64ArrLiteral) Compare(other LiteralExpr) (int, bool) {
+ if o, ok := other.(*int64ArrLiteral); ok {
+ return 0, slices.Equal(i.arr, o.arr)
}
return 0, false
}
-func (i *int64ArrLiteral) BelongTo(tagValue *modelv1.TagValue) bool {
- intValue := tagValue.GetInt()
- if intValue != nil {
- return slices.Contains(i.arr, intValue.Value)
+func (i *int64ArrLiteral) BelongTo(other LiteralExpr) bool {
Review Comment:
yes, it sounds like a bug here.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org