Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Hi All,
I am planning to add new drill functions to optiq syntax for parsing.
We have new functions like regr_avgx/regr_avgy etc and existing ones like
covar_samp,covar_pop, correlation/corr etc.

Going through this old thread[1] I see that we need to add the definitions
in the CombinedParser.jj[2] in Optiq.

Is that the way we still use or its handled differently now ?

Thanks.

1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
2.
https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

I would like to take a stab at it.
@Venki/Julian - Is there any existing example where something similar is
done and I can explore it.
Will anyways be exploring the Parser.tdd and CombinedParser.jj .

Thanks.

On Wed, Sep 24, 2014 at 12:37 AM, Julian Hyde <ju...@hydromatic.net> wrote:

> Feel free to log an optiq issue to add the necessary templatage. I think
> extension points in NonReservedKeyWord(),  NonReservedJdbcFunctionName()
> and ReservedFunctionName() are required. You could obsolete
> CommonNonReservedKeyWord() — inlining it into ReservedKeyWord() — while you
> are at it.
>
>
> On Sep 23, 2014, at 10:16 AM, Venki Korukanti <ve...@gmail.com>
> wrote:
>
> > On Tue, Sep 23, 2014 at 9:59 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> >> @Julian - I have sent the pull request. Thanks for handling the
> formatting
> >> and messed up order of keywords.
> >>
> >> @Venki - You were right. The JavaCC Error was occurring because the
> >> functions were already declared in CombinedParser.jj.
> >>
> >> Just another question - Would it be possible to add keywords to
> >> Registered/Non-Registered keywords via Drill directly? What changes
> would I
> >> have to do in the Parser.tdd.
> >>
> > Currently, I don't see a way to do that, but we can add a freemarker
> > variable in Optiq's CombinedParser.jj so that clients have the option to
> > add keywords to reserved function names or non-reserved keyword list.
> >
> >>
> >> Thanks.
> >>
> >>
> >> On Tue, Sep 23, 2014 at 9:31 PM, Yash Sharma <ya...@gmail.com> wrote:
> >>
> >>> @venki/julian: i have got a limited internet connectivity tonight. Will
> >> do
> >>> it first thing tomm.
> >>>
> >>> Thanks both.
> >>> On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:
> >>>
> >>>> It looks good. (You messed up the formatting in one place and added
> >>>> out-of-order to alphabetized lists in several places. I have fixed
> >> these.)
> >>>>
> >>>> Please submit a pull request for your branch and I will commit with my
> >>>> changes.
> >>>>
> >>>> Julian
> >>>>
> >>>> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
> >>>>
> >>>>> @Jacques- I would wait for Venki's input. Till then I have created a
> >>>> patch
> >>>>> for Optiq.
> >>>>>
> >>>>> @Julian/Optiq Dev: Could you please review the commit[1] if
> everything
> >>>>> looks good. Also I am not able to run the test case.
> >>>>> I am using:
> >>>>> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> >>>>> It is not able to recognize the test cases.
> >>>>> Works fine with -DfailIfNoTests=flase flag.
> >>>>>
> >>>>> 1:
> >>>>>
> >>>>
> >>
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> >>>>>
> >>>>> Thanks
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> >>>> wrote:
> >>>>>
> >>>>>> I believe that Drill also allows addition of reserved words through
> >>>> some of
> >>>>>> the freemarker inclusions but could be mistaken.  I think Venki
> could
> >>>>>> provide more input if you can't find the spot.
> >>>>>>
> >>>>>> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <
> julian@hydromatic.net
> >>>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com>
> >> wrote:
> >>>>>>>
> >>>>>>>> Am debugging Optiqand DrillOperatorTable.
> >>>>>>>>
> >>>>>>>> Strangely this works with backticks - as pointed out by Kryatal in
> >>>>>>>> DRILL-1441
> >>>>>>>> select `covar_pop`(employee_id, employee_id) FROM
> >> cp.`employee.json`
> >>>>>>> limit
> >>>>>>>> 10;
> >>>>>>>
> >>>>>>> See my comments on REPLACE in
> >>>>>>> https://issues.apache.org/jira/browse/DRILL-1441. The problems
> >>>> relate to
> >>>>>>> using a non-reserved keyword as an identifier.
> >>>>>>>
> >>>>>>> Regarding COVAR_POP. Probably something similar happening regarding
> >>>>>>> reserved words being. You should probably add it to
> >>>>>> ReservedFunctionName(),
> >>>>>>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
> >>>> Drill —
> >>>>>>> so I suggest you contribute an Optiq patch. Be sure to add tests to
> >>>>>>> SqlOperatorBaseTest along the lines of testVarPopFunc.
> >>>>>>>
> >>>>>>> Julian
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>
> >>>>
> >>
>
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

I would like to take a stab at it.
@Venki/Julian - Is there any existing example where something similar is
done and I can explore it.
Will anyways be exploring the Parser.tdd and CombinedParser.jj .

Thanks.

On Wed, Sep 24, 2014 at 12:37 AM, Julian Hyde <ju...@hydromatic.net> wrote:

> Feel free to log an optiq issue to add the necessary templatage. I think
> extension points in NonReservedKeyWord(),  NonReservedJdbcFunctionName()
> and ReservedFunctionName() are required. You could obsolete
> CommonNonReservedKeyWord() — inlining it into ReservedKeyWord() — while you
> are at it.
>
>
> On Sep 23, 2014, at 10:16 AM, Venki Korukanti <ve...@gmail.com>
> wrote:
>
> > On Tue, Sep 23, 2014 at 9:59 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> >> @Julian - I have sent the pull request. Thanks for handling the
> formatting
> >> and messed up order of keywords.
> >>
> >> @Venki - You were right. The JavaCC Error was occurring because the
> >> functions were already declared in CombinedParser.jj.
> >>
> >> Just another question - Would it be possible to add keywords to
> >> Registered/Non-Registered keywords via Drill directly? What changes
> would I
> >> have to do in the Parser.tdd.
> >>
> > Currently, I don't see a way to do that, but we can add a freemarker
> > variable in Optiq's CombinedParser.jj so that clients have the option to
> > add keywords to reserved function names or non-reserved keyword list.
> >
> >>
> >> Thanks.
> >>
> >>
> >> On Tue, Sep 23, 2014 at 9:31 PM, Yash Sharma <ya...@gmail.com> wrote:
> >>
> >>> @venki/julian: i have got a limited internet connectivity tonight. Will
> >> do
> >>> it first thing tomm.
> >>>
> >>> Thanks both.
> >>> On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:
> >>>
> >>>> It looks good. (You messed up the formatting in one place and added
> >>>> out-of-order to alphabetized lists in several places. I have fixed
> >> these.)
> >>>>
> >>>> Please submit a pull request for your branch and I will commit with my
> >>>> changes.
> >>>>
> >>>> Julian
> >>>>
> >>>> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
> >>>>
> >>>>> @Jacques- I would wait for Venki's input. Till then I have created a
> >>>> patch
> >>>>> for Optiq.
> >>>>>
> >>>>> @Julian/Optiq Dev: Could you please review the commit[1] if
> everything
> >>>>> looks good. Also I am not able to run the test case.
> >>>>> I am using:
> >>>>> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> >>>>> It is not able to recognize the test cases.
> >>>>> Works fine with -DfailIfNoTests=flase flag.
> >>>>>
> >>>>> 1:
> >>>>>
> >>>>
> >>
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> >>>>>
> >>>>> Thanks
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> >>>> wrote:
> >>>>>
> >>>>>> I believe that Drill also allows addition of reserved words through
> >>>> some of
> >>>>>> the freemarker inclusions but could be mistaken.  I think Venki
> could
> >>>>>> provide more input if you can't find the spot.
> >>>>>>
> >>>>>> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <
> julian@hydromatic.net
> >>>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com>
> >> wrote:
> >>>>>>>
> >>>>>>>> Am debugging Optiqand DrillOperatorTable.
> >>>>>>>>
> >>>>>>>> Strangely this works with backticks - as pointed out by Kryatal in
> >>>>>>>> DRILL-1441
> >>>>>>>> select `covar_pop`(employee_id, employee_id) FROM
> >> cp.`employee.json`
> >>>>>>> limit
> >>>>>>>> 10;
> >>>>>>>
> >>>>>>> See my comments on REPLACE in
> >>>>>>> https://issues.apache.org/jira/browse/DRILL-1441. The problems
> >>>> relate to
> >>>>>>> using a non-reserved keyword as an identifier.
> >>>>>>>
> >>>>>>> Regarding COVAR_POP. Probably something similar happening regarding
> >>>>>>> reserved words being. You should probably add it to
> >>>>>> ReservedFunctionName(),
> >>>>>>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
> >>>> Drill —
> >>>>>>> so I suggest you contribute an Optiq patch. Be sure to add tests to
> >>>>>>> SqlOperatorBaseTest along the lines of testVarPopFunc.
> >>>>>>>
> >>>>>>> Julian
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>
> >>>>
> >>
>
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@hydromatic.net>]

Feel free to log an optiq issue to add the necessary templatage. I think extension points in NonReservedKeyWord(),  NonReservedJdbcFunctionName() and ReservedFunctionName() are required. You could obsolete CommonNonReservedKeyWord() — inlining it into ReservedKeyWord() — while you are at it.


On Sep 23, 2014, at 10:16 AM, Venki Korukanti <ve...@gmail.com> wrote:

> On Tue, Sep 23, 2014 at 9:59 AM, Yash Sharma <ya...@gmail.com> wrote:
> 
>> @Julian - I have sent the pull request. Thanks for handling the formatting
>> and messed up order of keywords.
>> 
>> @Venki - You were right. The JavaCC Error was occurring because the
>> functions were already declared in CombinedParser.jj.
>> 
>> Just another question - Would it be possible to add keywords to
>> Registered/Non-Registered keywords via Drill directly? What changes would I
>> have to do in the Parser.tdd.
>> 
> Currently, I don't see a way to do that, but we can add a freemarker
> variable in Optiq's CombinedParser.jj so that clients have the option to
> add keywords to reserved function names or non-reserved keyword list.
> 
>> 
>> Thanks.
>> 
>> 
>> On Tue, Sep 23, 2014 at 9:31 PM, Yash Sharma <ya...@gmail.com> wrote:
>> 
>>> @venki/julian: i have got a limited internet connectivity tonight. Will
>> do
>>> it first thing tomm.
>>> 
>>> Thanks both.
>>> On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:
>>> 
>>>> It looks good. (You messed up the formatting in one place and added
>>>> out-of-order to alphabetized lists in several places. I have fixed
>> these.)
>>>> 
>>>> Please submit a pull request for your branch and I will commit with my
>>>> changes.
>>>> 
>>>> Julian
>>>> 
>>>> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>>>> 
>>>>> @Jacques- I would wait for Venki's input. Till then I have created a
>>>> patch
>>>>> for Optiq.
>>>>> 
>>>>> @Julian/Optiq Dev: Could you please review the commit[1] if everything
>>>>> looks good. Also I am not able to run the test case.
>>>>> I am using:
>>>>> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
>>>>> It is not able to recognize the test cases.
>>>>> Works fine with -DfailIfNoTests=flase flag.
>>>>> 
>>>>> 1:
>>>>> 
>>>> 
>> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
>>>>> 
>>>>> Thanks
>>>>> 
>>>>> 
>>>>> 
>>>>> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
>>>> wrote:
>>>>> 
>>>>>> I believe that Drill also allows addition of reserved words through
>>>> some of
>>>>>> the freemarker inclusions but could be mistaken.  I think Venki could
>>>>>> provide more input if you can't find the spot.
>>>>>> 
>>>>>> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <julian@hydromatic.net
>>> 
>>>>>> wrote:
>>>>>> 
>>>>>>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com>
>> wrote:
>>>>>>> 
>>>>>>>> Am debugging Optiqand DrillOperatorTable.
>>>>>>>> 
>>>>>>>> Strangely this works with backticks - as pointed out by Kryatal in
>>>>>>>> DRILL-1441
>>>>>>>> select `covar_pop`(employee_id, employee_id) FROM
>> cp.`employee.json`
>>>>>>> limit
>>>>>>>> 10;
>>>>>>> 
>>>>>>> See my comments on REPLACE in
>>>>>>> https://issues.apache.org/jira/browse/DRILL-1441. The problems
>>>> relate to
>>>>>>> using a non-reserved keyword as an identifier.
>>>>>>> 
>>>>>>> Regarding COVAR_POP. Probably something similar happening regarding
>>>>>>> reserved words being. You should probably add it to
>>>>>> ReservedFunctionName(),
>>>>>>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
>>>> Drill —
>>>>>>> so I suggest you contribute an Optiq patch. Be sure to add tests to
>>>>>>> SqlOperatorBaseTest along the lines of testVarPopFunc.
>>>>>>> 
>>>>>>> Julian
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>> 
>>>> 
>> 

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@hydromatic.net>]

