jayzhan211 commented on code in PR #9920:
URL: https://github.com/apache/arrow-datafusion/pull/9920#discussion_r1549891289
##########
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:
> > I think it is confusing for the user to understand whether they need to
check or not. I think rename it to `disable_xxx` help or change the logic an
rename it to `enable_xxx`
>
> I don't quite understand this suggestion -- are you suggesting rename
`check_ignore_nulls` to `disable_ignore_nulls`?
yes, that is what I suggest, so we know exactly whether it is disabled or
not. But I think the comment here also helps, rename is not neccessary
--
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]
