> On June 26, 2012, 10:14 p.m., Jonathan Coveney wrote:
> > /trunk/src/org/apache/pig/scripting/groovy/GroovyScriptEngine.java, line 195
> > <https://reviews.apache.org/r/5591/diff/1/?file=116559#file116559line195>
> >
> >     It seems weird to allow Groovy static methods as UDFs. I suppose there 
> > is no harm in it, but given that in Pig all UDF's imply that they are 
> > instantiated, it proposes a potential strong departure from how people 
> > typically should think about UDF's.
> 
> Mathias Herberts wrote:
>     As stated earlier, a Groovy class should really be seen as a container 
> for multiple UDFs, not as containing a single one.
>     
>     Non static methods are needed for Accumulator UDFs, all other UDFs 
> maintain no state, thus the use of static methods. I guess non static methods 
> could be supported too.
> 
> Julien Le Dem wrote:
>     For stateless methods that don't need initialization, static methods are 
> easier. We should allow both

I added support of both in PIG-2763-3.patch


> On June 26, 2012, 10:14 p.m., Jonathan Coveney wrote:
> > /trunk/src/org/apache/pig/scripting/groovy/GroovyUtils.java, line 271
> > <https://reviews.apache.org/r/5591/diff/1/?file=116560#file116560line271>
> >
> >     I don't know if it should throw away the exception like this.
> 
> Mathias Herberts wrote:
>     What would you recommend? Throwing RuntimeException?
> 
> Julien Le Dem wrote:
>     yes. And chain the cause

Done in PIG-2763-3.patch


- Mathias


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


On June 26, 2012, 11:26 p.m., Mathias Herberts wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5591/
> -----------------------------------------------------------
> 
> (Updated June 26, 2012, 11:26 p.m.)
> 
> 
> Review request for pig, Julien Le Dem and Jonathan Coveney.
> 
> 
> Description
> -------
> 
> Adds support for Groovy UDFs in Pig.
> 
> 
> This addresses bug PIG-2763.
>     https://issues.apache.org/jira/browse/PIG-2763
> 
> 
> Diffs
> -----
> 
>   /trunk/ivy.xml 1353307 
>   /trunk/ivy/libraries.properties 1353307 
>   /trunk/src/org/apache/pig/scripting/ScriptEngine.java 1354285 
>   /trunk/src/org/apache/pig/scripting/groovy/AccumulatorAccumulate.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/AccumulatorCleanup.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/AccumulatorGetValue.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/AlgebraicFinal.java PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/AlgebraicInitial.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/AlgebraicIntermed.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/GroovyAccumulatorEvalFunc.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/GroovyAlgebraicEvalFunc.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/GroovyEvalFunc.java PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/GroovyEvalFuncObject.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/GroovyScriptEngine.java 
> PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/GroovyUtils.java PRE-CREATION 
>   /trunk/src/org/apache/pig/scripting/groovy/OutputSchemaFunction.java 
> PRE-CREATION 
>   /trunk/test/org/apache/pig/test/TestUDFGroovy.java PRE-CREATION 
>   /trunk/test/unit-tests 1353307 
> 
> Diff: https://reviews.apache.org/r/5591/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mathias Herberts
> 
>

Reply via email to