Feel free to log an optiq issue to add the necessary templatage. I think extension points in NonReservedKeyWord(),  NonReservedJdbcFunctionName() and ReservedFunctionName() are required. You could obsolete CommonNonReservedKeyWord() — inlining it into ReservedKeyWord() — while you are at it.


On Sep 23, 2014, at 10:16 AM, Venki Korukanti <ve...@gmail.com> wrote:

> On Tue, Sep 23, 2014 at 9:59 AM, Yash Sharma <ya...@gmail.com> wrote:
> 
>> @Julian - I have sent the pull request. Thanks for handling the formatting
>> and messed up order of keywords.
>> 
>> @Venki - You were right. The JavaCC Error was occurring because the
>> functions were already declared in CombinedParser.jj.
>> 
>> Just another question - Would it be possible to add keywords to
>> Registered/Non-Registered keywords via Drill directly? What changes would I
>> have to do in the Parser.tdd.
>> 
> Currently, I don't see a way to do that, but we can add a freemarker
> variable in Optiq's CombinedParser.jj so that clients have the option to
> add keywords to reserved function names or non-reserved keyword list.
> 
>> 
>> Thanks.
>> 
>> 
>> On Tue, Sep 23, 2014 at 9:31 PM, Yash Sharma <ya...@gmail.com> wrote:
>> 
>>> @venki/julian: i have got a limited internet connectivity tonight. Will
>> do
>>> it first thing tomm.
>>> 
>>> Thanks both.
>>> On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:
>>> 
>>>> It looks good. (You messed up the formatting in one place and added
>>>> out-of-order to alphabetized lists in several places. I have fixed
>> these.)
>>>> 
>>>> Please submit a pull request for your branch and I will commit with my
>>>> changes.
>>>> 
>>>> Julian
>>>> 
>>>> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>>>> 
>>>>> @Jacques- I would wait for Venki's input. Till then I have created a
>>>> patch
>>>>> for Optiq.
>>>>> 
>>>>> @Julian/Optiq Dev: Could you please review the commit[1] if everything
>>>>> looks good. Also I am not able to run the test case.
>>>>> I am using:
>>>>> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
>>>>> It is not able to recognize the test cases.
>>>>> Works fine with -DfailIfNoTests=flase flag.
>>>>> 
>>>>> 1:
>>>>> 
>>>> 
>> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
>>>>> 
>>>>> Thanks
>>>>> 
>>>>> 
>>>>> 
>>>>> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
>>>> wrote:
>>>>> 
>>>>>> I believe that Drill also allows addition of reserved words through
>>>> some of
>>>>>> the freemarker inclusions but could be mistaken.  I think Venki could
>>>>>> provide more input if you can't find the spot.
>>>>>> 
>>>>>> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <julian@hydromatic.net
>>> 
>>>>>> wrote:
>>>>>> 
>>>>>>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com>
>> wrote:
>>>>>>> 
>>>>>>>> Am debugging Optiqand DrillOperatorTable.
>>>>>>>> 
>>>>>>>> Strangely this works with backticks - as pointed out by Kryatal in
>>>>>>>> DRILL-1441
>>>>>>>> select `covar_pop`(employee_id, employee_id) FROM
>> cp.`employee.json`
>>>>>>> limit
>>>>>>>> 10;
>>>>>>> 
>>>>>>> See my comments on REPLACE in
>>>>>>> https://issues.apache.org/jira/browse/DRILL-1441. The problems
>>>> relate to
>>>>>>> using a non-reserved keyword as an identifier.
>>>>>>> 
>>>>>>> Regarding COVAR_POP. Probably something similar happening regarding
>>>>>>> reserved words being. You should probably add it to
>>>>>> ReservedFunctionName(),
>>>>>>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
>>>> Drill —
>>>>>>> so I suggest you contribute an Optiq patch. Be sure to add tests to
>>>>>>> SqlOperatorBaseTest along the lines of testVarPopFunc.
>>>>>>> 
>>>>>>> Julian
>>>>>>> 
>>>>>>> 
>>>>>> 
>>>> 
>>>> 
>> 

Re: Adding new drill functions to optiq syntax

[Venki Korukanti <ve...@gmail.com>]

On Tue, Sep 23, 2014 at 9:59 AM, Yash Sharma <ya...@gmail.com> wrote:

> @Julian - I have sent the pull request. Thanks for handling the formatting
> and messed up order of keywords.
>
> @Venki - You were right. The JavaCC Error was occurring because the
> functions were already declared in CombinedParser.jj.
>
> Just another question - Would it be possible to add keywords to
> Registered/Non-Registered keywords via Drill directly? What changes would I
> have to do in the Parser.tdd.
>
Currently, I don't see a way to do that, but we can add a freemarker
variable in Optiq's CombinedParser.jj so that clients have the option to
add keywords to reserved function names or non-reserved keyword list.

>
> Thanks.
>
>
> On Tue, Sep 23, 2014 at 9:31 PM, Yash Sharma <ya...@gmail.com> wrote:
>
> > @venki/julian: i have got a limited internet connectivity tonight. Will
> do
> > it first thing tomm.
> >
> > Thanks both.
> > On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:
> >
> >> It looks good. (You messed up the formatting in one place and added
> >> out-of-order to alphabetized lists in several places. I have fixed
> these.)
> >>
> >> Please submit a pull request for your branch and I will commit with my
> >> changes.
> >>
> >> Julian
> >>
> >> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
> >>
> >> > @Jacques- I would wait for Venki's input. Till then I have created a
> >> patch
> >> > for Optiq.
> >> >
> >> > @Julian/Optiq Dev: Could you please review the commit[1] if everything
> >> > looks good. Also I am not able to run the test case.
> >> > I am using:
> >> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> >> > It is not able to recognize the test cases.
> >> > Works fine with -DfailIfNoTests=flase flag.
> >> >
> >> > 1:
> >> >
> >>
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> >> >
> >> > Thanks
> >> >
> >> >
> >> >
> >> > On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> >> wrote:
> >> >
> >> >> I believe that Drill also allows addition of reserved words through
> >> some of
> >> >> the freemarker inclusions but could be mistaken.  I think Venki could
> >> >> provide more input if you can't find the spot.
> >> >>
> >> >> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <julian@hydromatic.net
> >
> >> >> wrote:
> >> >>
> >> >>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com>
> wrote:
> >> >>>
> >> >>>> Am debugging Optiqand DrillOperatorTable.
> >> >>>>
> >> >>>> Strangely this works with backticks - as pointed out by Kryatal in
> >> >>>> DRILL-1441
> >> >>>> select `covar_pop`(employee_id, employee_id) FROM
> cp.`employee.json`
> >> >>> limit
> >> >>>> 10;
> >> >>>
> >> >>> See my comments on REPLACE in
> >> >>> https://issues.apache.org/jira/browse/DRILL-1441. The problems
> >> relate to
> >> >>> using a non-reserved keyword as an identifier.
> >> >>>
> >> >>> Regarding COVAR_POP. Probably something similar happening regarding
> >> >>> reserved words being. You should probably add it to
> >> >> ReservedFunctionName(),
> >> >>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
> >> Drill —
> >> >>> so I suggest you contribute an Optiq patch. Be sure to add tests to
> >> >>> SqlOperatorBaseTest along the lines of testVarPopFunc.
> >> >>>
> >> >>> Julian
> >> >>>
> >> >>>
> >> >>
> >>
> >>
>

Re: Adding new drill functions to optiq syntax

[Venki Korukanti <ve...@gmail.com>]

On Tue, Sep 23, 2014 at 9:59 AM, Yash Sharma <ya...@gmail.com> wrote:

> @Julian - I have sent the pull request. Thanks for handling the formatting
> and messed up order of keywords.
>
> @Venki - You were right. The JavaCC Error was occurring because the
> functions were already declared in CombinedParser.jj.
>
> Just another question - Would it be possible to add keywords to
> Registered/Non-Registered keywords via Drill directly? What changes would I
> have to do in the Parser.tdd.
>
Currently, I don't see a way to do that, but we can add a freemarker
variable in Optiq's CombinedParser.jj so that clients have the option to
add keywords to reserved function names or non-reserved keyword list.

>
> Thanks.
>
>
> On Tue, Sep 23, 2014 at 9:31 PM, Yash Sharma <ya...@gmail.com> wrote:
>
> > @venki/julian: i have got a limited internet connectivity tonight. Will
> do
> > it first thing tomm.
> >
> > Thanks both.
> > On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:
> >
> >> It looks good. (You messed up the formatting in one place and added
> >> out-of-order to alphabetized lists in several places. I have fixed
> these.)
> >>
> >> Please submit a pull request for your branch and I will commit with my
> >> changes.
> >>
> >> Julian
> >>
> >> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
> >>
> >> > @Jacques- I would wait for Venki's input. Till then I have created a
> >> patch
> >> > for Optiq.
> >> >
> >> > @Julian/Optiq Dev: Could you please review the commit[1] if everything
> >> > looks good. Also I am not able to run the test case.
> >> > I am using:
> >> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> >> > It is not able to recognize the test cases.
> >> > Works fine with -DfailIfNoTests=flase flag.
> >> >
> >> > 1:
> >> >
> >>
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> >> >
> >> > Thanks
> >> >
> >> >
> >> >
> >> > On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> >> wrote:
> >> >
> >> >> I believe that Drill also allows addition of reserved words through
> >> some of
> >> >> the freemarker inclusions but could be mistaken.  I think Venki could
> >> >> provide more input if you can't find the spot.
> >> >>
> >> >> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <julian@hydromatic.net
> >
> >> >> wrote:
> >> >>
> >> >>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com>
> wrote:
> >> >>>
> >> >>>> Am debugging Optiqand DrillOperatorTable.
> >> >>>>
> >> >>>> Strangely this works with backticks - as pointed out by Kryatal in
> >> >>>> DRILL-1441
> >> >>>> select `covar_pop`(employee_id, employee_id) FROM
> cp.`employee.json`
> >> >>> limit
> >> >>>> 10;
> >> >>>
> >> >>> See my comments on REPLACE in
> >> >>> https://issues.apache.org/jira/browse/DRILL-1441. The problems
> >> relate to
> >> >>> using a non-reserved keyword as an identifier.
> >> >>>
> >> >>> Regarding COVAR_POP. Probably something similar happening regarding
> >> >>> reserved words being. You should probably add it to
> >> >> ReservedFunctionName(),
> >> >>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
> >> Drill —
> >> >>> so I suggest you contribute an Optiq patch. Be sure to add tests to
> >> >>> SqlOperatorBaseTest along the lines of testVarPopFunc.
> >> >>>
> >> >>> Julian
> >> >>>
> >> >>>
> >> >>
> >>
> >>
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

