Copilot commented on code in PR #1019:
URL:
https://github.com/apache/skywalking-banyandb/pull/1019#discussion_r2972320083
##########
.github/workflows/test-fodc-e2e.yml:
##########
@@ -60,10 +60,11 @@ jobs:
download-artifacts: 'true'
setup-docker: 'true'
- - name: Setup kubectl
- uses: azure/setup-kubectl@v4
- with:
- version: v1.30.0
+ - name: Install kubectl
+ run: |
+ mkdir -p "${HOME}/.local/bin"
+ bash test/e2e-v2/script/prepare/setup-e2e-shell/install-kubectl.sh
/tmp "${HOME}/.local/bin" v1.30.0
Review Comment:
The workflow passes `v1.30.0` as a third argument to `install-kubectl.sh`,
but the script currently ignores positional arg #3 and only reads `K8SVERSION`
from the environment. As a result, this step will still install the script’s
default kubectl version. Set `K8SVERSION=v1.30.0` (or update the script to
accept the version argument) so the intended version is installed.
```suggestion
K8SVERSION=v1.30.0 bash
test/e2e-v2/script/prepare/setup-e2e-shell/install-kubectl.sh /tmp
"${HOME}/.local/bin"
```
##########
banyand/internal/sidx/query.go:
##########
@@ -484,15 +483,17 @@ func extractOrdering(req QueryRequest) bool {
return req.Order.Sort != modelv1.Sort_SORT_DESC
}
-// selectPartsForQuery selects relevant parts from snapshot based on key range.
-func selectPartsForQuery(snap *Snapshot, minKey, maxKey int64) []*part {
+// selectPartsForQuery selects relevant parts from snapshot based on key range
and optional timestamp range.
+func selectPartsForQuery(snap *Snapshot, minKey, maxKey int64, minTS, maxTS
*int64) []*part {
var selectedParts []*part
-
for _, pw := range snap.parts {
- if pw.overlapsKeyRange(minKey, maxKey) {
- selectedParts = append(selectedParts, pw.p)
+ if !pw.overlapsKeyRange(minKey, maxKey) {
+ continue
}
+ if minTS != nil && maxTS != nil &&
!pw.overlapsTimestampRange(*minTS, *maxTS) {
+ continue
Review Comment:
Timestamp pruning is only applied when *both* `minTS` and `maxTS` are
non-nil. If a caller supplies only one bound (open-ended range), part selection
will ignore timestamps entirely and may scan/query irrelevant parts. Consider
supporting one-sided ranges (e.g., if `minTS != nil` then require
`part.MaxTimestamp >= *minTS`; if `maxTS != nil` then require
`part.MinTimestamp <= *maxTS`).
```suggestion
switch {
case minTS != nil && maxTS != nil:
if !pw.overlapsTimestampRange(*minTS, *maxTS) {
continue
}
case minTS != nil:
if !pw.overlapsTimestampRange(*minTS, math.MaxInt64) {
continue
}
case maxTS != nil:
if !pw.overlapsTimestampRange(math.MinInt64, *maxTS) {
continue
}
```
##########
banyand/internal/sidx/sidx.go:
##########
@@ -100,6 +100,8 @@ func (s *sidx) ConvertToMemPart(reqs []WriteRequest,
segmentID int64) (*MemPart,
mp := GenerateMemPart()
mp.mustInitFromElements(es)
mp.partMetadata.SegmentID = segmentID
+ mp.partMetadata.MinTimestamp = minTimestamp
+ mp.partMetadata.MaxTimestamp = maxTimestamp
return mp, nil
Review Comment:
`ConvertToMemPart` now accepts optional `minTimestamp/maxTimestamp` but
doesn’t validate their relationship. If callers accidentally pass `minTimestamp
> maxTimestamp`, the invalid manifest can be written (marshal doesn’t call
`partMetadata.validate()`), and later opens/reads will fail validation. Add a
check here (or in `mustWriteMetadata`/`marshal`) to reject invalid timestamp
ranges early.
##########
banyand/internal/sidx/scan_query.go:
##########
@@ -102,6 +105,21 @@ func (s *sidx) ScanQuery(ctx context.Context, req
ScanQueryRequest) ([]*QueryRes
return results, nil
}
+// selectPartsForScan selects parts that overlap key and optional timestamp
range for scan queries.
+func selectPartsForScan(snap *Snapshot, minKey, maxKey int64, minTS, maxTS
*int64) []*partWrapper {
+ var selected []*partWrapper
+ for _, pw := range snap.parts {
+ if !pw.overlapsKeyRange(minKey, maxKey) {
+ continue
+ }
+ if minTS != nil && maxTS != nil &&
!pw.overlapsTimestampRange(*minTS, *maxTS) {
+ continue
Review Comment:
Timestamp pruning is only applied when *both* `minTS` and `maxTS` are
non-nil. If a caller supplies only one bound (open-ended range), scans will
ignore timestamps entirely. Consider supporting one-sided timestamp ranges in
`selectPartsForScan` similarly to key range handling.
```suggestion
// Apply timestamp pruning when either bound is provided; treat
missing bounds as open-ended.
if minTS != nil || maxTS != nil {
from := int64(math.MinInt64)
to := int64(math.MaxInt64)
if minTS != nil {
from = *minTS
}
if maxTS != nil {
to = *maxTS
}
if !pw.overlapsTimestampRange(from, to) {
continue
}
```
##########
banyand/internal/sidx/timestamp_test.go:
##########
@@ -0,0 +1,394 @@
+// 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 sidx
+
+import (
+ "context"
+ "encoding/json"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "github.com/apache/skywalking-banyandb/api/common"
+ "github.com/apache/skywalking-banyandb/banyand/observability"
+ "github.com/apache/skywalking-banyandb/banyand/protector"
+ "github.com/apache/skywalking-banyandb/pkg/fs"
+)
+
+// writeTestDataWithTimeRange writes test data with optional min/max
timestamps (back-compat: nil means no timestamps).
+func writeTestDataWithTimeRange(t *testing.T, sidx SIDX, reqs []WriteRequest,
segmentID int64, partID uint64, minTS, maxTS *int64) {
+ t.Helper()
+ memPart, err := sidx.ConvertToMemPart(reqs, segmentID, minTS, maxTS)
+ require.NoError(t, err)
+ require.NotNil(t, memPart)
+ sidx.IntroduceMemPart(partID, memPart)
+}
+
+func ptrInt64Ts(v int64) *int64 {
+ val := v
+ return &val
+}
+
+// TestConvertToMemPart_Timestamps verifies ConvertToMemPart with and without
timestamps.
+func TestConvertToMemPart_Timestamps(t *testing.T) {
+ sidx := createTestSIDX(t)
+ defer func() {
+ assert.NoError(t, sidx.Close())
+ }()
+
+ reqs := []WriteRequest{
+ createTestWriteRequest(1, 1000, "data1"),
+ createTestWriteRequest(1, 2000, "data2"),
+ }
+
+ t.Run("nil_timestamps_back_compat", func(t *testing.T) {
+ mp, err := sidx.ConvertToMemPart(reqs, 1, nil, nil)
+ require.NoError(t, err)
+ require.NotNil(t, mp)
+ require.Nil(t, mp.partMetadata.MinTimestamp)
+ require.Nil(t, mp.partMetadata.MaxTimestamp)
+ ReleaseMemPart(mp)
+ })
+
+ t.Run("with_timestamps", func(t *testing.T) {
+ minTS := int64(500)
+ maxTS := int64(2500)
+ mp, err := sidx.ConvertToMemPart(reqs, 1, &minTS, &maxTS)
+ require.NoError(t, err)
+ require.NotNil(t, mp)
+ require.NotNil(t, mp.partMetadata.MinTimestamp)
+ require.NotNil(t, mp.partMetadata.MaxTimestamp)
+ assert.Equal(t, int64(500), *mp.partMetadata.MinTimestamp)
+ assert.Equal(t, int64(2500), *mp.partMetadata.MaxTimestamp)
+ ReleaseMemPart(mp)
+ })
+}
+
+// TestManifest_BackCompat verifies manifest.json omits timestamps when nil
(back-compat).
+func TestManifest_BackCompat(t *testing.T) {
+ dir := t.TempDir()
+ fileSystem := fs.NewLocalFileSystem()
+ opts := NewDefaultOptions()
+ opts.Memory = protector.NewMemory(observability.NewBypassRegistry())
+ opts.Path = dir
+
+ sidxIface, err := NewSIDX(fileSystem, opts)
+ require.NoError(t, err)
+ raw := sidxIface.(*sidx)
+ defer func() {
+ assert.NoError(t, raw.Close())
+ }()
+
+ reqs := []WriteRequest{
+ createTestWriteRequest(1, 100, "data"),
+ }
+
+ t.Run("manifest_without_timestamps", func(t *testing.T) {
+ writeTestDataWithTimeRange(t, raw, reqs, 1, 1, nil, nil)
+ flushIntro, err := raw.Flush(map[uint64]struct{}{1: {}})
+ require.NoError(t, err)
+ raw.IntroduceFlushed(flushIntro)
+ flushIntro.Release()
+
+ manifestPath := filepath.Join(dir, "0000000000000001",
manifestFilename)
+ data, readErr := fileSystem.Read(manifestPath)
+ require.NoError(t, readErr)
+
+ var m struct {
+ MinTimestamp *int64 `json:"minTimestamp,omitempty"`
+ MaxTimestamp *int64 `json:"maxTimestamp,omitempty"`
+ MinKey int64 `json:"minKey"`
+ MaxKey int64 `json:"maxKey"`
+ }
+ require.NoError(t, json.Unmarshal(data, &m))
+ require.Nil(t, m.MinTimestamp)
+ require.Nil(t, m.MaxTimestamp)
+ assert.Equal(t, int64(100), m.MinKey)
+ assert.Equal(t, int64(100), m.MaxKey)
+ })
+
+ t.Run("manifest_with_timestamps", func(t *testing.T) {
+ minTS := int64(1000)
+ maxTS := int64(2000)
+ writeTestDataWithTimeRange(t, raw, reqs, 2, 2, &minTS, &maxTS)
+ flushIntro, err := raw.Flush(map[uint64]struct{}{2: {}})
+ require.NoError(t, err)
+ raw.IntroduceFlushed(flushIntro)
+ flushIntro.Release()
+
+ manifestPath := filepath.Join(dir, "0000000000000002",
manifestFilename)
+ data, readErr := fileSystem.Read(manifestPath)
+ require.NoError(t, readErr)
+
+ var m struct {
+ MinTimestamp *int64 `json:"minTimestamp,omitempty"`
+ MaxTimestamp *int64 `json:"maxTimestamp,omitempty"`
+ }
+ require.NoError(t, json.Unmarshal(data, &m))
+ require.NotNil(t, m.MinTimestamp)
+ require.NotNil(t, m.MaxTimestamp)
+ assert.Equal(t, int64(1000), *m.MinTimestamp)
+ assert.Equal(t, int64(2000), *m.MaxTimestamp)
+ })
+}
+
+// TestQuery_TimestampSelection verifies timestamp-aware part selection and
key-range fallback.
+func TestQuery_TimestampSelection(t *testing.T) {
+ reqs := []WriteRequest{
+ createTestWriteRequest(1, 1000, "data1000"),
+ createTestWriteRequest(1, 2000, "data2000"),
+ }
+
+ t.Run("back_compat_key_range_only", func(t *testing.T) {
+ sidx := createTestSIDX(t)
+ defer func() {
+ assert.NoError(t, sidx.Close())
+ }()
+ writeTestDataWithTimeRange(t, sidx, reqs, 1, 1, nil, nil)
+ waitForIntroducerLoop()
+
+ minKey := int64(500)
+ maxKey := int64(2500)
+ qr := QueryRequest{
+ SeriesIDs: []common.SeriesID{1},
+ MinKey: &minKey,
+ MaxKey: &maxKey,
+ }
+ resultsCh, errCh := sidx.StreamingQuery(context.Background(),
qr)
+ var keys []int64
+ for res := range resultsCh {
+ require.NoError(t, res.Error)
+ keys = append(keys, res.Keys...)
+ }
+ select {
+ case err, ok := <-errCh:
+ if ok {
+ require.NoError(t, err)
+ }
+ default:
+ }
+ assert.GreaterOrEqual(t, len(keys), 0, "key-range filter should
work without timestamps")
+ })
+
+ t.Run("timestamp_filter_overlaps", func(t *testing.T) {
+ sidx := createTestSIDX(t)
+ defer func() {
+ assert.NoError(t, sidx.Close())
+ }()
+ minTS := ptrInt64Ts(1000)
+ maxTS := ptrInt64Ts(3000)
+ writeTestDataWithTimeRange(t, sidx, reqs, 1, 1, minTS, maxTS)
+ waitForIntroducerLoop()
+
+ qMinTS := ptrInt64Ts(1500)
+ qMaxTS := ptrInt64Ts(2500)
+ qr := QueryRequest{
+ SeriesIDs: []common.SeriesID{1},
+ MinTimestamp: qMinTS,
+ MaxTimestamp: qMaxTS,
+ }
+ resultsCh, errCh := sidx.StreamingQuery(context.Background(),
qr)
+ var count int
+ for res := range resultsCh {
+ require.NoError(t, res.Error)
+ count += res.Len()
+ }
+ select {
+ case err, ok := <-errCh:
+ if ok {
+ require.NoError(t, err)
+ }
+ default:
+ }
+ assert.GreaterOrEqual(t, count, 0, "query with overlapping
timestamp range")
+ })
+
+ t.Run("timestamp_filter_no_overlap_excludes_part", func(t *testing.T) {
+ sidx := createTestSIDX(t)
+ defer func() {
+ assert.NoError(t, sidx.Close())
+ }()
+ minTS := ptrInt64Ts(1000)
+ maxTS := ptrInt64Ts(2000)
+ writeTestDataWithTimeRange(t, sidx, reqs, 1, 1, minTS, maxTS)
+ waitForIntroducerLoop()
+
+ qMinTS := ptrInt64Ts(5000)
+ qMaxTS := ptrInt64Ts(6000)
+ qr := QueryRequest{
+ SeriesIDs: []common.SeriesID{1},
+ MinTimestamp: qMinTS,
+ MaxTimestamp: qMaxTS,
+ }
+ resultsCh, errCh := sidx.StreamingQuery(context.Background(),
qr)
+ var count int
+ for res := range resultsCh {
+ require.NoError(t, res.Error)
+ count += res.Len()
+ }
+ select {
+ case err, ok := <-errCh:
+ if ok {
+ require.NoError(t, err)
+ }
+ default:
+ }
+ assert.Equal(t, 0, count, "parts outside timestamp range should
be excluded")
+ })
+}
+
+// TestScanQuery_TimestampSelection verifies ScanQuery timestamp-aware part
selection.
+func TestScanQuery_TimestampSelection(t *testing.T) {
+ reqs := []WriteRequest{
+ createTestWriteRequest(1, 100, "data"),
+ }
+
+ t.Run("back_compat_no_timestamp_filter", func(t *testing.T) {
+ sidx := createTestSIDX(t)
+ defer func() {
+ assert.NoError(t, sidx.Close())
+ }()
+ writeTestDataWithTimeRange(t, sidx, reqs, 1, 1, nil, nil)
+ waitForIntroducerLoop()
+
+ sqr := ScanQueryRequest{}
+ res, err := sidx.ScanQuery(context.Background(), sqr)
+ require.NoError(t, err)
+ assert.GreaterOrEqual(t, len(res), 0)
Review Comment:
This scan test assertion `assert.GreaterOrEqual(t, len(res), 0)` is always
true and doesn’t verify correctness. Please assert expected non-empty results
(or expected row counts) for the back-compat scan case so the new
part-selection logic is actually exercised.
```suggestion
total := 0
for _, r := range res {
total += r.Len()
}
assert.Equal(t, 1, total, "expected one row from back-compat
scan without timestamp filter")
```
##########
banyand/internal/sidx/timestamp_test.go:
##########
@@ -0,0 +1,394 @@
+// 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 sidx
+
+import (
+ "context"
+ "encoding/json"
+ "path/filepath"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "github.com/apache/skywalking-banyandb/api/common"
+ "github.com/apache/skywalking-banyandb/banyand/observability"
+ "github.com/apache/skywalking-banyandb/banyand/protector"
+ "github.com/apache/skywalking-banyandb/pkg/fs"
+)
+
+// writeTestDataWithTimeRange writes test data with optional min/max
timestamps (back-compat: nil means no timestamps).
+func writeTestDataWithTimeRange(t *testing.T, sidx SIDX, reqs []WriteRequest,
segmentID int64, partID uint64, minTS, maxTS *int64) {
+ t.Helper()
+ memPart, err := sidx.ConvertToMemPart(reqs, segmentID, minTS, maxTS)
+ require.NoError(t, err)
+ require.NotNil(t, memPart)
+ sidx.IntroduceMemPart(partID, memPart)
+}
+
+func ptrInt64Ts(v int64) *int64 {
+ val := v
+ return &val
+}
+
+// TestConvertToMemPart_Timestamps verifies ConvertToMemPart with and without
timestamps.
+func TestConvertToMemPart_Timestamps(t *testing.T) {
+ sidx := createTestSIDX(t)
+ defer func() {
+ assert.NoError(t, sidx.Close())
+ }()
+
+ reqs := []WriteRequest{
+ createTestWriteRequest(1, 1000, "data1"),
+ createTestWriteRequest(1, 2000, "data2"),
+ }
+
+ t.Run("nil_timestamps_back_compat", func(t *testing.T) {
+ mp, err := sidx.ConvertToMemPart(reqs, 1, nil, nil)
+ require.NoError(t, err)
+ require.NotNil(t, mp)
+ require.Nil(t, mp.partMetadata.MinTimestamp)
+ require.Nil(t, mp.partMetadata.MaxTimestamp)
+ ReleaseMemPart(mp)
+ })
+
+ t.Run("with_timestamps", func(t *testing.T) {
+ minTS := int64(500)
+ maxTS := int64(2500)
+ mp, err := sidx.ConvertToMemPart(reqs, 1, &minTS, &maxTS)
+ require.NoError(t, err)
+ require.NotNil(t, mp)
+ require.NotNil(t, mp.partMetadata.MinTimestamp)
+ require.NotNil(t, mp.partMetadata.MaxTimestamp)
+ assert.Equal(t, int64(500), *mp.partMetadata.MinTimestamp)
+ assert.Equal(t, int64(2500), *mp.partMetadata.MaxTimestamp)
+ ReleaseMemPart(mp)
+ })
+}
+
+// TestManifest_BackCompat verifies manifest.json omits timestamps when nil
(back-compat).
+func TestManifest_BackCompat(t *testing.T) {
+ dir := t.TempDir()
+ fileSystem := fs.NewLocalFileSystem()
+ opts := NewDefaultOptions()
+ opts.Memory = protector.NewMemory(observability.NewBypassRegistry())
+ opts.Path = dir
+
+ sidxIface, err := NewSIDX(fileSystem, opts)
+ require.NoError(t, err)
+ raw := sidxIface.(*sidx)
+ defer func() {
+ assert.NoError(t, raw.Close())
+ }()
+
+ reqs := []WriteRequest{
+ createTestWriteRequest(1, 100, "data"),
+ }
+
+ t.Run("manifest_without_timestamps", func(t *testing.T) {
+ writeTestDataWithTimeRange(t, raw, reqs, 1, 1, nil, nil)
+ flushIntro, err := raw.Flush(map[uint64]struct{}{1: {}})
+ require.NoError(t, err)
+ raw.IntroduceFlushed(flushIntro)
+ flushIntro.Release()
+
+ manifestPath := filepath.Join(dir, "0000000000000001",
manifestFilename)
+ data, readErr := fileSystem.Read(manifestPath)
+ require.NoError(t, readErr)
+
+ var m struct {
+ MinTimestamp *int64 `json:"minTimestamp,omitempty"`
+ MaxTimestamp *int64 `json:"maxTimestamp,omitempty"`
+ MinKey int64 `json:"minKey"`
+ MaxKey int64 `json:"maxKey"`
+ }
+ require.NoError(t, json.Unmarshal(data, &m))
+ require.Nil(t, m.MinTimestamp)
+ require.Nil(t, m.MaxTimestamp)
+ assert.Equal(t, int64(100), m.MinKey)
+ assert.Equal(t, int64(100), m.MaxKey)
+ })
+
+ t.Run("manifest_with_timestamps", func(t *testing.T) {
+ minTS := int64(1000)
+ maxTS := int64(2000)
+ writeTestDataWithTimeRange(t, raw, reqs, 2, 2, &minTS, &maxTS)
+ flushIntro, err := raw.Flush(map[uint64]struct{}{2: {}})
+ require.NoError(t, err)
+ raw.IntroduceFlushed(flushIntro)
+ flushIntro.Release()
+
+ manifestPath := filepath.Join(dir, "0000000000000002",
manifestFilename)
+ data, readErr := fileSystem.Read(manifestPath)
+ require.NoError(t, readErr)
+
+ var m struct {
+ MinTimestamp *int64 `json:"minTimestamp,omitempty"`
+ MaxTimestamp *int64 `json:"maxTimestamp,omitempty"`
+ }
+ require.NoError(t, json.Unmarshal(data, &m))
+ require.NotNil(t, m.MinTimestamp)
+ require.NotNil(t, m.MaxTimestamp)
+ assert.Equal(t, int64(1000), *m.MinTimestamp)
+ assert.Equal(t, int64(2000), *m.MaxTimestamp)
+ })
+}
+
+// TestQuery_TimestampSelection verifies timestamp-aware part selection and
key-range fallback.
+func TestQuery_TimestampSelection(t *testing.T) {
+ reqs := []WriteRequest{
+ createTestWriteRequest(1, 1000, "data1000"),
+ createTestWriteRequest(1, 2000, "data2000"),
+ }
+
+ t.Run("back_compat_key_range_only", func(t *testing.T) {
+ sidx := createTestSIDX(t)
+ defer func() {
+ assert.NoError(t, sidx.Close())
+ }()
+ writeTestDataWithTimeRange(t, sidx, reqs, 1, 1, nil, nil)
+ waitForIntroducerLoop()
+
+ minKey := int64(500)
+ maxKey := int64(2500)
+ qr := QueryRequest{
+ SeriesIDs: []common.SeriesID{1},
+ MinKey: &minKey,
+ MaxKey: &maxKey,
+ }
+ resultsCh, errCh := sidx.StreamingQuery(context.Background(),
qr)
+ var keys []int64
+ for res := range resultsCh {
+ require.NoError(t, res.Error)
+ keys = append(keys, res.Keys...)
+ }
+ select {
+ case err, ok := <-errCh:
+ if ok {
+ require.NoError(t, err)
+ }
+ default:
+ }
+ assert.GreaterOrEqual(t, len(keys), 0, "key-range filter should
work without timestamps")
+ })
+
+ t.Run("timestamp_filter_overlaps", func(t *testing.T) {
+ sidx := createTestSIDX(t)
+ defer func() {
+ assert.NoError(t, sidx.Close())
+ }()
+ minTS := ptrInt64Ts(1000)
+ maxTS := ptrInt64Ts(3000)
+ writeTestDataWithTimeRange(t, sidx, reqs, 1, 1, minTS, maxTS)
+ waitForIntroducerLoop()
+
+ qMinTS := ptrInt64Ts(1500)
+ qMaxTS := ptrInt64Ts(2500)
+ qr := QueryRequest{
+ SeriesIDs: []common.SeriesID{1},
+ MinTimestamp: qMinTS,
+ MaxTimestamp: qMaxTS,
+ }
+ resultsCh, errCh := sidx.StreamingQuery(context.Background(),
qr)
+ var count int
+ for res := range resultsCh {
+ require.NoError(t, res.Error)
+ count += res.Len()
+ }
+ select {
+ case err, ok := <-errCh:
+ if ok {
+ require.NoError(t, err)
+ }
+ default:
+ }
+ assert.GreaterOrEqual(t, count, 0, "query with overlapping
timestamp range")
Review Comment:
These assertions don’t validate behavior: `assert.GreaterOrEqual(t, count,
0)` (and similar) will always pass. To ensure the timestamp-based selection
works, assert on concrete expected results (e.g., `count > 0` for the overlap
case, and specific key/data values where possible).
```suggestion
assert.Greater(t, count, 0, "query with overlapping timestamp
range")
```
--
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]
