All the points have been nicely addressed by Jinfeng and Mehant. I took the luxury of writing some code around it - since it has come up couple of times now.
@Jinfeng/Mehant/All: I have few open questions though- - What should be the ValueHolder type for the @Varargs types. Can we leverage FeildReader to use RepeatedList or something - Should we restrict all the parameters in varargs to be of same type - Should we restrict the number of varargs user passes in the functtion Let me know if the initial partial wire-frame looks okay. Jira: https://issues.apache.org/jira/browse/DRILL-1415 Review board for wire-frame : https://reviews.apache.org/r/25601 Regards On Sat, Sep 13, 2014 at 12:01 AM, Matthew Burgess <[email protected]> wrote: > Ah, exactly what I was looking for, thanks! > > Seems like if we knew the arity of the DrillFunc at rewrite time, we could > use the left/right associativity (from call.getOperator().getLeftPrec() and > getRightPrec()) to do generic associative rewrites, perhaps as another > else-if clause in DrillOptiq.getDrillFunctionFromOptiqCall(), rather than > the logic being built into an else-if block for each operator that supports > it. There's a TODO in there that mentions rewrite patterns, this might be > one of them :) > > As an example for custom binary operators, how about an "apply" operator > that takes a string param and two Freemarker-generated params (with the > same > type as @Output)? Then we can do a similar rewrite (using concat as the > example): > > apply('concat', 'Hello', ' ', 'World', '!') = concat('Hello', concat(' ', > concat('World', '!'))) or concat(concat(concat('Hello',' '), 'World'), '!') > depending on associativity > > This could be used by custom functions, whose domain and codomain types > match, and are associative. If this is not generally useful, what about a > hook in the rewrite phase, such that a custom Drill Function could also > perform its own rewrite? > > In any case, I'm having lots of fun working with Drill, I hope to be able > to > contribute something soon! > > Regards, > Matt > > From: Mehant Baid <[email protected]> > Reply-To: <[email protected]> > Date: Friday, September 12, 2014 at 1:56 PM > To: <[email protected]> > Subject: Re: Varargs in DrillSimpleFunc > > The implementation of concat has only two arguments and its fixed. We > rewrite the concat function in DrillOptiq > < > https://github.com/apache/incubator-drill/blob/master/exec/java-exec/src/ma > in/java/org/apache/drill/exec/planner/logical/DrillOptiq.java#L357> > class to be stacked on top of each other so that it uses only two > arguments. > > Eg: concat('a', 'b', 'c') ==> concat(concat('a', 'b'), 'c') > > Thanks > Mehant > > > On 9/12/14, 10:50 AM, Matt Burgess wrote: > > Thanks all for the great info, I will look into the Complex type stuff. > > > > I'm still confused about concatenation though, it seems that concat is > > defined having two parameters but I can call it on more than that (I > tried up > > to 5). How does that work, is it generated by Freemarker or something to > > support different (but discrete) numbers of params? > > > > Thanks again, > > Matt > > > > > >> On Sep 12, 2014, at 1:33 PM, Jinfeng Ni <[email protected]> wrote: > >> > >> VarArgs is currently not supported. To add support of VarArgs, things > >> might require to be changed: > >> > >> 1. In function template, a new annotation would be added to mark > parameter > >> as VarArgs. > >> 2. In FunctionConverter, need recognize this new annotation, and > build the > >> function holder accordingly. > >> 3. FunctionResolver need add logic to handle VarArgs during function > >> resolution. > >> 4. The run-time code generation for VarArgs also would require change. > >> > >> You may check with Yash, since he might have more thoughts about > VarArgs > >> support. > >> > >> If you want to have a function which takes an array or map, you may > want to > >> consider using complex type's FieldReader / ComplexWriter interface, > just > >> like Ted suggested. (Take a look at JsonConvertTo /JsonConvertFrom as > >> example). > >> > >> > >>> On Fri, Sep 12, 2014 at 8:15 AM, Ted Dunning <[email protected]> > wrote: > >>> > >>>> On Fri, Sep 12, 2014 at 6:31 AM, Matt Burgess <[email protected]> > wrote: > >>>> > >>>> SELECT transform("concatTab", *) from dfs.`my_file.csv` > >>>> > >>>> the intent here is that a result set is returned that basically > changes a > >>>> CSV file to a TSV result set. > >>>> > >>> This should be done at a higher level than the SQL itself. > >>> > > > > >