@Julian - I have sent the pull request. Thanks for handling the formatting
and messed up order of keywords.

@Venki - You were right. The JavaCC Error was occurring because the
functions were already declared in CombinedParser.jj.

Just another question - Would it be possible to add keywords to
Registered/Non-Registered keywords via Drill directly? What changes would I
have to do in the Parser.tdd.

Thanks.


On Tue, Sep 23, 2014 at 9:31 PM, Yash Sharma <ya...@gmail.com> wrote:

> @venki/julian: i have got a limited internet connectivity tonight. Will do
> it first thing tomm.
>
> Thanks both.
> On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:
>
>> It looks good. (You messed up the formatting in one place and added
>> out-of-order to alphabetized lists in several places. I have fixed these.)
>>
>> Please submit a pull request for your branch and I will commit with my
>> changes.
>>
>> Julian
>>
>> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>>
>> > @Jacques- I would wait for Venki's input. Till then I have created a
>> patch
>> > for Optiq.
>> >
>> > @Julian/Optiq Dev: Could you please review the commit[1] if everything
>> > looks good. Also I am not able to run the test case.
>> > I am using:
>> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
>> > It is not able to recognize the test cases.
>> > Works fine with -DfailIfNoTests=flase flag.
>> >
>> > 1:
>> >
>> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
>> >
>> > Thanks
>> >
>> >
>> >
>> > On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
>> wrote:
>> >
>> >> I believe that Drill also allows addition of reserved words through
>> some of
>> >> the freemarker inclusions but could be mistaken.  I think Venki could
>> >> provide more input if you can't find the spot.
>> >>
>> >> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
>> >> wrote:
>> >>
>> >>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>> >>>
>> >>>> Am debugging Optiqand DrillOperatorTable.
>> >>>>
>> >>>> Strangely this works with backticks - as pointed out by Kryatal in
>> >>>> DRILL-1441
>> >>>> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
>> >>> limit
>> >>>> 10;
>> >>>
>> >>> See my comments on REPLACE in
>> >>> https://issues.apache.org/jira/browse/DRILL-1441. The problems
>> relate to
>> >>> using a non-reserved keyword as an identifier.
>> >>>
>> >>> Regarding COVAR_POP. Probably something similar happening regarding
>> >>> reserved words being. You should probably add it to
>> >> ReservedFunctionName(),
>> >>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
>> Drill —
>> >>> so I suggest you contribute an Optiq patch. Be sure to add tests to
>> >>> SqlOperatorBaseTest along the lines of testVarPopFunc.
>> >>>
>> >>> Julian
>> >>>
>> >>>
>> >>
>>
>>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

@Julian - I have sent the pull request. Thanks for handling the formatting
and messed up order of keywords.

@Venki - You were right. The JavaCC Error was occurring because the
functions were already declared in CombinedParser.jj.

Just another question - Would it be possible to add keywords to
Registered/Non-Registered keywords via Drill directly? What changes would I
have to do in the Parser.tdd.

Thanks.


On Tue, Sep 23, 2014 at 9:31 PM, Yash Sharma <ya...@gmail.com> wrote:

> @venki/julian: i have got a limited internet connectivity tonight. Will do
> it first thing tomm.
>
> Thanks both.
> On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:
>
>> It looks good. (You messed up the formatting in one place and added
>> out-of-order to alphabetized lists in several places. I have fixed these.)
>>
>> Please submit a pull request for your branch and I will commit with my
>> changes.
>>
>> Julian
>>
>> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>>
>> > @Jacques- I would wait for Venki's input. Till then I have created a
>> patch
>> > for Optiq.
>> >
>> > @Julian/Optiq Dev: Could you please review the commit[1] if everything
>> > looks good. Also I am not able to run the test case.
>> > I am using:
>> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
>> > It is not able to recognize the test cases.
>> > Works fine with -DfailIfNoTests=flase flag.
>> >
>> > 1:
>> >
>> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
>> >
>> > Thanks
>> >
>> >
>> >
>> > On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
>> wrote:
>> >
>> >> I believe that Drill also allows addition of reserved words through
>> some of
>> >> the freemarker inclusions but could be mistaken.  I think Venki could
>> >> provide more input if you can't find the spot.
>> >>
>> >> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
>> >> wrote:
>> >>
>> >>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>> >>>
>> >>>> Am debugging Optiqand DrillOperatorTable.
>> >>>>
>> >>>> Strangely this works with backticks - as pointed out by Kryatal in
>> >>>> DRILL-1441
>> >>>> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
>> >>> limit
>> >>>> 10;
>> >>>
>> >>> See my comments on REPLACE in
>> >>> https://issues.apache.org/jira/browse/DRILL-1441. The problems
>> relate to
>> >>> using a non-reserved keyword as an identifier.
>> >>>
>> >>> Regarding COVAR_POP. Probably something similar happening regarding
>> >>> reserved words being. You should probably add it to
>> >> ReservedFunctionName(),
>> >>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
>> Drill —
>> >>> so I suggest you contribute an Optiq patch. Be sure to add tests to
>> >>> SqlOperatorBaseTest along the lines of testVarPopFunc.
>> >>>
>> >>> Julian
>> >>>
>> >>>
>> >>
>>
>>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

@venki/julian: i have got a limited internet connectivity tonight. Will do
it first thing tomm.

Thanks both.
On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:

> It looks good. (You messed up the formatting in one place and added
> out-of-order to alphabetized lists in several places. I have fixed these.)
>
> Please submit a pull request for your branch and I will commit with my
> changes.
>
> Julian
>
> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > @Jacques- I would wait for Venki's input. Till then I have created a
> patch
> > for Optiq.
> >
> > @Julian/Optiq Dev: Could you please review the commit[1] if everything
> > looks good. Also I am not able to run the test case.
> > I am using:
> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> > It is not able to recognize the test cases.
> > Works fine with -DfailIfNoTests=flase flag.
> >
> > 1:
> >
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> >
> > Thanks
> >
> >
> >
> > On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
> >
> >> I believe that Drill also allows addition of reserved words through
> some of
> >> the freemarker inclusions but could be mistaken.  I think Venki could
> >> provide more input if you can't find the spot.
> >>
> >> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
> >> wrote:
> >>
> >>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
> >>>
> >>>> Am debugging Optiqand DrillOperatorTable.
> >>>>
> >>>> Strangely this works with backticks - as pointed out by Kryatal in
> >>>> DRILL-1441
> >>>> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> >>> limit
> >>>> 10;
> >>>
> >>> See my comments on REPLACE in
> >>> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate
> to
> >>> using a non-reserved keyword as an identifier.
> >>>
> >>> Regarding COVAR_POP. Probably something similar happening regarding
> >>> reserved words being. You should probably add it to
> >> ReservedFunctionName(),
> >>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
> Drill —
> >>> so I suggest you contribute an Optiq patch. Be sure to add tests to
> >>> SqlOperatorBaseTest along the lines of testVarPopFunc.
> >>>
> >>> Julian
> >>>
> >>>
> >>
>
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

@venki/julian: i have got a limited internet connectivity tonight. Will do
it first thing tomm.

Thanks both.
On 23/09/2014 9:24 pm, "Julian Hyde" <ju...@gmail.com> wrote:

> It looks good. (You messed up the formatting in one place and added
> out-of-order to alphabetized lists in several places. I have fixed these.)
>
> Please submit a pull request for your branch and I will commit with my
> changes.
>
> Julian
>
> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > @Jacques- I would wait for Venki's input. Till then I have created a
> patch
> > for Optiq.
> >
> > @Julian/Optiq Dev: Could you please review the commit[1] if everything
> > looks good. Also I am not able to run the test case.
> > I am using:
> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> > It is not able to recognize the test cases.
> > Works fine with -DfailIfNoTests=flase flag.
> >
> > 1:
> >
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> >
> > Thanks
> >
> >
> >
> > On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
> >
> >> I believe that Drill also allows addition of reserved words through
> some of
> >> the freemarker inclusions but could be mistaken.  I think Venki could
> >> provide more input if you can't find the spot.
> >>
> >> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
> >> wrote:
> >>
> >>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
> >>>
> >>>> Am debugging Optiqand DrillOperatorTable.
> >>>>
> >>>> Strangely this works with backticks - as pointed out by Kryatal in
> >>>> DRILL-1441
> >>>> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> >>> limit
> >>>> 10;
> >>>
> >>> See my comments on REPLACE in
> >>> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate
> to
> >>> using a non-reserved keyword as an identifier.
> >>>
> >>> Regarding COVAR_POP. Probably something similar happening regarding
> >>> reserved words being. You should probably add it to
> >> ReservedFunctionName(),
> >>> just like VAR_POP and STDEV_POP. That is probably tricky to do in
> Drill —
> >>> so I suggest you contribute an Optiq patch. Be sure to add tests to
> >>> SqlOperatorBaseTest along the lines of testVarPopFunc.
> >>>
> >>> Julian
> >>>
> >>>
> >>
>
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@gmail.com>]

It looks good. (You messed up the formatting in one place and added out-of-order to alphabetized lists in several places. I have fixed these.)

Please submit a pull request for your branch and I will commit with my changes.

Julian

On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:

> @Jacques- I would wait for Venki's input. Till then I have created a patch
> for Optiq.
> 
> @Julian/Optiq Dev: Could you please review the commit[1] if everything
> looks good. Also I am not able to run the test case.
> I am using:
> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> It is not able to recognize the test cases.
> Works fine with -DfailIfNoTests=flase flag.
> 
> 1:
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> 
> Thanks
> 
> 
> 
> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org> wrote:
> 
>> I believe that Drill also allows addition of reserved words through some of
>> the freemarker inclusions but could be mistaken.  I think Venki could
>> provide more input if you can't find the spot.
>> 
>> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
>> wrote:
>> 
>>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>>> 
>>>> Am debugging Optiqand DrillOperatorTable.
>>>> 
>>>> Strangely this works with backticks - as pointed out by Kryatal in
>>>> DRILL-1441
>>>> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
>>> limit
>>>> 10;
>>> 
>>> See my comments on REPLACE in
>>> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
>>> using a non-reserved keyword as an identifier.
>>> 
>>> Regarding COVAR_POP. Probably something similar happening regarding
>>> reserved words being. You should probably add it to
>> ReservedFunctionName(),
>>> just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
>>> so I suggest you contribute an Optiq patch. Be sure to add tests to
>>> SqlOperatorBaseTest along the lines of testVarPopFunc.
>>> 
>>> Julian
>>> 
>>> 
>> 

