> On Aug. 26, 2014, 11:05 p.m., Jinfeng Ni wrote:
> > exec/java-exec/src/main/codegen/templates/RegrTypeFunctions.java, line 103
> > <https://reviews.apache.org/r/25012/diff/2/?file=667996#file667996line103>
> >
> > In Oracle's specification, regr_avgx and regr_avgy should discard all
> > pairs where either x or y is null.
> >
> > "Oracle applies the function to the set of (expr1, expr2) pairs after
> > eliminating all pairs for which either expr1 or expr2 is null. ..."
>
> Jinfeng Ni wrote:
> Maybe I miss something. I still see the code just check one input, not
> the other input, for nullability.
THis is the check, isn't this what you meant:
<#if type.inputType?starts_with("Nullable")>
sout: {
if (${method.primary}In.isSet == 0 || ${method.dependent}In.isSet ==
0) {
// Skip null values for Nullable types
break sout;
}
</#if>
> 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.
>
> Yash Sharma wrote:
> Actually wanted to test against more records. Have moved to smaller
> testcase. Enhanced data with broader test scenarios.
>
> Jinfeng Ni wrote:
> You use JSON format as data source. The columns from JSON file will be
> processed as nullable columns. This means the tests will test :
> regr_avgx( NullableBitInt, NullableBigInt).
>
> Can you add test with regr_avgx(BitInt, NullableBigInt)?
>
> My guess the code might be hit function resolution error for this new
> case. In your code, you only have function implementation when both sides are
> non-nullable, or both sides are nulllable. But the mixed cases do not have
> implementation. I'm not sure if impliciat cast will kick in, for the
> aggregation function.
Will this testcase reproduce similar scenario:
{
"ref" : "`EXPR$3`",
"expr" : "regr_avgx(`C`, `F`) "
}
Here 'C' has all non-null cols & 'F' has mixed null/non-null cols.
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25012/#review51602
-----------------------------------------------------------
On Aug. 28, 2014, 5:13 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25012/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 5:13 p.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
>
>
