Add returnsZeroOnEmptyInput to AggregationFunctionMetadata

[lukasz-stec]

Description

COUNT, COUNT_IF, and APPROX_DISTINCT do have not standard
behavior when no input was supplied to the function.
They return the value 0, as opposed to standard NULL.
Before this change, OptimizeMixedDistinctAggregations relied on
matching string function name to handle this case, but
this is brittle and may cause silent correctness issues
if a new function with this characteristic is added.

Additional context and related issues

Release notes

( X) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

[lukasz-stec]

@dain @sopel39 I'm not sure this is really necessary but it seems better than comparing strings in some rule. WDYT?

[sopel39]

lgtm % but I would wait for @dain review too

[lukasz-stec]

I rebased on the master and dropped the first commit with tests that landed there

[martint]

I’m not sure this is the right abstraction. There’s nothing standard about returning 0 on empty input. What would that property mean for a function that does not return a number but returns, say, an empty array or map on no input?

[lukasz-stec]

I’m not sure this is the right abstraction. There’s nothing standard about returning 0 on empty input. What would that property mean for a function that does not return a number but returns, say, an empty array or map on no input?

Without changes to the OptimizeMixedDistinctAggregations, a function like that would introduce a silent correctness issue in case OptimizeMixedDistinctAggregations is enabled.
The motivation for this change is to make the author of the new function think about this characteristic.

[martint]

I understand that, and it’s fine for the optimization to be targeted at functions that return 0 on no input. I’m not sure it should be a generic attribute for every aggregation, unless we can generalize it to aggregations that don’t return numbers.

[lukasz-stec]

Generalization would overcomplicate things and be hard to test, so I wouldn't introduce it until needed.
If not this generic attribute, do you see some other way to handle

if (signatureName.equals("count") || signatureName.equals("count_if") || signatureName.equals("approx_distinct")) {

?
Or we shouldn't worry about potential bug here?

[lukasz-stec]

As @martint does not agree it's a good approach, I'm closing the PR