Re: Adding new drill functions to optiq syntax

[Venki Korukanti <ve...@gmail.com>]

One thing currently there are no freemarker variables for adding keywords
to reserved function names or non-reserved keywords. So it may not help you
in resolving the parsing issue that needs backticks around function name.

Thanks
Venki

On Tue, Sep 23, 2014 at 7:07 AM, Venki Korukanti <ve...@gmail.com>
wrote:

> Yash: Here
> <https://github.com/apache/incubator-drill/blob/master/exec/java-exec/src/main/codegen/data/Parser.tdd#L30> is
> the link to the freemarker data file for parser. You can add new keywords
> to "keywords" section.
>
> On Tue, Sep 23, 2014 at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>
>> @Jacques- I would wait for Venki's input. Till then I have created a patch
>> for Optiq.
>>
>> @Julian/Optiq Dev: Could you please review the commit[1] if everything
>> looks good. Also I am not able to run the test case.
>> I am using:
>> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
>> It is not able to recognize the test cases.
>> Works fine with -DfailIfNoTests=flase flag.
>>
>> 1:
>>
>> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
>>
>> Thanks
>>
>>
>>
>> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
>> wrote:
>>
>> > I believe that Drill also allows addition of reserved words through
>> some of
>> > the freemarker inclusions but could be mistaken.  I think Venki could
>> > provide more input if you can't find the spot.
>> >
>> > On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
>> > wrote:
>> >
>> > > On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>> > >
>> > > > Am debugging Optiqand DrillOperatorTable.
>> > > >
>> > > > Strangely this works with backticks - as pointed out by Kryatal in
>> > > > DRILL-1441
>> > > > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
>> > > limit
>> > > > 10;
>> > >
>> > > See my comments on REPLACE in
>> > > https://issues.apache.org/jira/browse/DRILL-1441. The problems
>> relate to
>> > > using a non-reserved keyword as an identifier.
>> > >
>> > > Regarding COVAR_POP. Probably something similar happening regarding
>> > > reserved words being. You should probably add it to
>> > ReservedFunctionName(),
>> > > just like VAR_POP and STDEV_POP. That is probably tricky to do in
>> Drill —
>> > > so I suggest you contribute an Optiq patch. Be sure to add tests to
>> > > SqlOperatorBaseTest along the lines of testVarPopFunc.
>> > >
>> > > Julian
>> > >
>> > >
>> >
>>
>
>

Re: Adding new drill functions to optiq syntax

[Venki Korukanti <ve...@gmail.com>]

One thing currently there are no freemarker variables for adding keywords
to reserved function names or non-reserved keywords. So it may not help you
in resolving the parsing issue that needs backticks around function name.

Thanks
Venki

On Tue, Sep 23, 2014 at 7:07 AM, Venki Korukanti <ve...@gmail.com>
wrote:

> Yash: Here
> <https://github.com/apache/incubator-drill/blob/master/exec/java-exec/src/main/codegen/data/Parser.tdd#L30> is
> the link to the freemarker data file for parser. You can add new keywords
> to "keywords" section.
>
> On Tue, Sep 23, 2014 at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>
>> @Jacques- I would wait for Venki's input. Till then I have created a patch
>> for Optiq.
>>
>> @Julian/Optiq Dev: Could you please review the commit[1] if everything
>> looks good. Also I am not able to run the test case.
>> I am using:
>> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
>> It is not able to recognize the test cases.
>> Works fine with -DfailIfNoTests=flase flag.
>>
>> 1:
>>
>> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
>>
>> Thanks
>>
>>
>>
>> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
>> wrote:
>>
>> > I believe that Drill also allows addition of reserved words through
>> some of
>> > the freemarker inclusions but could be mistaken.  I think Venki could
>> > provide more input if you can't find the spot.
>> >
>> > On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
>> > wrote:
>> >
>> > > On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>> > >
>> > > > Am debugging Optiqand DrillOperatorTable.
>> > > >
>> > > > Strangely this works with backticks - as pointed out by Kryatal in
>> > > > DRILL-1441
>> > > > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
>> > > limit
>> > > > 10;
>> > >
>> > > See my comments on REPLACE in
>> > > https://issues.apache.org/jira/browse/DRILL-1441. The problems
>> relate to
>> > > using a non-reserved keyword as an identifier.
>> > >
>> > > Regarding COVAR_POP. Probably something similar happening regarding
>> > > reserved words being. You should probably add it to
>> > ReservedFunctionName(),
>> > > just like VAR_POP and STDEV_POP. That is probably tricky to do in
>> Drill —
>> > > so I suggest you contribute an Optiq patch. Be sure to add tests to
>> > > SqlOperatorBaseTest along the lines of testVarPopFunc.
>> > >
>> > > Julian
>> > >
>> > >
>> >
>>
>
>

Re: Adding new drill functions to optiq syntax

[Venki Korukanti <ve...@gmail.com>]

Hi Yash,

What is the JavaCC error you are seeing? If you are adding "COVAR_POP" which
already exists in the keywords list in Optiq's CombinedParser.jj.
"keywords" variable in Parser.tdd is used if you want to add new keywords
that don't exists in Optiq's keyword list (ex. "USE", "FILES", "DATABASES"
etc.)

You don't need to write corresponding methods unless you want to add new
SQL constructs that use the new keyword.

Thanks
Venki

On Tue, Sep 23, 2014 at 8:35 AM, Yash Sharma <ya...@gmail.com> wrote:

> Thanks lot Venki. Infact I reached this point and tried adding new keyword
> here but I couldn't build drill.
> It kept failing with JavaCC error.
> I also had a look at parserImpls.ftl but couldnot decode it.
>
> Do we also have to write the methods for the corresponding keywords?
>
> Thanks
> On 23/09/2014 8:25 pm, "Venki Korukanti" <ve...@gmail.com>
> wrote:
>
> > Yash: Here
> > <
> >
> https://github.com/apache/incubator-drill/blob/master/exec/java-exec/src/main/codegen/data/Parser.tdd#L30
> > >
> > is
> > the link to the freemarker data file for parser. You can add new keywords
> > to "keywords" section.
> >
> > On Tue, Sep 23, 2014 at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> > > @Jacques- I would wait for Venki's input. Till then I have created a
> > patch
> > > for Optiq.
> > >
> > > @Julian/Optiq Dev: Could you please review the commit[1] if everything
> > > looks good. Also I am not able to run the test case.
> > > I am using:
> > > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> > > It is not able to recognize the test cases.
> > > Works fine with -DfailIfNoTests=flase flag.
> > >
> > > 1:
> > >
> > >
> >
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> > >
> > > Thanks
> > >
> > >
> > >
> > > On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> > > wrote:
> > >
> > > > I believe that Drill also allows addition of reserved words through
> > some
> > > of
> > > > the freemarker inclusions but could be mistaken.  I think Venki could
> > > > provide more input if you can't find the spot.
> > > >
> > > > On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <julian@hydromatic.net
> >
> > > > wrote:
> > > >
> > > > > On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com>
> wrote:
> > > > >
> > > > > > Am debugging Optiqand DrillOperatorTable.
> > > > > >
> > > > > > Strangely this works with backticks - as pointed out by Kryatal
> in
> > > > > > DRILL-1441
> > > > > > select `covar_pop`(employee_id, employee_id) FROM
> > cp.`employee.json`
> > > > > limit
> > > > > > 10;
> > > > >
> > > > > See my comments on REPLACE in
> > > > > https://issues.apache.org/jira/browse/DRILL-1441. The problems
> > relate
> > > to
> > > > > using a non-reserved keyword as an identifier.
> > > > >
> > > > > Regarding COVAR_POP. Probably something similar happening regarding
> > > > > reserved words being. You should probably add it to
> > > > ReservedFunctionName(),
> > > > > just like VAR_POP and STDEV_POP. That is probably tricky to do in
> > > Drill —
> > > > > so I suggest you contribute an Optiq patch. Be sure to add tests to
> > > > > SqlOperatorBaseTest along the lines of testVarPopFunc.
> > > > >
> > > > > Julian
> > > > >
> > > > >
> > > >
> > >
> >
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Thanks lot Venki. Infact I reached this point and tried adding new keyword
here but I couldn't build drill.
It kept failing with JavaCC error.
I also had a look at parserImpls.ftl but couldnot decode it.

Do we also have to write the methods for the corresponding keywords?

Thanks
On 23/09/2014 8:25 pm, "Venki Korukanti" <ve...@gmail.com> wrote:

> Yash: Here
> <
> https://github.com/apache/incubator-drill/blob/master/exec/java-exec/src/main/codegen/data/Parser.tdd#L30
> >
> is
> the link to the freemarker data file for parser. You can add new keywords
> to "keywords" section.
>
> On Tue, Sep 23, 2014 at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > @Jacques- I would wait for Venki's input. Till then I have created a
> patch
> > for Optiq.
> >
> > @Julian/Optiq Dev: Could you please review the commit[1] if everything
> > looks good. Also I am not able to run the test case.
> > I am using:
> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> > It is not able to recognize the test cases.
> > Works fine with -DfailIfNoTests=flase flag.
> >
> > 1:
> >
> >
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> >
> > Thanks
> >
> >
> >
> > On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> > wrote:
> >
> > > I believe that Drill also allows addition of reserved words through
> some
> > of
> > > the freemarker inclusions but could be mistaken.  I think Venki could
> > > provide more input if you can't find the spot.
> > >
> > > On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
> > > wrote:
> > >
> > > > On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
> > > >
> > > > > Am debugging Optiqand DrillOperatorTable.
> > > > >
> > > > > Strangely this works with backticks - as pointed out by Kryatal in
> > > > > DRILL-1441
> > > > > select `covar_pop`(employee_id, employee_id) FROM
> cp.`employee.json`
> > > > limit
> > > > > 10;
> > > >
> > > > See my comments on REPLACE in
> > > > https://issues.apache.org/jira/browse/DRILL-1441. The problems
> relate
> > to
> > > > using a non-reserved keyword as an identifier.
> > > >
> > > > Regarding COVAR_POP. Probably something similar happening regarding
> > > > reserved words being. You should probably add it to
> > > ReservedFunctionName(),
> > > > just like VAR_POP and STDEV_POP. That is probably tricky to do in
> > Drill —
> > > > so I suggest you contribute an Optiq patch. Be sure to add tests to
> > > > SqlOperatorBaseTest along the lines of testVarPopFunc.
> > > >
> > > > Julian
> > > >
> > > >
> > >
> >
>

Re: Adding new drill functions to optiq syntax

[Venki Korukanti <ve...@gmail.com>]

Yash: Here
<https://github.com/apache/incubator-drill/blob/master/exec/java-exec/src/main/codegen/data/Parser.tdd#L30>
is
the link to the freemarker data file for parser. You can add new keywords
to "keywords" section.

