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


##########
api/src/main/java/org/apache/iceberg/expressions/ValueAggregate.java:
##########
@@ -30,13 +30,16 @@ protected ValueAggregate(Operation op, BoundTerm<T> term) {
 
   @Override
   public T eval(StructLike struct) {
-    return term().eval(struct);
+    if (struct.size() > 1) {
+      throw new UnsupportedOperationException("Expected struct like of size 
1");
+    }
+
+    return (T) struct.get(0, term().type().typeId().javaClass());
   }
 
   @Override
   public T eval(DataFile file) {
-    valueStruct.setValue(evaluateRef(file));
-    return term().eval(valueStruct);
+    return (T) evaluateRef(file);

Review Comment:
   Ok so the main thing here is do we acutally need to pass the single value 
down through term evaluation? what's a case where this would fail since all the 
tests currently pass without it but I'm doubtful that this generalizes. 



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