Copilot commented on code in PR #16764:
URL: https://github.com/apache/pinot/pull/16764#discussion_r2325172585
##########
pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkOrderByQueries.java:
##########
@@ -194,42 +196,36 @@ private void buildSegment(String segmentName)
}
@Benchmark
- public BrokerResponseNative sortedAsc() {
+ public BrokerResponseNative sortedTotally() {
+ if (_zasc) {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
+ + "FROM sorted "
Review Comment:
The table name has been changed from "MyTable" to "sorted", but this appears
to be inconsistent with the schema definition which likely expects "MyTable".
This could cause query execution failures in the benchmark.
##########
pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkOrderByQueries.java:
##########
@@ -194,42 +196,36 @@ private void buildSegment(String segmentName)
}
@Benchmark
- public BrokerResponseNative sortedAsc() {
+ public BrokerResponseNative sortedTotally() {
+ if (_zasc) {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
+ + "FROM sorted "
+ "ORDER BY SORTED_COL ASC "
- + "LIMIT 1052 "
- + "option(orderByAlgorithm=" + _orderByAlgorithm + ")");
- }
- @Benchmark
- public BrokerResponseNative sortedAscPartially() {
+ + "LIMIT " + _limit);
+ } else {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
- + "ORDER BY SORTED_COL ASC, LOW_CARDINALITY_STRING_COL "
- + "LIMIT 1052 "
- + "option(orderByAlgorithm=" + _orderByAlgorithm + ")");
+ + "FROM sorted "
Review Comment:
The table name has been changed from "MyTable" to "sorted", but this appears
to be inconsistent with the schema definition which likely expects "MyTable".
This could cause query execution failures in the benchmark.
##########
pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkOrderByQueries.java:
##########
@@ -194,42 +196,36 @@ private void buildSegment(String segmentName)
}
@Benchmark
- public BrokerResponseNative sortedAsc() {
+ public BrokerResponseNative sortedTotally() {
+ if (_zasc) {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
+ + "FROM sorted "
+ "ORDER BY SORTED_COL ASC "
- + "LIMIT 1052 "
- + "option(orderByAlgorithm=" + _orderByAlgorithm + ")");
- }
- @Benchmark
- public BrokerResponseNative sortedAscPartially() {
+ + "LIMIT " + _limit);
+ } else {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
- + "ORDER BY SORTED_COL ASC, LOW_CARDINALITY_STRING_COL "
- + "LIMIT 1052 "
- + "option(orderByAlgorithm=" + _orderByAlgorithm + ")");
+ + "FROM sorted "
+ + "ORDER BY SORTED_COL DESC "
+ + "LIMIT " + _limit);
+ }
}
-
@Benchmark
- public BrokerResponseNative sortedDesc() {
+ public BrokerResponseNative sortedPartially() {
+ if (_zasc) {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
- + "ORDER BY SORTED_COL DESC "
- + "LIMIT 1052 "
- + "option(orderByAlgorithm=" + _orderByAlgorithm + ")");
- }
-
- @Benchmark
- public BrokerResponseNative sortedDescPartially() {
+ + "FROM sorted "
Review Comment:
The table name has been changed from "MyTable" to "sorted", but this appears
to be inconsistent with the schema definition which likely expects "MyTable".
This could cause query execution failures in the benchmark.
##########
pinot-perf/src/main/java/org/apache/pinot/perf/BenchmarkOrderByQueries.java:
##########
@@ -194,42 +196,36 @@ private void buildSegment(String segmentName)
}
@Benchmark
- public BrokerResponseNative sortedAsc() {
+ public BrokerResponseNative sortedTotally() {
+ if (_zasc) {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
+ + "FROM sorted "
+ "ORDER BY SORTED_COL ASC "
- + "LIMIT 1052 "
- + "option(orderByAlgorithm=" + _orderByAlgorithm + ")");
- }
- @Benchmark
- public BrokerResponseNative sortedAscPartially() {
+ + "LIMIT " + _limit);
+ } else {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
- + "ORDER BY SORTED_COL ASC, LOW_CARDINALITY_STRING_COL "
- + "LIMIT 1052 "
- + "option(orderByAlgorithm=" + _orderByAlgorithm + ")");
+ + "FROM sorted "
+ + "ORDER BY SORTED_COL DESC "
+ + "LIMIT " + _limit);
+ }
}
-
@Benchmark
- public BrokerResponseNative sortedDesc() {
+ public BrokerResponseNative sortedPartially() {
+ if (_zasc) {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
- + "ORDER BY SORTED_COL DESC "
- + "LIMIT 1052 "
- + "option(orderByAlgorithm=" + _orderByAlgorithm + ")");
- }
-
- @Benchmark
- public BrokerResponseNative sortedDescPartially() {
+ + "FROM sorted "
+ + "ORDER BY SORTED_COL ASC, LOW_CARDINALITY_STRING_COL "
+ + "LIMIT " + _limit);
+ } else {
return getBrokerResponse(
"SELECT SORTED_COL "
- + "FROM MyTable "
+ + "FROM sorted "
Review Comment:
The table name has been changed from "MyTable" to "sorted", but this appears
to be inconsistent with the schema definition which likely expects "MyTable".
This could cause query execution failures in the benchmark.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
