kbendick commented on a change in pull request #3728:
URL: https://github.com/apache/iceberg/pull/3728#discussion_r769225800



##########
File path: 
spark/v3.2/spark/src/test/java/org/apache/iceberg/spark/SparkTestBase.java
##########
@@ -118,6 +119,8 @@ protected long waitUntilAfter(long timestampMillis) {
             return row.getList(pos);
           } else if (value instanceof scala.collection.Map) {
             return row.getJavaMap(pos);
+          } else if (value.getClass().isArray() && 
value.getClass().getComponentType().isPrimitive()) {
+            return IntStream.range(0, Array.getLength(value)).mapToObj(i -> 
Array.get(value, i)).toArray();

Review comment:
       Question: This change to the test class seems somewhat complicated 
relative to the other cases. Is there any way to check for `value instanceof 
scala.collection.Array` (assuming that Scala types are returned) like in the 
map case and in the basic list case?
   
   I guess my question is, why do we need this block when we have the check for 
`scala.collection.Seq` already. Can you help me understand?
   
   And possibly, since it is a bit more complicated, a comment addressing what 
case this is expected to handle in the code might be called for.




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

Reply via email to