On Tue, Sep 23, 2014 at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:

> @Jacques- I would wait for Venki's input. Till then I have created a patch
> for Optiq.
>
> @Julian/Optiq Dev: Could you please review the commit[1] if everything
> looks good. Also I am not able to run the test case.
> I am using:
> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> It is not able to recognize the test cases.
> Works fine with -DfailIfNoTests=flase flag.
>
> 1:
>
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
>
> Thanks
>
>
>
> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
>
> > I believe that Drill also allows addition of reserved words through some
> of
> > the freemarker inclusions but could be mistaken.  I think Venki could
> > provide more input if you can't find the spot.
> >
> > On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
> > wrote:
> >
> > > On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
> > >
> > > > Am debugging Optiqand DrillOperatorTable.
> > > >
> > > > Strangely this works with backticks - as pointed out by Kryatal in
> > > > DRILL-1441
> > > > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> > > limit
> > > > 10;
> > >
> > > See my comments on REPLACE in
> > > https://issues.apache.org/jira/browse/DRILL-1441. The problems relate
> to
> > > using a non-reserved keyword as an identifier.
> > >
> > > Regarding COVAR_POP. Probably something similar happening regarding
> > > reserved words being. You should probably add it to
> > ReservedFunctionName(),
> > > just like VAR_POP and STDEV_POP. That is probably tricky to do in
> Drill —
> > > so I suggest you contribute an Optiq patch. Be sure to add tests to
> > > SqlOperatorBaseTest along the lines of testVarPopFunc.
> > >
> > > Julian
> > >
> > >
> >
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@gmail.com>]

It looks good. (You messed up the formatting in one place and added out-of-order to alphabetized lists in several places. I have fixed these.)

Please submit a pull request for your branch and I will commit with my changes.

Julian

On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:

> @Jacques- I would wait for Venki's input. Till then I have created a patch
> for Optiq.
> 
> @Julian/Optiq Dev: Could you please review the commit[1] if everything
> looks good. Also I am not able to run the test case.
> I am using:
> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> It is not able to recognize the test cases.
> Works fine with -DfailIfNoTests=flase flag.
> 
> 1:
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
> 
> Thanks
> 
> 
> 
> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org> wrote:
> 
>> I believe that Drill also allows addition of reserved words through some of
>> the freemarker inclusions but could be mistaken.  I think Venki could
>> provide more input if you can't find the spot.
>> 
>> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
>> wrote:
>> 
>>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>>> 
>>>> Am debugging Optiqand DrillOperatorTable.
>>>> 
>>>> Strangely this works with backticks - as pointed out by Kryatal in
>>>> DRILL-1441
>>>> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
>>> limit
>>>> 10;
>>> 
>>> See my comments on REPLACE in
>>> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
>>> using a non-reserved keyword as an identifier.
>>> 
>>> Regarding COVAR_POP. Probably something similar happening regarding
>>> reserved words being. You should probably add it to
>> ReservedFunctionName(),
>>> just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
>>> so I suggest you contribute an Optiq patch. Be sure to add tests to
>>> SqlOperatorBaseTest along the lines of testVarPopFunc.
>>> 
>>> Julian
>>> 
>>> 
>> 

Re: Adding new drill functions to optiq syntax

[Venki Korukanti <ve...@gmail.com>]

Yash: Here
<https://github.com/apache/incubator-drill/blob/master/exec/java-exec/src/main/codegen/data/Parser.tdd#L30>
is
the link to the freemarker data file for parser. You can add new keywords
to "keywords" section.

On Tue, Sep 23, 2014 at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:

> @Jacques- I would wait for Venki's input. Till then I have created a patch
> for Optiq.
>
> @Julian/Optiq Dev: Could you please review the commit[1] if everything
> looks good. Also I am not able to run the test case.
> I am using:
> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> It is not able to recognize the test cases.
> Works fine with -DfailIfNoTests=flase flag.
>
> 1:
>
> https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2
>
> Thanks
>
>
>
> On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org>
> wrote:
>
> > I believe that Drill also allows addition of reserved words through some
> of
> > the freemarker inclusions but could be mistaken.  I think Venki could
> > provide more input if you can't find the spot.
> >
> > On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
> > wrote:
> >
> > > On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
> > >
> > > > Am debugging Optiqand DrillOperatorTable.
> > > >
> > > > Strangely this works with backticks - as pointed out by Kryatal in
> > > > DRILL-1441
> > > > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> > > limit
> > > > 10;
> > >
> > > See my comments on REPLACE in
> > > https://issues.apache.org/jira/browse/DRILL-1441. The problems relate
> to
> > > using a non-reserved keyword as an identifier.
> > >
> > > Regarding COVAR_POP. Probably something similar happening regarding
> > > reserved words being. You should probably add it to
> > ReservedFunctionName(),
> > > just like VAR_POP and STDEV_POP. That is probably tricky to do in
> Drill —
> > > so I suggest you contribute an Optiq patch. Be sure to add tests to
> > > SqlOperatorBaseTest along the lines of testVarPopFunc.
> > >
> > > Julian
> > >
> > >
> >
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Thanks you Julian.


On Wed, Sep 24, 2014 at 12:49 AM, Julian Hyde <ju...@hydromatic.net> wrote:

> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > I am not able to run the test case.
> > I am using:
> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> > It is not able to recognize the test cases.
> > Works fine with -DfailIfNoTests=flase flag.
>
> SqlOperatorBaseTest is abstract. Try with a concrete sub-class, e.g.
>
> $ mvn test -DfailIfNoTests=false
> -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
>
> Julian
>
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Have also added a new pull request for supporting the REGR_SYY aggregate
function.
It would be required for DRILL-1332 along with the REGR_SXX and REGR_SXY
which are already registered in Optiq.

Thanks

On Wed, Sep 24, 2014 at 3:13 PM, Yash Sharma <ya...@gmail.com> wrote:

> Hey Julian,
> Even this fails. Its somehow not able to pick the test cases-
> mvn test  -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
> Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
>
> It works with the failNoTests flag set to false.
>
> Thanks
>
> On Wed, Sep 24, 2014 at 12:49 AM, Julian Hyde <ju...@hydromatic.net>
> wrote:
>
>> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>>
>> > I am not able to run the test case.
>> > I am using:
>> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
>> > It is not able to recognize the test cases.
>> > Works fine with -DfailIfNoTests=flase flag.
>>
>> SqlOperatorBaseTest is abstract. Try with a concrete sub-class, e.g.
>>
>> $ mvn test -DfailIfNoTests=false
>> -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
>>
>> Julian
>>
>>
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Done.  Will also check from my end if I get any clues.

Thanks

On Wed, Sep 24, 2014 at 11:05 PM, Julian Hyde <ju...@gmail.com> wrote:

> I don’t know why. I even get failures when I run
>
> mvn test  -Dtest=OptiqSqlOperatorTest
>
> But OptiqSqlOperatorTest succeeds as part of the whole suite, or when I
> run it from Intellij. Frankly I don’t run individual tests from the command
> line — I run things from intellij. I only run the whole suite from the
> command line.
>
> Can you log a jira case please?
>
> Julian
>
>
> On Sep 24, 2014, at 2:43 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Hey Julian,
> > Even this fails. Its somehow not able to pick the test cases-
> > mvn test  -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
> > Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
> >
> > It works with the failNoTests flag set to false.
> >
> > Thanks
> >
> > On Wed, Sep 24, 2014 at 12:49 AM, Julian Hyde <ju...@hydromatic.net>
> wrote:
> >
> >> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
> >>
> >>> I am not able to run the test case.
> >>> I am using:
> >>> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> >>> It is not able to recognize the test cases.
> >>> Works fine with -DfailIfNoTests=flase flag.
> >>
> >> SqlOperatorBaseTest is abstract. Try with a concrete sub-class, e.g.
> >>
> >> $ mvn test -DfailIfNoTests=false
> >> -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
> >>
> >> Julian
> >>
> >>
>
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@gmail.com>]

I don’t know why. I even get failures when I run

mvn test  -Dtest=OptiqSqlOperatorTest

But OptiqSqlOperatorTest succeeds as part of the whole suite, or when I run it from Intellij. Frankly I don’t run individual tests from the command line — I run things from intellij. I only run the whole suite from the command line.

Can you log a jira case please?

Julian


On Sep 24, 2014, at 2:43 AM, Yash Sharma <ya...@gmail.com> wrote:

> Hey Julian,
> Even this fails. Its somehow not able to pick the test cases-
> mvn test  -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
> Tests run: 0, Failures: 0, Errors: 0, Skipped: 0
> 
> It works with the failNoTests flag set to false.
> 
> Thanks
> 
> On Wed, Sep 24, 2014 at 12:49 AM, Julian Hyde <ju...@hydromatic.net> wrote:
> 
>> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>> 
>>> I am not able to run the test case.
>>> I am using:
>>> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
>>> It is not able to recognize the test cases.
>>> Works fine with -DfailIfNoTests=flase flag.
>> 
>> SqlOperatorBaseTest is abstract. Try with a concrete sub-class, e.g.
>> 
>> $ mvn test -DfailIfNoTests=false
>> -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
>> 
>> Julian
>> 
>> 

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Hey Julian,
Even this fails. Its somehow not able to pick the test cases-
mvn test  -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
Tests run: 0, Failures: 0, Errors: 0, Skipped: 0

It works with the failNoTests flag set to false.

Thanks

On Wed, Sep 24, 2014 at 12:49 AM, Julian Hyde <ju...@hydromatic.net> wrote:

> On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > I am not able to run the test case.
> > I am using:
> > $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> > It is not able to recognize the test cases.
> > Works fine with -DfailIfNoTests=flase flag.
>
> SqlOperatorBaseTest is abstract. Try with a concrete sub-class, e.g.
>
> $ mvn test -DfailIfNoTests=false
> -Dtest=OptiqSqlOperatorTest#testStddevPopFunc
>
> Julian
>
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@hydromatic.net>]

On Sep 23, 2014, at 3:17 AM, Yash Sharma <ya...@gmail.com> wrote:

> I am not able to run the test case.
> I am using:
> $mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
> It is not able to recognize the test cases.
> Works fine with -DfailIfNoTests=flase flag.

SqlOperatorBaseTest is abstract. Try with a concrete sub-class, e.g. 

$ mvn test -DfailIfNoTests=false -Dtest=OptiqSqlOperatorTest#testStddevPopFunc

Julian

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

@Jacques- I would wait for Venki's input. Till then I have created a patch
for Optiq.

@Julian/Optiq Dev: Could you please review the commit[1] if everything
looks good. Also I am not able to run the test case.
I am using:
$mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
It is not able to recognize the test cases.
Works fine with -DfailIfNoTests=flase flag.

1:
https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2

Thanks



On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org> wrote:

> I believe that Drill also allows addition of reserved words through some of
> the freemarker inclusions but could be mistaken.  I think Venki could
> provide more input if you can't find the spot.
>
> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
> wrote:
>
> > On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> > > Am debugging Optiqand DrillOperatorTable.
> > >
> > > Strangely this works with backticks - as pointed out by Kryatal in
> > > DRILL-1441
> > > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> > limit
> > > 10;
> >
> > See my comments on REPLACE in
> > https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
> > using a non-reserved keyword as an identifier.
> >
> > Regarding COVAR_POP. Probably something similar happening regarding
> > reserved words being. You should probably add it to
> ReservedFunctionName(),
> > just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
> > so I suggest you contribute an Optiq patch. Be sure to add tests to
> > SqlOperatorBaseTest along the lines of testVarPopFunc.
> >
> > Julian
> >
> >
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

