amogh-jahagirdar commented on code in PR #12925:
URL: https://github.com/apache/iceberg/pull/12925#discussion_r2065200539


##########
data/src/test/java/org/apache/iceberg/data/TestLocalScan.java:
##########
@@ -263,9 +264,37 @@ public void testRandomData() throws IOException {
 
     append.commit();
 
-    Set<Record> records = Sets.newHashSet(IcebergGenerics.read(table).build());
+    RecordComparator comparator = new RecordComparator();
+    List<Record> records = 
Lists.newArrayList(IcebergGenerics.read(table).build());
+

Review Comment:
   cc @rdblue I changed this logic because now it's possible that we generate 
some duplicate records (now that the random generation of null fields is ever 
so slightly different). This surfaced an issue in how we assert here because 
expected is a list of all the records which we wrote. However, the current 
implementation takes the actual results, put them in a set, which will dedupe 
them and the assertion below on size would fail since the set would have fewer 
elements.
   
   For this test we just should compare the exact lists, and to do that I sort 
the records and just do an element wise comparison. It's only 1000 elements so 
the sort and element wise check seems inconsequential in terms of time.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to