Enable Date type input in function Count() #931
Conversation
Add IT Add Comparsion Test
Add IT
Codecov Report
@@ Coverage Diff @@
## develop #931 +/- ##
==========================================
Coverage 99.86% 99.86%
- Complexity 2265 2315 +50
==========================================
Files 229 230 +1
Lines 5235 5329 +94
Branches 346 346
==========================================
+ Hits 5228 5322 +94
Misses 5 5
Partials 2 2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
LGTM, thanks for the changes. A suggestion: it is fine to address comments and make changes to your code in the same pull request rather than open a new one for replacement. As long as you "squash and merge" all the commits to one commit at the end when you merge your PR.
| .put(new FunctionSignature(functionName, Collections.singletonList(DATE)), | ||
| arguments -> new CountAggregator(arguments, INTEGER)) | ||
| .put(new FunctionSignature(functionName, Collections.singletonList(DATETIME)), | ||
| arguments -> new CountAggregator(arguments, INTEGER)) | ||
| .put(new FunctionSignature(functionName, Collections.singletonList(TIMESTAMP)), | ||
| arguments -> new CountAggregator(arguments, INTEGER)) |
There was a problem hiding this comment.
I assume SQL COUNT() should work with all types. If so, is it more maintainable to generate function signatures from all core types like this way:
…rch#931) * Enable count(Date) Add IT Add Comparsion Test * Enable count(Date) Add IT * Add comparsion test file 916.txt * Consolidate count function to accept all field type
Issue #916, if available:
Description of changes:
Add three input type (DATE, DATETIME, TIMESTAMP) in Count()
Add UT (DATE, DATETIME or TIMESTAMP) input type
Add Comparison Test
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.