@Jacques- I would wait for Venki's input. Till then I have created a patch
for Optiq.

@Julian/Optiq Dev: Could you please review the commit[1] if everything
looks good. Also I am not able to run the test case.
I am using:
$mvn test -Dtest=SqlOperatorBaseTest#testStddevPopFunc
It is not able to recognize the test cases.
Works fine with -DfailIfNoTests=flase flag.

1:
https://github.com/yssharma/incubator-optiq/commit/9a0b063adf91cee78f3d167c002eb07c6fb7b9b2

Thanks



On Tue, Sep 23, 2014 at 9:27 AM, Jacques Nadeau <ja...@apache.org> wrote:

> I believe that Drill also allows addition of reserved words through some of
> the freemarker inclusions but could be mistaken.  I think Venki could
> provide more input if you can't find the spot.
>
> On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net>
> wrote:
>
> > On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> > > Am debugging Optiqand DrillOperatorTable.
> > >
> > > Strangely this works with backticks - as pointed out by Kryatal in
> > > DRILL-1441
> > > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> > limit
> > > 10;
> >
> > See my comments on REPLACE in
> > https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
> > using a non-reserved keyword as an identifier.
> >
> > Regarding COVAR_POP. Probably something similar happening regarding
> > reserved words being. You should probably add it to
> ReservedFunctionName(),
> > just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
> > so I suggest you contribute an Optiq patch. Be sure to add tests to
> > SqlOperatorBaseTest along the lines of testVarPopFunc.
> >
> > Julian
> >
> >
>

Re: Adding new drill functions to optiq syntax

[Jacques Nadeau <ja...@apache.org>]

I believe that Drill also allows addition of reserved words through some of
the freemarker inclusions but could be mistaken.  I think Venki could
provide more input if you can't find the spot.

On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net> wrote:

> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Am debugging Optiqand DrillOperatorTable.
> >
> > Strangely this works with backticks - as pointed out by Kryatal in
> > DRILL-1441
> > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> limit
> > 10;
>
> See my comments on REPLACE in
> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
> using a non-reserved keyword as an identifier.
>
> Regarding COVAR_POP. Probably something similar happening regarding
> reserved words being. You should probably add it to ReservedFunctionName(),
> just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
> so I suggest you contribute an Optiq patch. Be sure to add tests to
> SqlOperatorBaseTest along the lines of testVarPopFunc.
>
> Julian
>
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@hydromatic.net>]

Of course it would be nice if you provided an implementation of the aggregate functions, and reducing aggregates to a simpler form is one of the easier ways of implementing them. But I’m happy to accept a patch that just parses and validates them. Implementing them can wait.

Reducing aggregate functions has another benefit, even if, like Drill, you have your own implementation. When you reduce AVG to SUM / COUNT, you have two aggregate functions that can be rolled up. Therefore you can roll up AVG from partial results or materialized views. I believe some other aggregate functions have similar properties when they are rewritten in terms of simpler aggregate functions.

Julian


On Sep 22, 2014, at 1:35 PM, Yash Sharma <ya...@gmail.com> wrote:

> Thanks Julian. Have created OPTIQ-421 and will submit the patch for review.
> Also wanted to ask - is the reduce aggregation required? Since its already
> being done at Drill's end?
> 
> ReduceAggregatesRule: reduceStddev()
> 
> Thanks
> 
> 
> On Tue, Sep 23, 2014 at 12:58 AM, Julian Hyde <ju...@hydromatic.net> wrote:
> 
>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>> 
>>> Am debugging Optiqand DrillOperatorTable.
>>> 
>>> Strangely this works with backticks - as pointed out by Kryatal in
>>> DRILL-1441
>>> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
>> limit
>>> 10;
>> 
>> See my comments on REPLACE in
>> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
>> using a non-reserved keyword as an identifier.
>> 
>> Regarding COVAR_POP. Probably something similar happening regarding
>> reserved words being. You should probably add it to ReservedFunctionName(),
>> just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
>> so I suggest you contribute an Optiq patch. Be sure to add tests to
>> SqlOperatorBaseTest along the lines of testVarPopFunc.
>> 
>> Julian
>> 
>> 

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@hydromatic.net>]

Of course it would be nice if you provided an implementation of the aggregate functions, and reducing aggregates to a simpler form is one of the easier ways of implementing them. But I’m happy to accept a patch that just parses and validates them. Implementing them can wait.

Reducing aggregate functions has another benefit, even if, like Drill, you have your own implementation. When you reduce AVG to SUM / COUNT, you have two aggregate functions that can be rolled up. Therefore you can roll up AVG from partial results or materialized views. I believe some other aggregate functions have similar properties when they are rewritten in terms of simpler aggregate functions.

Julian


On Sep 22, 2014, at 1:35 PM, Yash Sharma <ya...@gmail.com> wrote:

> Thanks Julian. Have created OPTIQ-421 and will submit the patch for review.
> Also wanted to ask - is the reduce aggregation required? Since its already
> being done at Drill's end?
> 
> ReduceAggregatesRule: reduceStddev()
> 
> Thanks
> 
> 
> On Tue, Sep 23, 2014 at 12:58 AM, Julian Hyde <ju...@hydromatic.net> wrote:
> 
>> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>> 
>>> Am debugging Optiqand DrillOperatorTable.
>>> 
>>> Strangely this works with backticks - as pointed out by Kryatal in
>>> DRILL-1441
>>> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
>> limit
>>> 10;
>> 
>> See my comments on REPLACE in
>> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
>> using a non-reserved keyword as an identifier.
>> 
>> Regarding COVAR_POP. Probably something similar happening regarding
>> reserved words being. You should probably add it to ReservedFunctionName(),
>> just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
>> so I suggest you contribute an Optiq patch. Be sure to add tests to
>> SqlOperatorBaseTest along the lines of testVarPopFunc.
>> 
>> Julian
>> 
>> 

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Thanks Julian. Have created OPTIQ-421 and will submit the patch for review.
Also wanted to ask - is the reduce aggregation required? Since its already
being done at Drill's end?

ReduceAggregatesRule: reduceStddev()

Thanks


On Tue, Sep 23, 2014 at 12:58 AM, Julian Hyde <ju...@hydromatic.net> wrote:

> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Am debugging Optiqand DrillOperatorTable.
> >
> > Strangely this works with backticks - as pointed out by Kryatal in
> > DRILL-1441
> > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> limit
> > 10;
>
> See my comments on REPLACE in
> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
> using a non-reserved keyword as an identifier.
>
> Regarding COVAR_POP. Probably something similar happening regarding
> reserved words being. You should probably add it to ReservedFunctionName(),
> just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
> so I suggest you contribute an Optiq patch. Be sure to add tests to
> SqlOperatorBaseTest along the lines of testVarPopFunc.
>
> Julian
>
>

Re: Adding new drill functions to optiq syntax

[Jacques Nadeau <ja...@apache.org>]

I believe that Drill also allows addition of reserved words through some of
the freemarker inclusions but could be mistaken.  I think Venki could
provide more input if you can't find the spot.

On Mon, Sep 22, 2014 at 12:28 PM, Julian Hyde <ju...@hydromatic.net> wrote:

> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Am debugging Optiqand DrillOperatorTable.
> >
> > Strangely this works with backticks - as pointed out by Kryatal in
> > DRILL-1441
> > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> limit
> > 10;
>
> See my comments on REPLACE in
> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
> using a non-reserved keyword as an identifier.
>
> Regarding COVAR_POP. Probably something similar happening regarding
> reserved words being. You should probably add it to ReservedFunctionName(),
> just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
> so I suggest you contribute an Optiq patch. Be sure to add tests to
> SqlOperatorBaseTest along the lines of testVarPopFunc.
>
> Julian
>
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Thanks Julian. Have created OPTIQ-421 and will submit the patch for review.
Also wanted to ask - is the reduce aggregation required? Since its already
being done at Drill's end?

ReduceAggregatesRule: reduceStddev()

Thanks


On Tue, Sep 23, 2014 at 12:58 AM, Julian Hyde <ju...@hydromatic.net> wrote:

> On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Am debugging Optiqand DrillOperatorTable.
> >
> > Strangely this works with backticks - as pointed out by Kryatal in
> > DRILL-1441
> > select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json`
> limit
> > 10;
>
> See my comments on REPLACE in
> https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to
> using a non-reserved keyword as an identifier.
>
> Regarding COVAR_POP. Probably something similar happening regarding
> reserved words being. You should probably add it to ReservedFunctionName(),
> just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill —
> so I suggest you contribute an Optiq patch. Be sure to add tests to
> SqlOperatorBaseTest along the lines of testVarPopFunc.
>
> Julian
>
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@hydromatic.net>]

On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:

> Am debugging Optiqand DrillOperatorTable.
> 
> Strangely this works with backticks - as pointed out by Kryatal in
> DRILL-1441
> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json` limit
> 10;

See my comments on REPLACE in https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to using a non-reserved keyword as an identifier.

Regarding COVAR_POP. Probably something similar happening regarding reserved words being. You should probably add it to ReservedFunctionName(), just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill — so I suggest you contribute an Optiq patch. Be sure to add tests to SqlOperatorBaseTest along the lines of testVarPopFunc.

Julian

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@hydromatic.net>]

On Sep 22, 2014, at 11:07 AM, Yash Sharma <ya...@gmail.com> wrote:

> Am debugging Optiqand DrillOperatorTable.
> 
> Strangely this works with backticks - as pointed out by Kryatal in
> DRILL-1441
> select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json` limit
> 10;

See my comments on REPLACE in https://issues.apache.org/jira/browse/DRILL-1441. The problems relate to using a non-reserved keyword as an identifier.

Regarding COVAR_POP. Probably something similar happening regarding reserved words being. You should probably add it to ReservedFunctionName(), just like VAR_POP and STDEV_POP. That is probably tricky to do in Drill — so I suggest you contribute an Optiq patch. Be sure to add tests to SqlOperatorBaseTest along the lines of testVarPopFunc.

Julian

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Am debugging Optiqand DrillOperatorTable.

Strangely this works with backticks - as pointed out by Kryatal in
DRILL-1441
select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json` limit
10;

On Mon, Sep 22, 2014 at 9:36 PM, Jacques Nadeau <ja...@apache.org> wrote:

> Agreed on all points.
>
> On Mon, Sep 22, 2014 at 8:53 AM, Julian Hyde <ju...@gmail.com> wrote:
>
> > If these functions don’t require new syntax — and it looks as if
> > regr_avgx, for one, is just simple function syntax — then you don’t need
> to
> > change the parser. If the parser needs semantic knowledge, such as
> whether
> > regr_avgx is a regular function or an aggregate function, you may be able
> > to pass that in via the SqlOperatorTable.
> >
> > In short, very unlikely you need to parser.
> >
> > Lastly, if you are adding SQL standard functionality to the parser or
> > validator, please consider contributing them to Optiq rather than
> handling
> > them in Drill-specific code.
> >
> > Julian
> >
> >
> > On Sep 22, 2014, at 5:50 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> > > Hi All,
> > > I am planning to add new drill functions to optiq syntax for parsing.
> > > We have new functions like regr_avgx/regr_avgy etc and existing ones
> like
> > > covar_samp,covar_pop, correlation/corr etc.
> > >
> > > Going through this old thread[1] I see that we need to add the
> > definitions
> > > in the CombinedParser.jj[2] in Optiq.
> > >
> > > Is that the way we still use or its handled differently now ?
> > >
> > > Thanks.
> > >
> > > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> > > 2.
> > >
> >
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
> >
> >
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Am debugging Optiqand DrillOperatorTable.

Strangely this works with backticks - as pointed out by Kryatal in
DRILL-1441
select `covar_pop`(employee_id, employee_id) FROM cp.`employee.json` limit
10;

On Mon, Sep 22, 2014 at 9:36 PM, Jacques Nadeau <ja...@apache.org> wrote:

> Agreed on all points.
>
> On Mon, Sep 22, 2014 at 8:53 AM, Julian Hyde <ju...@gmail.com> wrote:
>
> > If these functions don’t require new syntax — and it looks as if
> > regr_avgx, for one, is just simple function syntax — then you don’t need
> to
> > change the parser. If the parser needs semantic knowledge, such as
> whether
> > regr_avgx is a regular function or an aggregate function, you may be able
> > to pass that in via the SqlOperatorTable.
> >
> > In short, very unlikely you need to parser.
> >
> > Lastly, if you are adding SQL standard functionality to the parser or
> > validator, please consider contributing them to Optiq rather than
> handling
> > them in Drill-specific code.
> >
> > Julian
> >
> >
> > On Sep 22, 2014, at 5:50 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> > > Hi All,
> > > I am planning to add new drill functions to optiq syntax for parsing.
> > > We have new functions like regr_avgx/regr_avgy etc and existing ones
> like
> > > covar_samp,covar_pop, correlation/corr etc.
> > >
> > > Going through this old thread[1] I see that we need to add the
> > definitions
> > > in the CombinedParser.jj[2] in Optiq.
> > >
> > > Is that the way we still use or its handled differently now ?
> > >
> > > Thanks.
> > >
> > > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> > > 2.
> > >
> >
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
> >
> >
>

Re: Adding new drill functions to optiq syntax

[Jacques Nadeau <ja...@apache.org>]

Agreed on all points.

On Mon, Sep 22, 2014 at 8:53 AM, Julian Hyde <ju...@gmail.com> wrote:

> If these functions don’t require new syntax — and it looks as if
> regr_avgx, for one, is just simple function syntax — then you don’t need to
> change the parser. If the parser needs semantic knowledge, such as whether
> regr_avgx is a regular function or an aggregate function, you may be able
> to pass that in via the SqlOperatorTable.
>
> In short, very unlikely you need to parser.
>
> Lastly, if you are adding SQL standard functionality to the parser or
> validator, please consider contributing them to Optiq rather than handling
> them in Drill-specific code.
>
> Julian
>
>
> On Sep 22, 2014, at 5:50 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Hi All,
> > I am planning to add new drill functions to optiq syntax for parsing.
> > We have new functions like regr_avgx/regr_avgy etc and existing ones like
> > covar_samp,covar_pop, correlation/corr etc.
> >
> > Going through this old thread[1] I see that we need to add the
> definitions
> > in the CombinedParser.jj[2] in Optiq.
> >
> > Is that the way we still use or its handled differently now ?
> >
> > Thanks.
> >
> > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> > 2.
> >
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
>
>

Re: Adding new drill functions to optiq syntax

[Jacques Nadeau <ja...@apache.org>]

Agreed on all points.

On Mon, Sep 22, 2014 at 8:53 AM, Julian Hyde <ju...@gmail.com> wrote:

> If these functions don’t require new syntax — and it looks as if
> regr_avgx, for one, is just simple function syntax — then you don’t need to
> change the parser. If the parser needs semantic knowledge, such as whether
> regr_avgx is a regular function or an aggregate function, you may be able
> to pass that in via the SqlOperatorTable.
>
> In short, very unlikely you need to parser.
>
> Lastly, if you are adding SQL standard functionality to the parser or
> validator, please consider contributing them to Optiq rather than handling
> them in Drill-specific code.
>
> Julian
>
>
> On Sep 22, 2014, at 5:50 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Hi All,
> > I am planning to add new drill functions to optiq syntax for parsing.
> > We have new functions like regr_avgx/regr_avgy etc and existing ones like
> > covar_samp,covar_pop, correlation/corr etc.
> >
> > Going through this old thread[1] I see that we need to add the
> definitions
> > in the CombinedParser.jj[2] in Optiq.
> >
> > Is that the way we still use or its handled differently now ?
> >
> > Thanks.
> >
> > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> > 2.
> >
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
>
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@gmail.com>]

If these functions don’t require new syntax — and it looks as if regr_avgx, for one, is just simple function syntax — then you don’t need to change the parser. If the parser needs semantic knowledge, such as whether regr_avgx is a regular function or an aggregate function, you may be able to pass that in via the SqlOperatorTable.

In short, very unlikely you need to parser.

Lastly, if you are adding SQL standard functionality to the parser or validator, please consider contributing them to Optiq rather than handling them in Drill-specific code.

Julian


On Sep 22, 2014, at 5:50 AM, Yash Sharma <ya...@gmail.com> wrote:

> Hi All,
> I am planning to add new drill functions to optiq syntax for parsing.
> We have new functions like regr_avgx/regr_avgy etc and existing ones like
> covar_samp,covar_pop, correlation/corr etc.
> 
> Going through this old thread[1] I see that we need to add the definitions
> in the CombinedParser.jj[2] in Optiq.
> 
> Is that the way we still use or its handled differently now ?
> 
> Thanks.
> 
> 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> 2.
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Thanks Jason, even I din't face issues while implementing Math functions
and Bitwise/Logical aggr functions. Drill would just pick them up.

I am having this issue with the Correlation/Covariance/Regression
functions. It would not parse the plan and fail
with org.eigenbase.sql.parser.SqlParseException.



On Mon, Sep 22, 2014 at 8:12 PM, Jason Altekruse <al...@gmail.com>
wrote:

> Hi Yash,
>
> I did not work on the code for this, but if the functions are just acting
> on columns to provide aggregates I believe they are just supported by
> Drills default mechanism for tying into the parser. We ended up modifying
> the parser with some FreeMarker hooks that allows for consumers of optiq to
> provide custom syntax that extends the parser without forking the whole
> thing.
>
> I believe that this works together with our function registry to provide
> default support for UDFs (which our system functions are defined in the
> same way as UDFs) without having to modify the parser itself. I haven't had
> any trouble just adding new DrillFuncs and having them be discovered by
> Drill and work after a new build. I believe I have only written scalar
> functions and not aggregations, but you can look at the SumFunctions class
> for some reference on the differences.
>
> -Jason
>
> On Mon, Sep 22, 2014 at 7:50 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Hi All,
> > I am planning to add new drill functions to optiq syntax for parsing.
> > We have new functions like regr_avgx/regr_avgy etc and existing ones like
> > covar_samp,covar_pop, correlation/corr etc.
> >
> > Going through this old thread[1] I see that we need to add the
> definitions
> > in the CombinedParser.jj[2] in Optiq.
> >
> > Is that the way we still use or its handled differently now ?
> >
> > Thanks.
> >
> > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> > 2.
> >
> >
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
> >
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Something similar to what Julian mentioned in the mail thread -  by adding
new commands for parsing.

| < STDDEV_POP: "STDDEV_POP" >
| < STDDEV_SAMP: "STDDEV_SAMP" >

Thanks.

On Mon, Sep 22, 2014 at 8:58 PM, Yash Sharma <ya...@gmail.com> wrote:

> Cool. So do we add the new function definitions in the CombinedParser.jj
> in Optiq?
> I see that we did something like that for standard deviation functions.
>
>
> On Mon, Sep 22, 2014 at 8:49 PM, Jinfeng Ni <ji...@gmail.com> wrote:
>
>> There are some differences between Drill simple functions and aggregate
>> functions.
>>
>> For Drill simple function, optiq will check Drill function registry
>> whether
>> a function exists. So, all you need to is to provide the function
>> template,
>> the function converter will automatically add the  function template into
>> function registry.
>>
>> Aggregate functions are different in that simple function could appear
>> anywhere in the query, while aggregate function could be only in certain
>> clause ( no aggregation function in where clause, etc).  Optiq have to
>> know
>> one certain function is aggregate function, in order to validate such
>> logic.  Seems we either need modify optiq to let it know new function
>> "regr_avgx"
>> is an aggregate, or Drill need provide a way to let optiq know "regr_avgx"
>> is aggregate function, not simple function.
>>
>>
>>
>> On Mon, Sep 22, 2014 at 7:42 AM, Jason Altekruse <
>> altekrusejason@gmail.com>
>> wrote:
>>
>> > Hi Yash,
>> >
>> > I did not work on the code for this, but if the functions are just
>> acting
>> > on columns to provide aggregates I believe they are just supported by
>> > Drills default mechanism for tying into the parser. We ended up
>> modifying
>> > the parser with some FreeMarker hooks that allows for consumers of
>> optiq to
>> > provide custom syntax that extends the parser without forking the whole
>> > thing.
>> >
>> > I believe that this works together with our function registry to provide
>> > default support for UDFs (which our system functions are defined in the
>> > same way as UDFs) without having to modify the parser itself. I haven't
>> had
>> > any trouble just adding new DrillFuncs and having them be discovered by
>> > Drill and work after a new build. I believe I have only written scalar
>> > functions and not aggregations, but you can look at the SumFunctions
>> class
>> > for some reference on the differences.
>> >
>> > -Jason
>> >
>> > On Mon, Sep 22, 2014 at 7:50 AM, Yash Sharma <ya...@gmail.com> wrote:
>> >
>> > > Hi All,
>> > > I am planning to add new drill functions to optiq syntax for parsing.
>> > > We have new functions like regr_avgx/regr_avgy etc and existing ones
>> like
>> > > covar_samp,covar_pop, correlation/corr etc.
>> > >
>> > > Going through this old thread[1] I see that we need to add the
>> > definitions
>> > > in the CombinedParser.jj[2] in Optiq.
>> > >
>> > > Is that the way we still use or its handled differently now ?
>> > >
>> > > Thanks.
>> > >
>> > > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
>> > > 2.
>> > >
>> > >
>> >
>> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
>> > >
>> >
>>
>
>

Re: Adding new drill functions to optiq syntax

[Yash Sharma <ya...@gmail.com>]

Cool. So do we add the new function definitions in the CombinedParser.jj in
Optiq?
I see that we did something like that for standard deviation functions.


On Mon, Sep 22, 2014 at 8:49 PM, Jinfeng Ni <ji...@gmail.com> wrote:

