Copilot commented on code in PR #13668:
URL: https://github.com/apache/skywalking/pull/13668#discussion_r2693343019
##########
oap-server/server-library/library-banyandb-client/src/main/java/org/apache/skywalking/library/banyandb/v1/client/PropertyQuery.java:
##########
@@ -95,7 +98,32 @@ public BanyandbProperty.QueryRequest build() throws
BanyanDBException {
if (!this.ids.isEmpty()) {
builder.addAllIds(this.ids);
}
-
+ if (this.orderBy != null) {
+ builder.setOrderBy(orderBy.build());
+ }
return builder.build();
}
+
+ public static class OrderBy {
+ private final String tagName;
+ private final Sort type;
+
+ /**
+ * Create an orderBy condition with given tag name and sort type
+ */
+ public OrderBy(final String tagName, final Sort type) {
Review Comment:
The `Sort` type reference is unqualified. Use `AbstractQuery.Sort` instead
of `Sort` to properly reference the enum from the parent class.
##########
oap-server/server-library/library-banyandb-client/src/main/java/org/apache/skywalking/library/banyandb/v1/client/PropertyQuery.java:
##########
@@ -95,7 +98,32 @@ public BanyandbProperty.QueryRequest build() throws
BanyanDBException {
if (!this.ids.isEmpty()) {
builder.addAllIds(this.ids);
}
-
+ if (this.orderBy != null) {
+ builder.setOrderBy(orderBy.build());
+ }
return builder.build();
}
+
+ public static class OrderBy {
+ private final String tagName;
+ private final Sort type;
Review Comment:
The `Sort` enum is not imported or fully qualified. Since `Sort` is defined
as a nested enum in `AbstractQuery`, it should be referenced as
`AbstractQuery.Sort` or imported statically. This will cause a compilation
error.
##########
oap-server/server-library/library-banyandb-client/src/main/java/org/apache/skywalking/library/banyandb/v1/client/PropertyQuery.java:
##########
@@ -95,7 +98,32 @@ public BanyandbProperty.QueryRequest build() throws
BanyanDBException {
if (!this.ids.isEmpty()) {
builder.addAllIds(this.ids);
}
-
+ if (this.orderBy != null) {
+ builder.setOrderBy(orderBy.build());
+ }
return builder.build();
}
+
+ public static class OrderBy {
+ private final String tagName;
+ private final Sort type;
+
+ /**
+ * Create an orderBy condition with given tag name and sort type
+ */
+ public OrderBy(final String tagName, final Sort type) {
+ this.tagName = tagName;
+ this.type = type;
+ }
+
+ BanyandbProperty.QueryOrder build() {
+ final BanyandbProperty.QueryOrder.Builder builder =
BanyandbProperty.QueryOrder.newBuilder();
+ if (tagName != null) {
+ builder.setTagName(tagName);
+ }
+ builder.setSort(
+ Sort.DESC.equals(type) ? BanyandbModel.Sort.SORT_DESC :
BanyandbModel.Sort.SORT_ASC);
Review Comment:
The `Sort.DESC` reference is unqualified. Use `AbstractQuery.Sort.DESC`
instead to properly reference the enum from the parent class.
##########
oap-server/server-library/library-banyandb-client/src/main/java/org/apache/skywalking/library/banyandb/v1/client/PropertyQuery.java:
##########
@@ -95,7 +98,32 @@ public BanyandbProperty.QueryRequest build() throws
BanyanDBException {
if (!this.ids.isEmpty()) {
builder.addAllIds(this.ids);
}
-
+ if (this.orderBy != null) {
+ builder.setOrderBy(orderBy.build());
+ }
return builder.build();
}
+
+ public static class OrderBy {
+ private final String tagName;
+ private final Sort type;
+
+ /**
+ * Create an orderBy condition with given tag name and sort type
+ */
+ public OrderBy(final String tagName, final Sort type) {
+ this.tagName = tagName;
+ this.type = type;
+ }
+
+ BanyandbProperty.QueryOrder build() {
+ final BanyandbProperty.QueryOrder.Builder builder =
BanyandbProperty.QueryOrder.newBuilder();
+ if (tagName != null) {
+ builder.setTagName(tagName);
+ }
+ builder.setSort(
+ Sort.DESC.equals(type) ? BanyandbModel.Sort.SORT_DESC :
BanyandbModel.Sort.SORT_ASC);
+ return builder.build();
+ }
+ }
Review Comment:
The new OrderBy functionality lacks test coverage. Consider adding tests to
ITBanyanDBPropertyTests to verify that property queries can be sorted correctly
by tag name in both ascending and descending order.
##########
oap-server/server-library/library-banyandb-client/src/main/java/org/apache/skywalking/library/banyandb/v1/client/PropertyQuery.java:
##########
@@ -95,7 +98,32 @@ public BanyandbProperty.QueryRequest build() throws
BanyanDBException {
if (!this.ids.isEmpty()) {
builder.addAllIds(this.ids);
}
-
+ if (this.orderBy != null) {
+ builder.setOrderBy(orderBy.build());
+ }
return builder.build();
}
+
+ public static class OrderBy {
+ private final String tagName;
+ private final Sort type;
+
+ /**
+ * Create an orderBy condition with given tag name and sort type
+ */
+ public OrderBy(final String tagName, final Sort type) {
+ this.tagName = tagName;
+ this.type = type;
+ }
+
+ BanyandbProperty.QueryOrder build() {
Review Comment:
The build() method should be package-private to match the visibility pattern
used in AbstractQuery.OrderBy.build(). Both are internal builder methods not
intended for public API use.
--
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]
