[GitHub] [flink] dawidwys commented on a change in pull request #13802: [FLINK-19793][connector-kafka] Harden KafkaTableITCase.testKafkaSourceSinkWithMetadata

[GitBox <gi...@apache.org>]

dawidwys commented on a change in pull request #13802:
URL: https://github.com/apache/flink/pull/13802#discussion_r512578037



##########
File path: flink-core/src/main/java/org/apache/flink/types/Row.java
##########
@@ -294,15 +295,24 @@ public static boolean deepEquals(Row row1, Row row2) {
 
 	/**
 	 * Compares two {@link List}s of {@link Row} for deep equality. This method supports all conversion
-	 * classes of the table ecosystem.
+	 * classes of the table ecosystem. The top-level lists can be compared with or without order.
 	 *
 	 * <p>The current implementation of {@link Row#equals(Object)} is not able to compare all deeply
 	 * nested row structures that might be created in the table ecosystem. For example, it does not
 	 * support comparing arrays stored in the values of a map. We might update the {@link #equals(Object)}
 	 * with this implementation in future versions.
 	 */
-	public static boolean deepEquals(List<Row> l1, List<Row> l2) {
-		return deepEqualsInternal(l1, l2);
+	public static boolean deepEquals(List<Row> l1, List<Row> l2, boolean ignoreOrder) {

Review comment:
       nit: Shouldn't this be a method in some test utility? It's rather strictly tailored for a particular test. It does not cover all the possible cases (e.g. nested ordered/unordered comparison).
   
   Overall I think it does not harm having the method, but I'd at least have a method with `ignoreOrder = false` by default. Personally though I'd prefer to have it only in the `TableTestMatchers`




----------------------------------------------------------------
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