cshuo commented on a change in pull request #17634:
URL: https://github.com/apache/flink/pull/17634#discussion_r743368304
##########
File path:
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/SumAggFunction.java
##########
@@ -66,7 +66,10 @@ public int operandCount() {
ifThenElse(
isNull(operand(0)),
sum,
- ifThenElse(isNull(sum), operand(0), plus(sum,
operand(0)))))
+ ifThenElse(
Review comment:
This fix probably does not work. You can check by the following test
case,
```
@Test
def testDecimalSum(): Unit = {
val data = new mutable.MutableList[(Double, Int)]
data .+= ((1.11111111, 1))
data .+= ((1.11111111, 2))
env.setParallelism(1)
val t = failingDataSource(data).toTable(tEnv, 'a, 'b)
tEnv.registerTable("T", t)
val t1 = tEnv.sqlQuery("select sum(cast(a as decimal(32, 8))) from T")
val sink = new TestingRetractSink
t1.toRetractStream[Row].addSink(sink)
env.execute()
val expected = List("2.22222220")
assertEquals(expected, sink.getRetractResults)
}
```
As shown above, the result is also finally rounded to (38, 7), then cast to
(38, 8) which also have the precision losing problem.
This is because that during codegen of DeclarativeAggregateFunction, 'plus'
expression is convert to RexNode `PLUS` firstly, and during the converting
there also exists type inference(..ReturnTypes.NULLABLE_SUM) which infers the
return type of 'PLUS' as decimal(38, 7)...
So, maybe we should introduce an internal built-in SqlFunction, e.g,
'SUM_PLUS' for SUM, which has a different return type inference (maybe
ReturnTypes.LEAST_RESTRICTIVE is enough), and infers return type of SUM just
same as the type of operand.
Besides, there is same problem for AVG.
--
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]
