Add aggregate function count_if for relational table #14511
Conversation
| @@ -567,6 +567,7 @@ && isIntegerNumber(argumentTypes.get(2)))) { | |||
| case SqlConstant.MIN: | |||
| case SqlConstant.MAX: | |||
| case SqlConstant.MODE: | |||
| case SqlConstant.COUNT_IF: | |||
There was a problem hiding this comment.
the type of argument should be boolean?
| @Override | ||
| public void addInput(int[] groupIds, Column[] arguments) { | ||
| for (int i = 0; i < groupIds.length; i++) { | ||
| if (arguments[0].getBoolean(i)) { |
There was a problem hiding this comment.
firstly should check the nullability of arguments by if (arguments[0].isNull(i))
| @Override | ||
| public void addInput(int[] groupIds, Column[] arguments) { | ||
| for (int i = 0; i < groupIds.length; i++) { | ||
| if (arguments[0].getBoolean(i)) { |
There was a problem hiding this comment.
| if (arguments[0].getBoolean(i)) { | |
| if (!arguments[0].isNull(i) && arguments[0].getBoolean(i)) { |
| } | ||
|
|
||
| @Override | ||
| public void addStatistics(Statistics[] statistics) {} |
There was a problem hiding this comment.
better to throw unsupported exception if we don't plan to support push down count_if into scan
|
|
||
| private long countState = 0; | ||
|
|
||
| public CountIfAccumulator(List<TSDataType> seriesDataType) { |
There was a problem hiding this comment.
don't need to pass seriesDataType here, you should check data type in the analyze phase.
| public void addInput(Column[] arguments) { | ||
| int count = arguments[0].getPositionCount(); | ||
| for (int i = 0; i < count; i++) { | ||
| if (arguments[0].getBoolean(i)) { |
There was a problem hiding this comment.
| if (arguments[0].getBoolean(i)) { | |
| if (!arguments[0].isNull(i) && arguments[0].getBoolean(i)) { |
| @Override | ||
| public void removeInput(Column[] arguments) { | ||
| for (int i = 0; i < arguments[0].getPositionCount(); i++) { | ||
| if (arguments[0].getBoolean(i)) { |
There was a problem hiding this comment.
| if (arguments[0].getBoolean(i)) { | |
| if (!arguments[0].isNull(i) && arguments[0].getBoolean(i)) { |
| } | ||
|
|
||
| @Override | ||
| public void removeInput(Column[] arguments) { |
There was a problem hiding this comment.
you should also override removable method to return true.
| @@ -149,6 +150,8 @@ private static GroupedAccumulator createBuiltinGroupedAccumulator( | |||
| switch (aggregationType) { | |||
| case COUNT: | |||
| return new GroupedCountAccumulator(); | |||
| case COUNT_IF: | |||
| return new GroupedCountIfAccumulator(inputDataTypes); | |||
There was a problem hiding this comment.
| return new GroupedCountIfAccumulator(inputDataTypes); | |
| return new GroupedCountIfAccumulator(); |
| @@ -433,6 +433,327 @@ public void countTest() { | |||
| tableResultSetEqualTest("select count(*) from table1", expectedHeader, retArray, DATABASE_NAME); | |||
| } | |||
|
|
|||
| @Test | |||
| public void countIfTest() { | |||
There was a problem hiding this comment.
also need to add some extra unexpected exception test, like:
- input expression is not boolean type
- number of input expressions are not only 1
Add aggregate function count_if for relational table
Content1 ...
Content2 ...
Content3 ...
This PR has:
for an unfamiliar reader.
for code coverage.
Key changed/added classes (or packages if there are too many classes) in this PR