feat: add aggregate count functions with decimal return type #670
Conversation
scgkiran
requested review from
jacques-n,
cpcloud,
westonpace,
EpsilonPrime and
vbarua
as code owners
| %YAML 1.2 | ||
| --- | ||
| aggregate_functions: | ||
| - name: "count" |
There was a problem hiding this comment.
overloading functions based only on return type causes problems, see e.g. #606 (comment)
There was a problem hiding this comment.
I have them in a different yaml file. can this still cause problems?
There was a problem hiding this comment.
different files should be ok.
There was a problem hiding this comment.
I believe it'll still break substrait-java, see substrait-io/substrait-java#275
There was a problem hiding this comment.
I think we should fix that in substrait-java then as opposed to holding back the specification. Extensions being independent is key to extensions being extensions 😊
There was a problem hiding this comment.
It's possible for us to work around duplicates in substrait-java by excluding them from the default extensions for now. See substrait-io/substrait-java#275 (comment)
| @@ -0,0 +1,41 @@ | |||
| %YAML 1.2 | |||
| --- | |||
| aggregate_functions: | |||
There was a problem hiding this comment.
the name of this file is weird. in other cases type names in filenames are about input types and this one is about output types. I suggest we rename file to something that is more descriptive. Maybe also add a little text to the functions clarifying how they are different than classic count functions.
No description provided.