hanahmily commented on code in PR #505:
URL: 
https://github.com/apache/skywalking-banyandb/pull/505#discussion_r1703417074


##########
banyand/internal/storage/index.go:
##########
@@ -245,31 +223,49 @@ func (s *seriesIndex) Search(ctx context.Context, series 
[]*pbv1.Series, opts In
                        span.Stop()
                }()
        }
+       seriesMatchers := make([]index.SeriesMatcher, len(series))
+       for i := range series {
+               seriesMatchers[i], err = 
convertEntityValuesToSeriesMatcher(series[i])
+               if err != nil {
+                       return nil, nil, err
+               }
+       }
+       var query index.Query
+       if opts.Query != nil {
+               query, err = s.store.Query(seriesMatchers, opts.Query)
+       } else {
+               query, err = s.store.Query(seriesMatchers, nil)
+       }

Review Comment:
   ```suggestion
        query, err = s.store.Query(seriesMatchers, opts.Query)
   ```



##########
pkg/index/index.go:
##########
@@ -204,8 +206,7 @@ type SeriesDocument struct {
 type SeriesStore interface {
        Store
        // Search returns a list of series that match the given matchers.
-       Search(context.Context, []SeriesMatcher, []FieldKey) ([]SeriesDocument, 
error)
-       Execute(context.Context, Query) (posting.List, error)
+       Search(context.Context, []SeriesMatcher, []FieldKey, Query) 
([]SeriesDocument, error)

Review Comment:
   I might merge `SeriesMatcher` and `Query` into `Query`. The storage.index 
could build the query first, then pass it to the `Search`.



##########
pkg/index/index.go:
##########
@@ -156,7 +157,8 @@ type Writer interface {
 
 // FieldIterable allows building a FieldIterator.
 type FieldIterable interface {
-       Iterator(fieldKey FieldKey, termRange RangeOpts, order modelv1.Sort, 
preLoadSize int) (iter FieldIterator[*DocumentResult], err error)
+       Query(seriesMatchers []SeriesMatcher, secondaryQuery Query) (Query, 
error)

Review Comment:
   May we call it `BuildQuery`?



##########
pkg/index/index.go:
##########
@@ -156,7 +157,8 @@ type Writer interface {
 
 // FieldIterable allows building a FieldIterator.
 type FieldIterable interface {
-       Iterator(fieldKey FieldKey, termRange RangeOpts, order modelv1.Sort, 
preLoadSize int) (iter FieldIterator[*DocumentResult], err error)
+       Query(seriesMatchers []SeriesMatcher, secondaryQuery Query) (Query, 
error)
+       Iterator(fieldKey FieldKey, termRange RangeOpts, order modelv1.Sort, 
preLoadSize int, query Query, fields []string) (iter 
FieldIterator[*DocumentResult], err error)

Review Comment:
   ```suggestion
        Iterator(fieldKey FieldKey, termRange RangeOpts, order modelv1.Sort, 
preLoadSize int, query Query, fields []FieldKey) (iter 
FieldIterator[*DocumentResult], err error)
   ```
   
   Please ensure that the semantics are consistent with `SeriesStore.Search`.



-- 
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]

Reply via email to