yashmayya commented on code in PR #13193:
URL: https://github.com/apache/pinot/pull/13193#discussion_r1608662915
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/SqlResultComparator.java:
##########
@@ -163,6 +167,97 @@ public static boolean areEqual(JsonNode actual, JsonNode
expected, String query)
return areResultsEqual;
}
+ public static boolean areMultiStageQueriesEqual(JsonNode actual, JsonNode
expected, String query)
+ throws IOException {
+ if (hasExceptions(actual)) {
+ return false;
+ }
+
+ if (areEmpty(actual, expected)) {
+ return true;
+ }
+
+ if (!areDataSchemaEqual(actual, expected)) {
+ return false;
+ }
+
+ ArrayNode actualRows = (ArrayNode)
actual.get(FIELD_RESULT_TABLE).get(FIELD_ROWS);
+ ArrayNode expectedRows = (ArrayNode)
expected.get(FIELD_RESULT_TABLE).get(FIELD_ROWS);
+ ArrayNode columnDataTypes = (ArrayNode)
expected.get(FIELD_RESULT_TABLE).get(FIELD_DATA_SCHEMA).
+ get(FIELD_COLUMN_DATA_TYPES);
+
+ convertNumbersToString(expectedRows, columnDataTypes);
+ convertNumbersToString(actualRows, columnDataTypes);
+
+ List<String> actualElementsSerialized = new ArrayList<>();
+ List<String> expectedElementsSerialized = new ArrayList<>();
+ for (int i = 0; i < actualRows.size(); i++) {
+ actualElementsSerialized.add(actualRows.get(i).toString());
+ }
+ for (int i = 0; i < expectedRows.size(); i++) {
+ expectedElementsSerialized.add(expectedRows.get(i).toString());
+ }
+
+ if (!areLengthsEqual(actual, expected)) {
+ return false;
+ }
+
+ // For now, just directly compare elements in result set
+ if (!areNonOrderByQueryElementsEqual(actualElementsSerialized,
expectedElementsSerialized)) {
+ return false;
+ }
+
+ // Compare stage stats
+ ObjectNode actualStageStats = (ObjectNode)
actual.get(FIELD_MULTI_STAGE_STATS);
+ ObjectNode expectedStageStats = (ObjectNode)
expected.get(FIELD_MULTI_STAGE_STATS);
+ return areMultiStageStatsEqual(actualStageStats, expectedStageStats);
+ }
+
+ private static boolean areMultiStageStatsEqual(ObjectNode actualStageStats,
ObjectNode expectedStageStats) {
+ String actualType = actualStageStats.get(FIELD_MULTI_STAGE_STATS_TYPE) !=
null
+ ? actualStageStats.get(FIELD_MULTI_STAGE_STATS_TYPE).asText()
+ : null;
+ String expectedType = expectedStageStats.get(FIELD_MULTI_STAGE_STATS_TYPE)
!= null
+ ? expectedStageStats.get(FIELD_MULTI_STAGE_STATS_TYPE).asText()
+ : null;
+
+ if (actualType != null && !actualType.equals(expectedType)) {
+ LOGGER.error("Mismatch in stage stats type. Actual: {}, Expected: {}",
actualType, expectedType);
+ return false;
+ }
+
+ ArrayNode actualChildren = (ArrayNode)
actualStageStats.get(FIELD_MULTI_STAGE_STATS_CHILDREN);
+ ArrayNode expectedChildren = (ArrayNode)
expectedStageStats.get(FIELD_MULTI_STAGE_STATS_CHILDREN);
+
+ if (actualChildren == null && expectedChildren == null) {
+ return true;
+ }
+ if (actualChildren == null) {
+ LOGGER.error("No children found in stage stats for type: {}. Expected {}
children.",
+ actualType, expectedChildren.size());
+ return false;
+ }
+ if (expectedChildren == null) {
+ LOGGER.error("Found unexpected children in stage stats for type: {}.
Expected no children.", actualType);
+ return false;
+ }
+ if (actualChildren.size() != expectedChildren.size()) {
+ LOGGER.error("Mismatch in number of children for stage stats for type:
{}. Actual: {}, Expected: {}",
+ actualType, actualChildren.size(), expectedChildren.size());
+ return false;
+ }
+
+ for (int i = 0; i < actualChildren.size(); i++) {
+ if (!areMultiStageStatsEqual((ObjectNode) actualChildren.get(i),
(ObjectNode) expectedChildren.get(i))) {
+ return false;
+ }
+ }
+
+ // TODO: Verify other stats like emittedRows, fanIn, fanOut,
inMemoryMessages, rawMessages etc.?
Review Comment:
Yeah, makes sense. I already ran into an issue when running this on a
slightly older commit because of the change in the structure of multi-stage
stats. Should I keep this as is or remove this stats check altogether?
--
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]
