jtao15 commented on a change in pull request #6224:
URL: https://github.com/apache/incubator-pinot/pull/6224#discussion_r521663461
##########
File path:
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/ClusterIntegrationTestUtils.java
##########
@@ -833,6 +700,33 @@ static void testSqlQuery(String pinotQuery, String
brokerUrl, org.apache.pinot.c
return;
}
+ BrokerRequest brokerRequest =
+
PinotQueryParserFactory.get(CommonConstants.Broker.Request.SQL).compileToBrokerRequest(pinotQuery);
+ // Add order by columns which are not in selection clause for comparison
purpose
+ List<String> orderByColumns = new ArrayList<>();
+ List<String> selectionColumns = new ArrayList<>();
+ if (isSelectionQuery(brokerRequest) && brokerRequest.getOrderBy() != null
&& brokerRequest.getOrderBy().size() > 0) {
+
orderByColumns.addAll(CalciteSqlParser.extractIdentifiers(brokerRequest.getPinotQuery().getOrderByList(),
false));
+
selectionColumns.addAll(CalciteSqlParser.extractIdentifiers(brokerRequest.getPinotQuery().getSelectList(),
false));
+ convertToUpperCase(orderByColumns);
+ convertToUpperCase(selectionColumns);
+
+ if (!selectionColumns.containsAll(orderByColumns)) {
+ List<String> inputRequests = new ArrayList<>();
+ inputRequests.add(pinotQuery);
+ inputRequests.addAll(sqlQueries);
+
+ Set<String> orderByColumnsExcluded = new HashSet<>(orderByColumns);
+ orderByColumnsExcluded.removeAll(selectionColumns);
+
+ // Append order-by columns not in selection clause so the order of
query responses can be verified
+ // e.g. we can't verify the order of query `SELECT firstName FROM
mytable ORDER BY lastName' if there are duplicate lastNames
Review comment:
I think the problem here is whether we want to check the returned order
of selection queries which order-by columns are not in selection clause. We
skipped comparison for this case for previous sql comparison, and current pql
logic will not verify the order but only compare values. So without the
order-by clause append, the sql comparison will be consistent with current pql
logic, and all test cases should pass.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
