mihaibudiu commented on code in PR #3733:
URL: https://github.com/apache/calcite/pull/3733#discussion_r1621083730


##########
babel/src/test/resources/sql/redshift.iq:
##########
@@ -523,78 +523,78 @@ select deptno, ratio_to_report(sal) over (partition by 
deptno) from emp;
 !}
 
 # STDDEV_POP
-select empno, stddev_pop(comm) over (order by empno rows unbounded preceding) 
from emp where deptno = 30 order by 1;
+select empno, stddev_pop(CAST(comm AS DECIMAL(12, 4))) over (order by empno 
rows unbounded preceding) from emp where deptno = 30 order by 1;

Review Comment:
   I agree that this is unpleasant. However, please note that this 
implementation only affects the Enumerable-based implementation, which is used 
to evaluate the Quidem tests. Other runtimes can choose other strategies for 
the precision of intermediate results.
   
   There was a discussion about this in JIRA: 
https://issues.apache.org/jira/browse/CALCITE-6324, where @julianhyde pointed 
out that the problem is indeed the strategy chosen for implementing functions 
such as STDDEV.
   
   So I think this is really a separate issue. If we think that the 
implementation of STDDEV (and a few other aggregates in Calcite's runtime is 
wrong, we should file an issue about that and address it. Then we can revert 
these tests to use a narrower type for the result.



-- 
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]

Reply via email to