mqliang commented on a change in pull request #6583:
URL: https://github.com/apache/incubator-pinot/pull/6583#discussion_r582218992
##########
File path:
pinot-integration-tests/src/test/java/org/apache/pinot/compat/tests/QueryOp.java
##########
@@ -55,7 +72,97 @@ public void setExpectedResultsFileName(String
expectedResultsFileName) {
@Override
boolean runOp() {
- System.out.println("Verifying queries in " + _queryFileName + " against
results in " + _expectedResultsFileName);
- return true;
+ System.out.printf("Verifying queries in %s against results in %s\n",
_queryFileName, _expectedResultsFileName);
+ try {
+ return verifyQueries();
+ } catch (Exception e) {
+ LOGGER.error("FAILED to verify queries in {}: {}", _queryFileName, e);
+ return false;
+ }
+ }
+
+ boolean verifyQueries()
+ throws Exception {
+ BufferedReader expectedResultReader = null;
+ boolean testPassed = false;
+
+ try (BufferedReader queryReader = new BufferedReader(
+ new InputStreamReader(new FileInputStream(_queryFileName),
StandardCharsets.UTF_8))) {
+ if (_queryFileName == null) {
+ LOGGER.error("Result file is missing!");
+ return testPassed;
+ } else {
+ expectedResultReader = new BufferedReader(
+ new InputStreamReader(new
FileInputStream(_expectedResultsFileName), StandardCharsets.UTF_8));
+ }
+
+ int passed = 0;
+ int total = 0;
+ int queryLineNum = 0;
+ String query;
+
+ while ((query = queryReader.readLine()) != null) {
+ queryLineNum++;
+ if (shouldIgnore(query)) {
+ continue;
+ }
+
+ JsonNode expectedJson = null;
+ try {
+ String expectedResultLine = expectedResultReader.readLine();
+ while (shouldIgnore(expectedResultLine)) {
+ expectedResultLine = expectedResultReader.readLine();
+ }
+ expectedJson = JsonUtils.stringToJsonNode(expectedResultLine);
+ } catch (Exception e) {
+ LOGGER.error("Comparison FAILED: Line: {} Exception caught while
getting expected response for query: '{}'",
Review comment:
From all-queries point of view, there is no early-termination logic
here. Say we have 100 queries, if the first query failed, instead of returning
test failure immediately, we will continue to verify the remaining 99 queries,
and we will return test failure at the very end since 99 != 100. In this way,
can detect as many problematic queries as possible in one go.
From a single query point of view, yes if an exception was caught when
parsing the expected results, we indeed skip firing queries to the cluster:
```
if (expectedJson != null) {
try {
actualJson = QueryProcessor.postSqlQuery(query);
} catch (Exception e) {
LOGGER.error("Comparison FAILED: Line: {} Exception caught while
running query: '{}'", queryLineNum, query,
e);
}
}
```
We only fire query to cluster if `expectedJson != null`
----------------------------------------------------------------
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
