mihaibudiu commented on code in PR #4448:
URL: https://github.com/apache/calcite/pull/4448#discussion_r2175578490
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -15906,6 +15909,9 @@ void testTimestampDiff(boolean coercionEnabled) {
f.checkType("variance(cast(null as varchar(2)))", "DECIMAL(19, 9)");
f.checkType("variance(CAST(NULL AS INTEGER))", "INTEGER");
f.checkAggType("variance(DISTINCT 1.5)", "DECIMAL(2, 1) NOT NULL");
+
+ final String[] values2 = {"cast(64.34 as double)", "64.34", "64.34"};
+ f.checkAgg("variance(x)", values2, isExactly(0));
Review Comment:
This looks somewhat dangerous; FP arithmetic is rarely exact.
For example, I have seen differences between x86 and AARCH64.
But if it works now, probably it will continue to work.
##########
core/src/main/java/org/apache/calcite/rel/rules/AggregateReduceFunctionsRule.java:
##########
@@ -608,8 +609,31 @@ private static RexNode reduceStddev(
aggCallMapping,
oldAggRel.getInput()::fieldIsNullable);
+ final RexNode avgSumSquaredArg =
+ rexBuilder.makeCall(pos, SqlStdOperatorTable.DIVIDE, sumSquaredArg,
countArg);
+ final RexNode diff =
+ rexBuilder.makeCall(pos, SqlStdOperatorTable.MINUS, sumArgSquared,
avgSumSquaredArg);
+
+ final RelDataType oldArgType =
+ SqlTypeUtil.projectTypes(oldAggRel.getInput().getRowType(),
oldCall.getArgList())
+ .get(0);
+ final RexNode correctedDiff;
+
+ switch (oldArgType.getSqlTypeName()) {
+ case DOUBLE: case DECIMAL:
+ final RexNode zeroLiteral =
+ rexBuilder.makeExactLiteral(BigDecimal.ZERO);
+ correctedDiff =
+ rexBuilder.makeCall(SqlStdOperatorTable.CASE,
+ rexBuilder.makeCall(SqlStdOperatorTable.LESS_THAN, diff,
zeroLiteral),
Review Comment:
I wonder whether using less than or equal would make sense
doubles have this concept of a negative zero, which may cause problems
I am not sure whether it can surface here, but there should be no harm
--
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]
