Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Drill, mail # dev - Re: Review Request 18372: Hive UDFs in Drill


Copy link to this message
-
Re: Review Request 18372: Hive UDFs in Drill
Jinfeng Ni 2014-03-22, 00:39

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

common/src/main/java/org/apache/drill/common/expression/ExpressionValidator.java
<https://reviews.apache.org/r/18372/#comment70295>

    On line 44, for AggregateChecker and ConstantChecker, when there is nested aggregated function call, you will throw IllegalArgumentException. This is different from the logic in method visitIfExpression, where error message is added into a ErrorCollector. Do you think we had better to make the behavior consistent in ExpressionValidator? If yes, we need pass in ErrorCollector to AggregateChecker.
    
    Similar for ConstantChecker on line 47.

common/src/main/java/org/apache/drill/common/expression/visitors/ConstantChecker.java
<https://reviews.apache.org/r/18372/#comment70297>

    I feel the logic here is not right. You only check whether the i-th argument is constant when argConstantOnly(i) is true. If not, then you did not check the argument at all, but you will still return "true".
    
    For example,   1 + col seems to return true from this method.
    
- Jinfeng Ni
On March 20, 2014, 10:04 a.m., Venki Korukanti wrote: