jayzhan211 commented on code in PR #9920:
URL: https://github.com/apache/arrow-datafusion/pull/9920#discussion_r1549603208


##########
datafusion/expr/src/function.rs:
##########
@@ -67,6 +88,24 @@ impl<'a> AccumulatorArgs<'a> {
             sort_exprs,
         }
     }
+
+    /// Return a not yet implemented error if IGNORE NULLs is true
+    pub fn check_ignore_nulls(&self, name: &str) -> Result<()> {
+        if self.ignore_nulls {

Review Comment:
   But I think there is also a problem that because if they forget to add the 
`checking` in the accumulator, they can still run the function but incorrectly 
compute unnecessary things, `order by` for example.
   
   To *enforce* they specified the options for their functions, I think we can 
implement options directly in `AggregateUDFImpl`.
   
   Either 
   ```rust
   // We will check if the AccArgs meet the requirement or not.
   fn options() -> Args {
      Args {
        ignore_nulls: true/ false,
        has_ordering: true / false
       }
   }
   ```
   
   or 
   ```rust
   // The same with this one, but separate each option
   fn support_ignore_nulls() -> bool
   fn support_ordering() -> bool
   ```



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

Reply via email to