> On Aug. 26, 2014, 11:05 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/codegen/templates/RegrTypeFunctions.java, line 110
> > <https://reviews.apache.org/r/25012/diff/2/?file=667996#file667996line110>
> >
> >     How do we handle overflow in the numeric operation?

How can it be best handled? Should we select any of the Decimal*Holders ?


> On Aug. 26, 2014, 11:05 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/codegen/templates/RegrTypeFunctions.java, line 120
> > <https://reviews.apache.org/r/25012/diff/2/?file=667996#file667996line120>
> >
> >     How do we handle the case where count = 0?

Returning 0 for zero count/ all null records


> On Aug. 26, 2014, 11:05 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/test/resources/regr_input.json, line 300
> > <https://reviews.apache.org/r/25012/diff/2/?file=667999#file667999line300>
> >
> >     I'm not sure what other people will think about this input data, 
> > containing 300 rows. I feel it's not easy to manually compute the result, 
> > and verify whether the result is correct. For unit test, I think we had 
> > better keep the data as simple as possilbe, while cover as wide scenarios 
> > as possible.
> >     
> >     Also, the unit test does not cover the case of some paris contain null 
> > values, and the case all the pairs contain nulls.

Actually wanted to test against more records. Have moved to smaller testcase. 
Enhanced data with broader test scenarios.


> On Aug. 26, 2014, 11:05 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/codegen/templates/RegrTypeFunctions.java, line 129
> > <https://reviews.apache.org/r/25012/diff/2/?file=667996#file667996line129>
> >
> >     Also, seems to me the code for "regr_avgx" and "regr_avgy" are 
> > essentially same. Can we use one single template to generate them, by 
> > adding a list ("x", "y") to the freemarker template?

Merged into single template. But it makes the template slightly complex.


- Yash


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25012/#review51602
-----------------------------------------------------------


On Aug. 24, 2014, 9:05 a.m., Yash Sharma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25012/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2014, 9:05 a.m.)
> 
> 
> Review request for drill, Jinfeng Ni and Mehant Baid.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Stats function regr_avgx, regr_avgy
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/codegen/config.fmpp ff6135d 
>   exec/java-exec/src/main/codegen/data/RegrTypes.tdd PRE-CREATION 
>   exec/java-exec/src/main/codegen/templates/RegrTypeFunctions.java 
> PRE-CREATION 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunction.java
>  5e57dc7 
>   exec/java-exec/src/test/resources/functions/test_regression.json 
> PRE-CREATION 
>   exec/java-exec/src/test/resources/regr_input.json PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25012/diff/
> 
> 
> Testing
> -------
> 
> Done.
> $mvn test -Dtest=TestAggregateFunction#testRegr
> 
> 
> Thanks,
> 
> Yash Sharma
> 
>

Reply via email to