You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Mehant Baid <ba...@gmail.com> on 2015/05/10 01:34:20 UTC
Review Request 34022: DRILL-2870: Fix return type of aggregate
functions to be nullable (part 2)
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34022/
-----------------------------------------------------------
Review request for drill and Aman Sinha.
Repository: drill-git
Description
-------
This patch modifies aggregate functions so when we perform an aggregate (other than count) on an empty set we get null (for both required and optional input types).
Tdd file changes:
Modified tdd files so that the output type of aggregate functions is optional
Template file changes:
Maintain a nonNullCount to indiciate if the output should be null or not.
Diffs
-----
exec/java-exec/src/main/codegen/data/AggrBitwiseLogicalTypes.tdd 2b72abd
exec/java-exec/src/main/codegen/data/AggrTypes1.tdd 8952417
exec/java-exec/src/main/codegen/data/AggrTypes2.tdd ee64daf
exec/java-exec/src/main/codegen/data/AggrTypes3.tdd 0c3a358
exec/java-exec/src/main/codegen/templates/AggrBitwiseLogicalTypeFunctions.java b159421
exec/java-exec/src/main/codegen/templates/AggrTypeFunctions1.java 19a6d46
exec/java-exec/src/main/codegen/templates/AggrTypeFunctions2.java 6701f09
exec/java-exec/src/main/codegen/templates/AggrTypeFunctions3.java c005446
exec/java-exec/src/main/codegen/templates/DateIntervalAggrFunctions1.java e934167
exec/java-exec/src/main/codegen/templates/IntervalAggrFunctions2.java b29fa08
exec/java-exec/src/main/codegen/templates/VarCharAggrFunctions1.java 53474ea
exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java 01db7c2
exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestAgg.java b39566a
exec/java-exec/src/test/resources/parquet/alltypes_required.parquet PRE-CREATION
Diff: https://reviews.apache.org/r/34022/diff/
Testing
-------
Added unit tests.
Thanks,
Mehant Baid
Re: Review Request 34022: DRILL-2870: Fix return type of aggregate
functions to be nullable (part 2)
Posted by Mehant Baid <ba...@gmail.com>.
> On May 10, 2015, 12:15 a.m., Aman Sinha wrote:
> > exec/java-exec/src/main/codegen/templates/AggrTypeFunctions3.java, line 93
> > <https://reviews.apache.org/r/34022/diff/1/?file=954683#file954683line93>
> >
> > I can see the reason but want to clarify: previously we were incrementing the nonNullCount, now we are setting it to 1 since we only care about the presence of at least 1 non-null value, right ?
Yes that is correct
- Mehant
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34022/#review83166
-----------------------------------------------------------
On May 9, 2015, 11:34 p.m., Mehant Baid wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34022/
> -----------------------------------------------------------
>
> (Updated May 9, 2015, 11:34 p.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch modifies aggregate functions so when we perform an aggregate (other than count) on an empty set we get null (for both required and optional input types).
>
> Tdd file changes:
> Modified tdd files so that the output type of aggregate functions is optional
>
> Template file changes:
> Maintain a nonNullCount to indiciate if the output should be null or not.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/data/AggrBitwiseLogicalTypes.tdd 2b72abd
> exec/java-exec/src/main/codegen/data/AggrTypes1.tdd 8952417
> exec/java-exec/src/main/codegen/data/AggrTypes2.tdd ee64daf
> exec/java-exec/src/main/codegen/data/AggrTypes3.tdd 0c3a358
> exec/java-exec/src/main/codegen/templates/AggrBitwiseLogicalTypeFunctions.java b159421
> exec/java-exec/src/main/codegen/templates/AggrTypeFunctions1.java 19a6d46
> exec/java-exec/src/main/codegen/templates/AggrTypeFunctions2.java 6701f09
> exec/java-exec/src/main/codegen/templates/AggrTypeFunctions3.java c005446
> exec/java-exec/src/main/codegen/templates/DateIntervalAggrFunctions1.java e934167
> exec/java-exec/src/main/codegen/templates/IntervalAggrFunctions2.java b29fa08
> exec/java-exec/src/main/codegen/templates/VarCharAggrFunctions1.java 53474ea
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java 01db7c2
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestAgg.java b39566a
> exec/java-exec/src/test/resources/parquet/alltypes_required.parquet PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34022/diff/
>
>
> Testing
> -------
>
> Added unit tests.
>
>
> Thanks,
>
> Mehant Baid
>
>
Re: Review Request 34022: DRILL-2870: Fix return type of aggregate
functions to be nullable (part 2)
Posted by Aman Sinha <as...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34022/#review83166
-----------------------------------------------------------
Ship it!
It would be good to get a performance run with these changes.
exec/java-exec/src/main/codegen/templates/AggrTypeFunctions3.java
<https://reviews.apache.org/r/34022/#comment134044>
I can see the reason but want to clarify: previously we were incrementing the nonNullCount, now we are setting it to 1 since we only care about the presence of at least 1 non-null value, right ?
- Aman Sinha
On May 9, 2015, 11:34 p.m., Mehant Baid wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34022/
> -----------------------------------------------------------
>
> (Updated May 9, 2015, 11:34 p.m.)
>
>
> Review request for drill and Aman Sinha.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> This patch modifies aggregate functions so when we perform an aggregate (other than count) on an empty set we get null (for both required and optional input types).
>
> Tdd file changes:
> Modified tdd files so that the output type of aggregate functions is optional
>
> Template file changes:
> Maintain a nonNullCount to indiciate if the output should be null or not.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/data/AggrBitwiseLogicalTypes.tdd 2b72abd
> exec/java-exec/src/main/codegen/data/AggrTypes1.tdd 8952417
> exec/java-exec/src/main/codegen/data/AggrTypes2.tdd ee64daf
> exec/java-exec/src/main/codegen/data/AggrTypes3.tdd 0c3a358
> exec/java-exec/src/main/codegen/templates/AggrBitwiseLogicalTypeFunctions.java b159421
> exec/java-exec/src/main/codegen/templates/AggrTypeFunctions1.java 19a6d46
> exec/java-exec/src/main/codegen/templates/AggrTypeFunctions2.java 6701f09
> exec/java-exec/src/main/codegen/templates/AggrTypeFunctions3.java c005446
> exec/java-exec/src/main/codegen/templates/DateIntervalAggrFunctions1.java e934167
> exec/java-exec/src/main/codegen/templates/IntervalAggrFunctions2.java b29fa08
> exec/java-exec/src/main/codegen/templates/VarCharAggrFunctions1.java 53474ea
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestAggregateFunctions.java 01db7c2
> exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/agg/TestAgg.java b39566a
> exec/java-exec/src/test/resources/parquet/alltypes_required.parquet PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34022/diff/
>
>
> Testing
> -------
>
> Added unit tests.
>
>
> Thanks,
>
> Mehant Baid
>
>