You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2019/12/04 03:12:05 UTC

[GitHub] [flink] KurtYoung commented on a change in pull request #10405: [FLINK-15044][e2e tests] Clean up TpcdsResultComparator

KurtYoung commented on a change in pull request #10405: [FLINK-15044][e2e tests] Clean up TpcdsResultComparator
URL: https://github.com/apache/flink/pull/10405#discussion_r353530112
 
 

 ##########
 File path: flink-end-to-end-tests/flink-tpcds-test/src/main/java/org/apache/flink/table/tpcds/utils/TpcdsResultComparator.java
 ##########
 @@ -47,180 +47,188 @@
 			"91", "92", "93", "94", "95", "96", "97", "98", "99"
 	);
 
+	private static final HashSet<String> NULL_LITERALS = new HashSet<>(Arrays.asList("%", "-", "NULL", "[NULL]"));
+
 	private static final String REGEX_SPLIT_BAR = "\\|";
-	private static final String FILE_SEPARATOR = "/";
 	private static final String RESULT_SUFFIX = ".ans";
 	private static final double TOLERATED_DOUBLE_DEVIATION = 0.01d;
 
 	public static void main(String[] args) throws Exception {
 		ParameterTool params = ParameterTool.fromArgs(args);
 		String expectedDir = params.getRequired("expectedDir");
 		String actualDir = params.getRequired("actualDir");
+
 		int passCnt = 0;
+
 		for (String queryId : VALIDATE_QUERIES) {
-			File expectedFile = new File(expectedDir + FILE_SEPARATOR + queryId + RESULT_SUFFIX);
-			File actualFile = new File(actualDir + FILE_SEPARATOR + queryId + RESULT_SUFFIX);
+			File expectedFile = new File(expectedDir, queryId + RESULT_SUFFIX);
+			File actualFile = new File(actualDir, queryId + RESULT_SUFFIX);
 
-			if (compareResult(expectedFile, actualFile)) {
+			if (compareResult(queryId, expectedFile, actualFile)) {
 				passCnt++;
-				System.out.println("[INFO] validate success, file: " + expectedFile.getName() + " total query:" + VALIDATE_QUERY_NUM + " passed query:" + passCnt);
+				System.out.println(String.format("[INFO] Validation succeeded for file: %s (%d/%d)",
+					expectedFile.getName(), passCnt, VALIDATE_QUERIES.size()));
 			} else {
-				System.out.println("[ERROR] validate fail, file: " + expectedFile.getName() + "\n");
+				System.out.println("[ERROR] Validation failed for file: " + expectedFile.getName() + '\n');
 			}
 		}
-		if (passCnt == VALIDATE_QUERY_NUM) {
+
+		if (passCnt == VALIDATE_QUERIES.size()) {
 			System.exit(0);
 		}
 		System.exit(1);
 	}
 
-	private static boolean compareResult(File expectedFile, File actualFile) throws Exception {
-		BufferedReader expectedReader = new BufferedReader(new FileReader(expectedFile));
-		BufferedReader actualReader = new BufferedReader(new FileReader(actualFile));
-
-		int expectedLineNum = 0;
-		int actualLineNum = 0;
-
-		String expectedLine, actualLine;
-		while ((expectedLine = expectedReader.readLine()) != null &&
-				(actualLine = actualReader.readLine()) != null) {
-			expectedLineNum++;
-			actualLineNum++;
-
-			// reslut top 8 line of query 34,
-			// result line  2、3  0f query 77
-			// result line 18、 19 of query 79
-			// have different order with answer set, because of Flink keep nulls last for DESC, nulls first for ASC.
-			// it's still up to TPC-DS standard.
-			if ("34.ans".equals(expectedFile.getName()) && expectedLineNum == 1) {
-				List<String> expectedTop8Line = new ArrayList<>(8);
-				List<String> actualTop8Line = new ArrayList<>(8);
-
-				String expectedLine2 = expectedReader.readLine();
-				expectedLineNum++;
-				while (expectedLineNum < 8) {
-					expectedTop8Line.add(expectedReader.readLine());
-					expectedLineNum++;
-				}
-				expectedTop8Line.add(expectedLine);
-				expectedTop8Line.add(expectedLine2);
+	private static boolean compareResult(String queryId, File expectedFile, File actualFile) throws Exception {
+		final String[] expectedLines = Files.readAllLines(expectedFile.toPath(), StandardCharsets.UTF_8).toArray(new String[0]);
+		final String[] actualLines = Files.readAllLines(actualFile.toPath(), StandardCharsets.UTF_8).toArray(new String[0]);
 
-				actualTop8Line.add(actualLine);
-				while (actualLineNum < 8) {
-					actualTop8Line.add(actualReader.readLine());
-					actualLineNum++;
-				}
-				for (int i = 0; i < 8; i++) {
-					if (!isEqualLine(expectedTop8Line.get(i), actualTop8Line.get(i), i)) {
-						return false;
-					}
-				}
-				continue;
-			}
-			if ("77.ans".equals(expectedFile.getName()) && expectedLineNum == 2) {
-				String expectedLine3 = expectedReader.readLine();
-				String actualLine3 = actualReader.readLine();
-				expectedLineNum++;
-				actualLineNum++;
-
-				if (isEqualLine(expectedLine, actualLine3, 2) && isEqualLine(expectedLine3, actualLine, 3)) {
-					continue;
-				}
-				if (isEqualLine(expectedLine, actualLine, 2) && isEqualLine(expectedLine3, actualLine3, 3)) {
-					continue;
-				}
-				return false;
-			}
+		if (expectedLines.length != actualLines.length) {
+			System.out.println(String.format(
+				"[ERROR] Incorrect number of lines! Expecting %d lines, but found %d lines.",
+				expectedLines.length, actualLines.length));
 
-			if ("79.ans".equals(expectedFile.getName()) && expectedLineNum == 18) {
-				String expectedLine19 = expectedReader.readLine();
-				String actualLine19 = actualReader.readLine();
-				expectedLineNum++;
-				actualLineNum++;
+			return false;
+		}
 
-				if (isEqualLine(expectedLine, actualLine, 18) && isEqualLine(expectedLine19, actualLine19, 19)) {
-					continue;
-				}
-				if (isEqualLine(expectedLine, actualLine19, 18) && isEqualLine(expectedLine19, actualLine, 19)) {
-					continue;
-				}
-				return false;
-			}
+		if ("34".equals(queryId)) {
+			return compareQuery34(expectedLines, actualLines);
+		}
+		else if ("77".equals(queryId)) {
+			return compareQuery77(expectedLines, actualLines);
+		}
+		else if ("79".equals(queryId)) {
+			return compareQuery79(expectedLines, actualLines);
+		}
+		else {
+			return compareLinesPrintingErrors(expectedLines, actualLines, 0);
+		}
+	}
 
-			if (!isEqualLine(expectedLine, actualLine, expectedLineNum)) {
-				return false;
-			}
+	// ------------------------------------------------------------------------
+
+	private static boolean compareQuery34(String[] expectedLines, String[] actualLines) {
+		// take the first two lines and move them back to lines 7 and 8
+		final String expected1 = expectedLines[0];
+		final String expected2 = expectedLines[1];
+		System.arraycopy(expectedLines, 2, expectedLines, 0, 6);
+		expectedLines[6] = expected1;
+		expectedLines[7] = expected2;
+
+		return compareLinesPrintingErrors(expectedLines, actualLines, 0);
+	}
+
+	private static boolean compareQuery77(String[] expectedLines, String[] actualLines) {
+		if (!compareLinePrintingErrors(expectedLines[0], actualLines[0], 1)) {
+			return false;
+		}
+		if (!comparePairOfLinesPrintingErrors(expectedLines[1], expectedLines[2], actualLines[1], actualLines[2], 2, 3)) {
+			return false;
 		}
+		return compareLinesPrintingErrors(expectedLines, actualLines, 3);
+	}
 
-		while (expectedReader.readLine() != null) {
-			expectedLineNum++;
+	private static boolean compareQuery79(String[] expectedLines, String[] actualLines) {
+		if (!compareLinesPrintingErrors(expectedLines, actualLines, 0, 17)) {
+			return false;
+		}
+		if (!comparePairOfLinesPrintingErrors(expectedLines[17], expectedLines[18], actualLines[17], actualLines[18], 18, 19)) {
+			return false;
 		}
-		while (actualReader.readLine() != null) {
-			actualLineNum++;
+		return compareLinesPrintingErrors(expectedLines, actualLines, 20);
+	}
+
+	// ------------------------------------------------------------------------
+
+	private static boolean compareLinesPrintingErrors(String[] expectedLines, String[] actualLines, int from, int num) {
+		for (int line = from; line < from + num; line++) {
+			if (!compareLinePrintingErrors(expectedLines[line], actualLines[line], line)) {
+				return false;
+			}
 		}
+		return true;
+	}
+
+	private static boolean compareLinesPrintingErrors(String[] expectedLines, String[] actualLines, int from) {
+		final int remaining = expectedLines.length - from;
+		return compareLinesPrintingErrors(expectedLines, actualLines, from, remaining);
+	}
+
+	private static boolean comparePairOfLinesPrintingErrors(
+			String expected1, String expected2,
+			String actual1, String actual2,
+			int lineNum1, int lineNum2) {
+
+		final boolean matchesPair = compareLine(expected1, actual1) && compareLine(expected2, actual2);
+		final boolean matchesCross = compareLine(expected2, actual1) && compareLine(expected1, actual2);
+
+		if (!(matchesPair || matchesCross)) {
+			System.out.println(String.format("[ERROR] Lines %d/%d do not match in any pairing." +
+					"\n - Expected %d: %s\n - Expected %d: %s\n - Actual %d: %s\n - Actual %d: %s",
+				lineNum1, lineNum2, lineNum1, expected1, lineNum2, expected2,
+				lineNum1, actual1, lineNum2, 2));
 
 Review comment:
   2 should be `actual2`?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services