[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6503: Bug fix, First accumulator multiple batch aware

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #6503:
URL: https://github.com/apache/arrow-datafusion/pull/6503#discussion_r1212296445


##########
datafusion/physical-expr/src/aggregate/first_last.rs:
##########
@@ -112,25 +112,35 @@ impl PartialEq<dyn Any> for FirstValue {
 #[derive(Debug)]
 struct FirstValueAccumulator {
     first: ScalarValue,
+    // At the beginning, `is_set` is `false`, this means `first` is not seen yet.
+    // Once we see (`is_set=true`) first value, we do not update `first`.
+    is_set: bool,

Review Comment:
   Another potential way that might let the compiler check that `is_set` was updated correctly would be something like:
   
   ```
   struct FirstValueAccumulator {
     /// None until the data has been seen
     first: Option<ScalarValue>
   }
   
   ```



-- 
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: github-unsubscribe@arrow.apache.org

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