Copilot commented on code in PR #778: URL: https://github.com/apache/skywalking-banyandb/pull/778#discussion_r2366201901
########## api/proto/banyandb/bydbql/v1/query.proto: ########## @@ -0,0 +1,232 @@ +// 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. + +syntax = "proto3"; + +package banyandb.bydbql.v1; + +import "banyandb/common/v1/common.proto"; +import "banyandb/common/v1/trace.proto"; +import "banyandb/measure/v1/query.proto"; +import "banyandb/measure/v1/topn.proto"; +import "banyandb/property/v1/rpc.proto"; +import "banyandb/stream/v1/query.proto"; +import "banyandb/trace/v1/query.proto"; +import "validate/validate.proto"; + +option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/bydbql/v1"; +option java_package = "org.apache.skywalking.banyandb.bydbql.v1"; + +// QueryExecutionContext provides resource context when FROM clause is omitted +message QueryExecutionContext { + // catalog indicates the type of resource (STREAM, MEASURE, PROPERTY, TRACE) + common.v1.Catalog catalog = 1 [(validate.rules).enum.defined_only = true]; + // name is the identity of the resource + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + string group = 3; +} + +// QueryRequest is the main request message for BydbQL queries +message QueryRequest { + // query is the BydbQL query string + string query = 1 [(validate.rules).string.min_len = 1]; + // execution_context provides resource context when FROM clause is omitted + // This field is optional when the FROM clause is present in the query + QueryExecutionContext execution_context = 2; +} + +// QueryResponse contains the result of a BydbQL query +message QueryResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryStreamRequest is used for stream-specific queries +message QueryStreamRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryStreamResponse is the response for stream-specific queries +message QueryStreamResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryMeasureRequest is used for measure-specific queries +message QueryMeasureRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryMeasureResponse is the response for measure-specific queries +message QueryMeasureResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; Review Comment: QueryMeasureResponse includes all result types including stream_result, property_result, and trace_result. For a measure-specific endpoint, it should only return measure_result and topn_result to maintain API clarity and prevent confusion. ```suggestion // measure_result is returned for measure queries measure.v1.QueryResponse measure_result = 2; ``` ########## api/proto/banyandb/bydbql/v1/query.proto: ########## @@ -0,0 +1,232 @@ +// 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. + +syntax = "proto3"; + +package banyandb.bydbql.v1; + +import "banyandb/common/v1/common.proto"; +import "banyandb/common/v1/trace.proto"; +import "banyandb/measure/v1/query.proto"; +import "banyandb/measure/v1/topn.proto"; +import "banyandb/property/v1/rpc.proto"; +import "banyandb/stream/v1/query.proto"; +import "banyandb/trace/v1/query.proto"; +import "validate/validate.proto"; + +option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/bydbql/v1"; +option java_package = "org.apache.skywalking.banyandb.bydbql.v1"; + +// QueryExecutionContext provides resource context when FROM clause is omitted +message QueryExecutionContext { + // catalog indicates the type of resource (STREAM, MEASURE, PROPERTY, TRACE) + common.v1.Catalog catalog = 1 [(validate.rules).enum.defined_only = true]; + // name is the identity of the resource + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + string group = 3; +} + +// QueryRequest is the main request message for BydbQL queries +message QueryRequest { + // query is the BydbQL query string + string query = 1 [(validate.rules).string.min_len = 1]; + // execution_context provides resource context when FROM clause is omitted + // This field is optional when the FROM clause is present in the query + QueryExecutionContext execution_context = 2; +} + +// QueryResponse contains the result of a BydbQL query +message QueryResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryStreamRequest is used for stream-specific queries +message QueryStreamRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryStreamResponse is the response for stream-specific queries +message QueryStreamResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryMeasureRequest is used for measure-specific queries +message QueryMeasureRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryMeasureResponse is the response for measure-specific queries +message QueryMeasureResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryPropertyRequest is used for property-specific queries +message QueryPropertyRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryPropertyResponse is the response for property-specific queries +message QueryPropertyResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; Review Comment: QueryPropertyResponse includes all result types including stream_result, measure_result, and trace_result. For a property-specific endpoint, it should only return property_result to maintain API clarity and prevent confusion. ```suggestion // result contains the actual query result for property queries oneof result { // property_result is returned for property queries property.v1.QueryResponse property_result = 1; } // trace contains the trace information of the query when trace is enabled common.v1.Trace trace = 2; // error contains error information if the query failed string error = 3; ``` ########## api/proto/banyandb/bydbql/v1/query.proto: ########## @@ -0,0 +1,232 @@ +// 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. + +syntax = "proto3"; + +package banyandb.bydbql.v1; + +import "banyandb/common/v1/common.proto"; +import "banyandb/common/v1/trace.proto"; +import "banyandb/measure/v1/query.proto"; +import "banyandb/measure/v1/topn.proto"; +import "banyandb/property/v1/rpc.proto"; +import "banyandb/stream/v1/query.proto"; +import "banyandb/trace/v1/query.proto"; +import "validate/validate.proto"; + +option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/bydbql/v1"; +option java_package = "org.apache.skywalking.banyandb.bydbql.v1"; + +// QueryExecutionContext provides resource context when FROM clause is omitted +message QueryExecutionContext { + // catalog indicates the type of resource (STREAM, MEASURE, PROPERTY, TRACE) + common.v1.Catalog catalog = 1 [(validate.rules).enum.defined_only = true]; + // name is the identity of the resource + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + string group = 3; +} + +// QueryRequest is the main request message for BydbQL queries +message QueryRequest { + // query is the BydbQL query string + string query = 1 [(validate.rules).string.min_len = 1]; + // execution_context provides resource context when FROM clause is omitted + // This field is optional when the FROM clause is present in the query + QueryExecutionContext execution_context = 2; +} + +// QueryResponse contains the result of a BydbQL query +message QueryResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryStreamRequest is used for stream-specific queries +message QueryStreamRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryStreamResponse is the response for stream-specific queries +message QueryStreamResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } Review Comment: The same oneof result structure is duplicated across multiple response messages (QueryStreamResponse, QueryMeasureResponse, QueryPropertyResponse, QueryTraceResponse, TopNResponse). Consider extracting this into a common message type to reduce duplication and improve maintainability. ########## api/proto/banyandb/bydbql/v1/query.proto: ########## @@ -0,0 +1,232 @@ +// 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. + +syntax = "proto3"; + +package banyandb.bydbql.v1; + +import "banyandb/common/v1/common.proto"; +import "banyandb/common/v1/trace.proto"; +import "banyandb/measure/v1/query.proto"; +import "banyandb/measure/v1/topn.proto"; +import "banyandb/property/v1/rpc.proto"; +import "banyandb/stream/v1/query.proto"; +import "banyandb/trace/v1/query.proto"; +import "validate/validate.proto"; + +option go_package = "github.com/apache/skywalking-banyandb/api/proto/banyandb/bydbql/v1"; +option java_package = "org.apache.skywalking.banyandb.bydbql.v1"; + +// QueryExecutionContext provides resource context when FROM clause is omitted +message QueryExecutionContext { + // catalog indicates the type of resource (STREAM, MEASURE, PROPERTY, TRACE) + common.v1.Catalog catalog = 1 [(validate.rules).enum.defined_only = true]; + // name is the identity of the resource + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + string group = 3; +} + +// QueryRequest is the main request message for BydbQL queries +message QueryRequest { + // query is the BydbQL query string + string query = 1 [(validate.rules).string.min_len = 1]; + // execution_context provides resource context when FROM clause is omitted + // This field is optional when the FROM clause is present in the query + QueryExecutionContext execution_context = 2; +} + +// QueryResponse contains the result of a BydbQL query +message QueryResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryStreamRequest is used for stream-specific queries +message QueryStreamRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryStreamResponse is the response for stream-specific queries +message QueryStreamResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryMeasureRequest is used for measure-specific queries +message QueryMeasureRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryMeasureResponse is the response for measure-specific queries +message QueryMeasureResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryPropertyRequest is used for property-specific queries +message QueryPropertyRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryPropertyResponse is the response for property-specific queries +message QueryPropertyResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// QueryTraceRequest is used for trace-specific queries +message QueryTraceRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// QueryTraceResponse is the response for trace-specific queries +message QueryTraceResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; + } + // trace contains the trace information of the query when trace is enabled + common.v1.Trace trace = 6; + // error contains error information if the query failed + string error = 7; +} + +// TopNRequest is used for TopN-specific queries +message TopNRequest { + // query is the BydbQL query string (typically without FROM clause) + string query = 1 [(validate.rules).string.min_len = 1]; + // name is the resource name extracted from the URL path + string name = 2 [(validate.rules).string.min_len = 1]; + // group indicates where the data point is stored + // This field is extracted from the URL path and is optional + string group = 3; +} + +// TopNResponse is the response for TopN-specific queries +message TopNResponse { + // result contains the actual query result based on the query type + oneof result { + // stream_result is returned for stream queries + stream.v1.QueryResponse stream_result = 1; + // measure_result is returned for measure queries + measure.v1.QueryResponse measure_result = 2; + // property_result is returned for property queries + property.v1.QueryResponse property_result = 3; + // trace_result is returned for trace queries + trace.v1.QueryResponse trace_result = 4; + // topn_result is returned for TopN queries + measure.v1.TopNResponse topn_result = 5; Review Comment: TopNResponse includes all result types but should only return topn_result since it's specifically for TopN queries against measure resources. Including other result types creates API ambiguity. ```suggestion // result contains the actual query result for TopN queries oneof result { // topn_result is returned for TopN queries measure.v1.TopNResponse topn_result = 1; ``` -- 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-unsubscr...@skywalking.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
