This is an automated email from the ASF dual-hosted git repository. hanahmily pushed a commit to branch sidx/query in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
commit 578401a80d838f8c100fa915c5fd2fd7f22f6859 Author: Gao Hongtao <[email protected]> AuthorDate: Wed Aug 27 18:54:08 2025 +0800 Refactor QueryRequest usage: Remove Name field from QueryRequest across various components and tests, replacing it with SeriesIDs for improved query validation and consistency. Update related tests to ensure proper error handling and functionality, enhancing overall clarity and robustness. --- banyand/internal/sidx/integration_test_framework.go | 4 ---- .../internal/sidx/integration_test_framework_test.go | 1 - banyand/internal/sidx/interfaces.go | 7 ------- banyand/internal/sidx/interfaces_examples.go | 13 ++++++------- banyand/internal/sidx/interfaces_examples_test.go | 20 +++++++++++--------- banyand/internal/sidx/mock_components_test.go | 8 ++------ banyand/internal/sidx/mock_sidx.go | 10 +++++++--- banyand/internal/sidx/mock_sidx_test.go | 14 ++++++-------- banyand/internal/sidx/mock_usage_test.go | 6 ------ banyand/internal/sidx/snapshot_test.go | 2 +- 10 files changed, 33 insertions(+), 52 deletions(-) diff --git a/banyand/internal/sidx/integration_test_framework.go b/banyand/internal/sidx/integration_test_framework.go index 18fd40d9..fa832fc5 100644 --- a/banyand/internal/sidx/integration_test_framework.go +++ b/banyand/internal/sidx/integration_test_framework.go @@ -519,7 +519,6 @@ func (itf *IntegrationTestFramework) registerDefaultScenarios() { // Query the data back using SeriesIDs that were written queryReq := QueryRequest{ - Name: "series_1", // Match the key used in MockSIDX write SeriesIDs: []common.SeriesID{common.SeriesID(1), common.SeriesID(2), common.SeriesID(3)}, MaxElementSize: 100, } @@ -635,7 +634,6 @@ func (itf *IntegrationTestFramework) registerDefaultScenarios() { seriesIDs[i-1] = common.SeriesID(i) } queryReq := QueryRequest{ - Name: "large-dataset-query", SeriesIDs: seriesIDs, MaxElementSize: 50, } @@ -752,7 +750,6 @@ func (itf *IntegrationTestFramework) registerDefaultBenchmarks() { for i := 0; i < numQueries; i++ { seriesID := common.SeriesID((i % 10) + 1) // Query existing series queryReq := QueryRequest{ - Name: fmt.Sprintf("series_%d", seriesID), SeriesIDs: []common.SeriesID{seriesID}, MaxElementSize: 50, } @@ -867,7 +864,6 @@ func (itf *IntegrationTestFramework) registerDefaultStressTests() { // Read operation - query one of the series we know exists (from setup) seriesID := common.SeriesID((workerID % 5) + 1) // Query from the 5 series in setup queryReq := QueryRequest{ - Name: fmt.Sprintf("stress-query-%d", workerID), SeriesIDs: []common.SeriesID{seriesID}, MaxElementSize: 20, } diff --git a/banyand/internal/sidx/integration_test_framework_test.go b/banyand/internal/sidx/integration_test_framework_test.go index 365cfe06..a4b6fd96 100644 --- a/banyand/internal/sidx/integration_test_framework_test.go +++ b/banyand/internal/sidx/integration_test_framework_test.go @@ -487,7 +487,6 @@ func BenchmarkIntegrationTestFramework_QueryPerformance(b *testing.B) { for i := 0; i < b.N; i++ { seriesID := common.SeriesID((i % 5) + 1) // Query existing series queryReq := QueryRequest{ - Name: fmt.Sprintf("bench-query-%d", i), SeriesIDs: []common.SeriesID{seriesID}, MaxElementSize: 50, } diff --git a/banyand/internal/sidx/interfaces.go b/banyand/internal/sidx/interfaces.go index 3863e710..d6116e52 100644 --- a/banyand/internal/sidx/interfaces.go +++ b/banyand/internal/sidx/interfaces.go @@ -127,7 +127,6 @@ type QueryRequest struct { Order *index.OrderBy MinKey *int64 MaxKey *int64 - Name string SeriesIDs []common.SeriesID TagProjection []model.TagProjection MaxElementSize int @@ -339,9 +338,6 @@ func (wr WriteRequest) Validate() error { // Validate validates a QueryRequest for correctness. func (qr QueryRequest) Validate() error { - if qr.Name == "" { - return fmt.Errorf("name cannot be empty") - } if len(qr.SeriesIDs) == 0 { return fmt.Errorf("at least one SeriesID is required") } @@ -363,7 +359,6 @@ func (qr QueryRequest) Validate() error { // Reset resets the QueryRequest to its zero state. func (qr *QueryRequest) Reset() { - qr.Name = "" qr.SeriesIDs = nil qr.Filter = nil qr.Order = nil @@ -375,8 +370,6 @@ func (qr *QueryRequest) Reset() { // CopyFrom copies the QueryRequest from other to qr. func (qr *QueryRequest) CopyFrom(other *QueryRequest) { - qr.Name = other.Name - // Deep copy for SeriesIDs if it's a slice if other.SeriesIDs != nil { qr.SeriesIDs = make([]common.SeriesID, len(other.SeriesIDs)) diff --git a/banyand/internal/sidx/interfaces_examples.go b/banyand/internal/sidx/interfaces_examples.go index bfb03ff3..14c99589 100644 --- a/banyand/internal/sidx/interfaces_examples.go +++ b/banyand/internal/sidx/interfaces_examples.go @@ -106,7 +106,6 @@ func (e *InterfaceUsageExamples) BasicWriteExample(ctx context.Context) error { func (e *InterfaceUsageExamples) AdvancedQueryExample(ctx context.Context) error { // Create query request with range and tag filtering queryReq := QueryRequest{ - Name: "trace-sidx", SeriesIDs: []common.SeriesID{1, 2, 3}, // Example series IDs Filter: nil, // In production, use actual index.Filter implementation Order: nil, // In production, use actual index.OrderBy implementation @@ -228,8 +227,8 @@ func (e *InterfaceUsageExamples) ErrorHandlingExample(ctx context.Context) { // Example 2: Query error handling invalidQuery := QueryRequest{ - Name: "", // Invalid name - MaxElementSize: -1, // Invalid size + SeriesIDs: nil, // Invalid - empty SeriesIDs + MaxElementSize: -1, // Invalid size } result, err := e.sidx.Query(ctx, invalidQuery) @@ -293,8 +292,8 @@ func (e *InterfaceUsageExamples) PerformanceOptimizationExample(ctx context.Cont // Best Practice 3: Use appropriate query limits queryReq := QueryRequest{ - Name: "performance-test", - MaxElementSize: 100, // Reasonable batch size + SeriesIDs: []common.SeriesID{1, 2, 3}, // Example series + MaxElementSize: 100, // Reasonable batch size TagProjection: []model.TagProjection{ {Family: "series", Names: []string{"id"}}, // Only project needed tags }, @@ -401,8 +400,8 @@ func (m *mockSIDX) Write(_ context.Context, reqs []WriteRequest) error { } func (m *mockSIDX) Query(_ context.Context, req QueryRequest) (QueryResult, error) { - if req.Name == "" { - return nil, fmt.Errorf("query name cannot be empty") + if len(req.SeriesIDs) == 0 { + return nil, fmt.Errorf("at least one SeriesID is required") } if req.MaxElementSize < 0 { return nil, fmt.Errorf("invalid MaxElementSize: %d", req.MaxElementSize) diff --git a/banyand/internal/sidx/interfaces_examples_test.go b/banyand/internal/sidx/interfaces_examples_test.go index 5b9573bb..5e689ad1 100644 --- a/banyand/internal/sidx/interfaces_examples_test.go +++ b/banyand/internal/sidx/interfaces_examples_test.go @@ -24,6 +24,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/apache/skywalking-banyandb/api/common" ) func TestInterfaceUsageExamples(t *testing.T) { @@ -89,15 +91,15 @@ func TestMockSIDXImplementation(t *testing.T) { }) t.Run("Query validates input", func(t *testing.T) { - // Test empty query name - result, err := sidx.Query(ctx, QueryRequest{Name: ""}) + // Test empty SeriesIDs + result, err := sidx.Query(ctx, QueryRequest{SeriesIDs: nil}) assert.Error(t, err) assert.Nil(t, result) - assert.Contains(t, err.Error(), "query name cannot be empty") + assert.Contains(t, err.Error(), "at least one SeriesID is required") // Test invalid MaxElementSize result, err = sidx.Query(ctx, QueryRequest{ - Name: "test", + SeriesIDs: []common.SeriesID{1}, MaxElementSize: -1, }) assert.Error(t, err) @@ -106,7 +108,7 @@ func TestMockSIDXImplementation(t *testing.T) { // Test valid query result, err = sidx.Query(ctx, QueryRequest{ - Name: "test", + SeriesIDs: []common.SeriesID{1}, MaxElementSize: 100, }) assert.NoError(t, err) @@ -246,11 +248,11 @@ func TestContractCompliance(t *testing.T) { t.Run("Query contract compliance", func(t *testing.T) { // Contract: MUST validate QueryRequest parameters - _, err := sidx.Query(ctx, QueryRequest{Name: ""}) - assert.Error(t, err, "Should validate query name") + _, err := sidx.Query(ctx, QueryRequest{SeriesIDs: nil}) + assert.Error(t, err, "Should validate SeriesIDs") // Contract: MUST return QueryResult with Pull/Release pattern - result, err := sidx.Query(ctx, QueryRequest{Name: "test"}) + result, err := sidx.Query(ctx, QueryRequest{SeriesIDs: []common.SeriesID{1}}) assert.NoError(t, err) assert.NotNil(t, result, "Should return QueryResult") @@ -332,7 +334,7 @@ func BenchmarkMockQueryOperations(b *testing.B) { ctx := context.Background() req := QueryRequest{ - Name: "benchmark", + SeriesIDs: []common.SeriesID{1}, MaxElementSize: 100, } diff --git a/banyand/internal/sidx/mock_components_test.go b/banyand/internal/sidx/mock_components_test.go index 04e3d68f..92f9dc56 100644 --- a/banyand/internal/sidx/mock_components_test.go +++ b/banyand/internal/sidx/mock_components_test.go @@ -219,7 +219,6 @@ func TestMockQuerier_BasicOperations(t *testing.T) { // Test query operations queryReq := QueryRequest{ - Name: "test-query", SeriesIDs: []common.SeriesID{1, 2}, MaxElementSize: 10, } @@ -264,7 +263,6 @@ func TestMockQuerier_QueryBatching(t *testing.T) { // Query with small batch size queryReq := QueryRequest{ - Name: "batch-test", SeriesIDs: []common.SeriesID{1}, MaxElementSize: 3, } @@ -297,7 +295,7 @@ func TestMockQuerier_ErrorInjection(t *testing.T) { querier := NewMockQuerier(config) ctx := context.Background() - queryReq := QueryRequest{Name: "test"} + queryReq := QueryRequest{SeriesIDs: []common.SeriesID{1}} _, err := querier.Query(ctx, queryReq) assert.Error(t, err) assert.Contains(t, err.Error(), "mock querier error injection") @@ -426,7 +424,6 @@ func TestMockComponentSuite_Integration(t *testing.T) { // Query elements queryReq := QueryRequest{ - Name: "integration-test", SeriesIDs: []common.SeriesID{1, 2}, MaxElementSize: 10, } @@ -462,7 +459,7 @@ func TestMockComponentSuite_ErrorInjection(t *testing.T) { err := suite.Writer.Write(ctx, writeReqs) assert.Error(t, err) - queryReq := QueryRequest{Name: "test"} + queryReq := QueryRequest{SeriesIDs: []common.SeriesID{1}} _, err = suite.Querier.Query(ctx, queryReq) assert.Error(t, err) @@ -631,7 +628,6 @@ func TestMockComponents_PerformanceCharacteristics(t *testing.T) { suite.SyncElements() queryReq := QueryRequest{ - Name: "performance-test", SeriesIDs: []common.SeriesID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, MaxElementSize: 100, } diff --git a/banyand/internal/sidx/mock_sidx.go b/banyand/internal/sidx/mock_sidx.go index 51b39df8..8628a413 100644 --- a/banyand/internal/sidx/mock_sidx.go +++ b/banyand/internal/sidx/mock_sidx.go @@ -209,9 +209,13 @@ func (m *MockSIDX) Query(_ context.Context, req QueryRequest) (QueryResult, erro m.mu.RLock() defer m.mu.RUnlock() - elements, exists := m.storage[req.Name] - if !exists { - elements = []mockElement{} + // Collect elements from all requested SeriesIDs + var elements []mockElement + for _, seriesID := range req.SeriesIDs { + name := fmt.Sprintf("series_%d", seriesID) + if seriesElements, exists := m.storage[name]; exists { + elements = append(elements, seriesElements...) + } } // Update query stats diff --git a/banyand/internal/sidx/mock_sidx_test.go b/banyand/internal/sidx/mock_sidx_test.go index a57f01b1..50370db8 100644 --- a/banyand/internal/sidx/mock_sidx_test.go +++ b/banyand/internal/sidx/mock_sidx_test.go @@ -96,7 +96,6 @@ func TestMockSIDX_BasicOperations(t *testing.T) { // Test query operations queryReq := QueryRequest{ - Name: "series_1", SeriesIDs: []common.SeriesID{1}, MaxElementSize: 10, } @@ -255,7 +254,7 @@ func TestMockSIDX_ErrorInjection(t *testing.T) { assert.Contains(t, err.Error(), "mock error injection") // Test query error injection - queryReq := QueryRequest{Name: "test"} + queryReq := QueryRequest{SeriesIDs: []common.SeriesID{1}} _, err = mock.Query(ctx, queryReq) assert.Error(t, err) assert.Contains(t, err.Error(), "mock error injection") @@ -296,7 +295,7 @@ func TestMockSIDX_Delays(t *testing.T) { // Test query delay start = time.Now() - queryReq := QueryRequest{Name: "series_1", SeriesIDs: []common.SeriesID{1}} + queryReq := QueryRequest{SeriesIDs: []common.SeriesID{1}} result, err := mock.Query(ctx, queryReq) require.NoError(t, err) result.Release() @@ -338,7 +337,7 @@ func TestMockSIDX_Sorting(t *testing.T) { require.NoError(t, err) // Query series 1 - should be sorted by key - queryReq := QueryRequest{Name: "series_1", SeriesIDs: []common.SeriesID{1}} + queryReq := QueryRequest{SeriesIDs: []common.SeriesID{1}} result, err := mock.Query(ctx, queryReq) require.NoError(t, err) defer result.Release() @@ -349,7 +348,7 @@ func TestMockSIDX_Sorting(t *testing.T) { assert.Equal(t, [][]byte{[]byte("data1"), []byte("data3")}, response.Data) // Query series 2 - should be sorted by key - queryReq = QueryRequest{Name: "series_2", SeriesIDs: []common.SeriesID{2}} + queryReq = QueryRequest{SeriesIDs: []common.SeriesID{2}} result, err = mock.Query(ctx, queryReq) require.NoError(t, err) defer result.Release() @@ -383,7 +382,6 @@ func TestMockSIDX_QueryBatching(t *testing.T) { // Query with small batch size queryReq := QueryRequest{ - Name: "series_1", SeriesIDs: []common.SeriesID{1}, MaxElementSize: 3, } @@ -475,7 +473,7 @@ func TestMockSIDX_CloseOperations(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "SIDX is closed") - queryReq := QueryRequest{Name: "test"} + queryReq := QueryRequest{SeriesIDs: []common.SeriesID{1}} _, err = mock.Query(ctx, queryReq) assert.Error(t, err) assert.Contains(t, err.Error(), "SIDX is closed") @@ -529,7 +527,7 @@ func TestMockSIDX_DynamicConfiguration(t *testing.T) { mock.SetQueryDelay(30) start = time.Now() - result, err := mock.Query(ctx, QueryRequest{Name: "series_1", SeriesIDs: []common.SeriesID{1}}) + result, err := mock.Query(ctx, QueryRequest{SeriesIDs: []common.SeriesID{1}}) require.NoError(t, err) result.Release() elapsed = time.Since(start) diff --git a/banyand/internal/sidx/mock_usage_test.go b/banyand/internal/sidx/mock_usage_test.go index 5a823521..688aaf6f 100644 --- a/banyand/internal/sidx/mock_usage_test.go +++ b/banyand/internal/sidx/mock_usage_test.go @@ -67,7 +67,6 @@ func TestDocumentation_BasicWriteReadWorkflow(t *testing.T) { // Query the data back (from documentation example) queryReq := QueryRequest{ - Name: "series_1", // Matches the series name used in mock SeriesIDs: []common.SeriesID{common.SeriesID(1)}, MaxElementSize: 100, } @@ -116,7 +115,6 @@ func TestDocumentation_ComponentIntegration(t *testing.T) { // Query data (from documentation example) queryReq := QueryRequest{ - Name: "test-query", SeriesIDs: []common.SeriesID{common.SeriesID(1)}, MaxElementSize: 10, } @@ -353,7 +351,6 @@ func TestDocumentation_TroubleshootingExamples(t *testing.T) { // Without syncing, query should return no results queryReq := QueryRequest{ - Name: "test-query", SeriesIDs: []common.SeriesID{common.SeriesID(1)}, MaxElementSize: 100, } @@ -496,7 +493,6 @@ func TestDocumentation_DebugTips(t *testing.T) { // Query to increment query count queryReq := QueryRequest{ - Name: "series_1", SeriesIDs: []common.SeriesID{common.SeriesID(1)}, MaxElementSize: 10, } @@ -531,7 +527,6 @@ func TestDocumentation_Migration(t *testing.T) { require.NoError(t, err) queryReq := QueryRequest{ - Name: "series_1", SeriesIDs: []common.SeriesID{common.SeriesID(1)}, MaxElementSize: 10, } @@ -576,7 +571,6 @@ func TestDocumentation_PerformanceCharacteristics(t *testing.T) { // Test query performance (from documentation) queryReq := QueryRequest{ - Name: "series_1", SeriesIDs: []common.SeriesID{common.SeriesID(1)}, MaxElementSize: 50, } diff --git a/banyand/internal/sidx/snapshot_test.go b/banyand/internal/sidx/snapshot_test.go index 0011ad8d..800eff74 100644 --- a/banyand/internal/sidx/snapshot_test.go +++ b/banyand/internal/sidx/snapshot_test.go @@ -613,7 +613,7 @@ func TestSnapshotReplacement_NoDataRacesDuringReplacement(t *testing.T) { case 2: // Query operation - accesses current snapshot queryReq := QueryRequest{ - Name: "test-index", + SeriesIDs: []common.SeriesID{1}, } result, err := sidx.Query(ctx, queryReq) if err == nil && result != nil {