> There are some differences between Drill simple functions and aggregate
> functions.
>
> For Drill simple function, optiq will check Drill function registry whether
> a function exists. So, all you need to is to provide the function template,
> the function converter will automatically add the  function template into
> function registry.
>
> Aggregate functions are different in that simple function could appear
> anywhere in the query, while aggregate function could be only in certain
> clause ( no aggregation function in where clause, etc).  Optiq have to know
> one certain function is aggregate function, in order to validate such
> logic.  Seems we either need modify optiq to let it know new function
> "regr_avgx"
> is an aggregate, or Drill need provide a way to let optiq know "regr_avgx"
> is aggregate function, not simple function.
>
>
>
> On Mon, Sep 22, 2014 at 7:42 AM, Jason Altekruse <altekrusejason@gmail.com
> >
> wrote:
>
> > Hi Yash,
> >
> > I did not work on the code for this, but if the functions are just acting
> > on columns to provide aggregates I believe they are just supported by
> > Drills default mechanism for tying into the parser. We ended up modifying
> > the parser with some FreeMarker hooks that allows for consumers of optiq
> to
> > provide custom syntax that extends the parser without forking the whole
> > thing.
> >
> > I believe that this works together with our function registry to provide
> > default support for UDFs (which our system functions are defined in the
> > same way as UDFs) without having to modify the parser itself. I haven't
> had
> > any trouble just adding new DrillFuncs and having them be discovered by
> > Drill and work after a new build. I believe I have only written scalar
> > functions and not aggregations, but you can look at the SumFunctions
> class
> > for some reference on the differences.
> >
> > -Jason
> >
> > On Mon, Sep 22, 2014 at 7:50 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> > > Hi All,
> > > I am planning to add new drill functions to optiq syntax for parsing.
> > > We have new functions like regr_avgx/regr_avgy etc and existing ones
> like
> > > covar_samp,covar_pop, correlation/corr etc.
> > >
> > > Going through this old thread[1] I see that we need to add the
> > definitions
> > > in the CombinedParser.jj[2] in Optiq.
> > >
> > > Is that the way we still use or its handled differently now ?
> > >
> > > Thanks.
> > >
> > > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> > > 2.
> > >
> > >
> >
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
> > >
> >
>

Re: Adding new drill functions to optiq syntax

[Jacques Nadeau <ja...@apache.org>]

Drill used to expose both scalar and aggregate functions correctly to
Optiq.  I'm confused on why it no longer does.

See here:
https://github.com/apache/incubator-drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFunctionRegistry.java#L79

Yash, can you debug Optiq to see why this isn't already working?

J

On Mon, Sep 22, 2014 at 8:19 AM, Jinfeng Ni <ji...@gmail.com> wrote:

> There are some differences between Drill simple functions and aggregate
> functions.
>
> For Drill simple function, optiq will check Drill function registry whether
> a function exists. So, all you need to is to provide the function template,
> the function converter will automatically add the  function template into
> function registry.
>
> Aggregate functions are different in that simple function could appear
> anywhere in the query, while aggregate function could be only in certain
> clause ( no aggregation function in where clause, etc).  Optiq have to know
> one certain function is aggregate function, in order to validate such
> logic.  Seems we either need modify optiq to let it know new function
> "regr_avgx"
> is an aggregate, or Drill need provide a way to let optiq know "regr_avgx"
> is aggregate function, not simple function.
>
>
>
> On Mon, Sep 22, 2014 at 7:42 AM, Jason Altekruse <altekrusejason@gmail.com
> >
> wrote:
>
> > Hi Yash,
> >
> > I did not work on the code for this, but if the functions are just acting
> > on columns to provide aggregates I believe they are just supported by
> > Drills default mechanism for tying into the parser. We ended up modifying
> > the parser with some FreeMarker hooks that allows for consumers of optiq
> to
> > provide custom syntax that extends the parser without forking the whole
> > thing.
> >
> > I believe that this works together with our function registry to provide
> > default support for UDFs (which our system functions are defined in the
> > same way as UDFs) without having to modify the parser itself. I haven't
> had
> > any trouble just adding new DrillFuncs and having them be discovered by
> > Drill and work after a new build. I believe I have only written scalar
> > functions and not aggregations, but you can look at the SumFunctions
> class
> > for some reference on the differences.
> >
> > -Jason
> >
> > On Mon, Sep 22, 2014 at 7:50 AM, Yash Sharma <ya...@gmail.com> wrote:
> >
> > > Hi All,
> > > I am planning to add new drill functions to optiq syntax for parsing.
> > > We have new functions like regr_avgx/regr_avgy etc and existing ones
> like
> > > covar_samp,covar_pop, correlation/corr etc.
> > >
> > > Going through this old thread[1] I see that we need to add the
> > definitions
> > > in the CombinedParser.jj[2] in Optiq.
> > >
> > > Is that the way we still use or its handled differently now ?
> > >
> > > Thanks.
> > >
> > > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> > > 2.
> > >
> > >
> >
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
> > >
> >
>

Re: Adding new drill functions to optiq syntax

[Jinfeng Ni <ji...@gmail.com>]

There are some differences between Drill simple functions and aggregate
functions.

For Drill simple function, optiq will check Drill function registry whether
a function exists. So, all you need to is to provide the function template,
the function converter will automatically add the  function template into
function registry.

Aggregate functions are different in that simple function could appear
anywhere in the query, while aggregate function could be only in certain
clause ( no aggregation function in where clause, etc).  Optiq have to know
one certain function is aggregate function, in order to validate such
logic.  Seems we either need modify optiq to let it know new function
"regr_avgx"
is an aggregate, or Drill need provide a way to let optiq know "regr_avgx"
is aggregate function, not simple function.



On Mon, Sep 22, 2014 at 7:42 AM, Jason Altekruse <al...@gmail.com>
wrote:

> Hi Yash,
>
> I did not work on the code for this, but if the functions are just acting
> on columns to provide aggregates I believe they are just supported by
> Drills default mechanism for tying into the parser. We ended up modifying
> the parser with some FreeMarker hooks that allows for consumers of optiq to
> provide custom syntax that extends the parser without forking the whole
> thing.
>
> I believe that this works together with our function registry to provide
> default support for UDFs (which our system functions are defined in the
> same way as UDFs) without having to modify the parser itself. I haven't had
> any trouble just adding new DrillFuncs and having them be discovered by
> Drill and work after a new build. I believe I have only written scalar
> functions and not aggregations, but you can look at the SumFunctions class
> for some reference on the differences.
>
> -Jason
>
> On Mon, Sep 22, 2014 at 7:50 AM, Yash Sharma <ya...@gmail.com> wrote:
>
> > Hi All,
> > I am planning to add new drill functions to optiq syntax for parsing.
> > We have new functions like regr_avgx/regr_avgy etc and existing ones like
> > covar_samp,covar_pop, correlation/corr etc.
> >
> > Going through this old thread[1] I see that we need to add the
> definitions
> > in the CombinedParser.jj[2] in Optiq.
> >
> > Is that the way we still use or its handled differently now ?
> >
> > Thanks.
> >
> > 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> > 2.
> >
> >
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
> >
>

Re: Adding new drill functions to optiq syntax

[Jason Altekruse <al...@gmail.com>]

Actually a quick follow up, mostly for anyone that finds this thread. It
isn't necessary to re-build drill to work with UDFs, if they can be found
anywhere on the classpath when Drill is running, they will be discovered
and added to the function registry which ties into the SQL validation in
optiq.

-Jason

On Mon, Sep 22, 2014 at 9:42 AM, Jason Altekruse <al...@gmail.com>
wrote:

> Hi Yash,
>
> I did not work on the code for this, but if the functions are just acting
> on columns to provide aggregates I believe they are just supported by
> Drills default mechanism for tying into the parser. We ended up modifying
> the parser with some FreeMarker hooks that allows for consumers of optiq to
> provide custom syntax that extends the parser without forking the whole
> thing.
>
> I believe that this works together with our function registry to provide
> default support for UDFs (which our system functions are defined in the
> same way as UDFs) without having to modify the parser itself. I haven't had
> any trouble just adding new DrillFuncs and having them be discovered by
> Drill and work after a new build. I believe I have only written scalar
> functions and not aggregations, but you can look at the SumFunctions class
> for some reference on the differences.
>
> -Jason
>
> On Mon, Sep 22, 2014 at 7:50 AM, Yash Sharma <ya...@gmail.com> wrote:
>
>> Hi All,
>> I am planning to add new drill functions to optiq syntax for parsing.
>> We have new functions like regr_avgx/regr_avgy etc and existing ones like
>> covar_samp,covar_pop, correlation/corr etc.
>>
>> Going through this old thread[1] I see that we need to add the definitions
>> in the CombinedParser.jj[2] in Optiq.
>>
>> Is that the way we still use or its handled differently now ?
>>
>> Thanks.
>>
>> 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
>> 2.
>>
>> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
>>
>
>

Re: Adding new drill functions to optiq syntax

[Jason Altekruse <al...@gmail.com>]

Hi Yash,

I did not work on the code for this, but if the functions are just acting
on columns to provide aggregates I believe they are just supported by
Drills default mechanism for tying into the parser. We ended up modifying
the parser with some FreeMarker hooks that allows for consumers of optiq to
provide custom syntax that extends the parser without forking the whole
thing.

I believe that this works together with our function registry to provide
default support for UDFs (which our system functions are defined in the
same way as UDFs) without having to modify the parser itself. I haven't had
any trouble just adding new DrillFuncs and having them be discovered by
Drill and work after a new build. I believe I have only written scalar
functions and not aggregations, but you can look at the SumFunctions class
for some reference on the differences.

-Jason

On Mon, Sep 22, 2014 at 7:50 AM, Yash Sharma <ya...@gmail.com> wrote:

> Hi All,
> I am planning to add new drill functions to optiq syntax for parsing.
> We have new functions like regr_avgx/regr_avgy etc and existing ones like
> covar_samp,covar_pop, correlation/corr etc.
>
> Going through this old thread[1] I see that we need to add the definitions
> in the CombinedParser.jj[2] in Optiq.
>
> Is that the way we still use or its handled differently now ?
>
> Thanks.
>
> 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> 2.
>
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj
>

Re: Adding new drill functions to optiq syntax

[Julian Hyde <ju...@gmail.com>]

If these functions don’t require new syntax — and it looks as if regr_avgx, for one, is just simple function syntax — then you don’t need to change the parser. If the parser needs semantic knowledge, such as whether regr_avgx is a regular function or an aggregate function, you may be able to pass that in via the SqlOperatorTable.

In short, very unlikely you need to parser.

Lastly, if you are adding SQL standard functionality to the parser or validator, please consider contributing them to Optiq rather than handling them in Drill-specific code.

Julian


On Sep 22, 2014, at 5:50 AM, Yash Sharma <ya...@gmail.com> wrote:

> Hi All,
> I am planning to add new drill functions to optiq syntax for parsing.
> We have new functions like regr_avgx/regr_avgy etc and existing ones like
> covar_samp,covar_pop, correlation/corr etc.
> 
> Going through this old thread[1] I see that we need to add the definitions
> in the CombinedParser.jj[2] in Optiq.
> 
> Is that the way we still use or its handled differently now ?
> 
> Thanks.
> 
> 1: https://groups.google.com/forum/#!topic/optiq-dev/dkkxsHh2MnE
> 2.
> https://github.com/yssharma/optiq/blob/retired/core/src/main/codegen/templates/CombinedParser.jj