You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Sergey Shelukhin <se...@hortonworks.com> on 2014/09/16 19:23:17 UTC

Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

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

Review request for hive, Ashutosh Chauhan and John Pullokkaran.


Repository: hive-git


Description
-------

see jira


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
  ql/src/test/queries/clientpositive/create_func1.q ad924d3 
  ql/src/test/results/clientpositive/create_func1.q.out 798f77f 

Diff: https://reviews.apache.org/r/25700/diff/


Testing
-------


Thanks,

Sergey Shelukhin


Re: Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

Posted by John Pullokkaran <jp...@hortonworks.com>.

> On Sept. 17, 2014, 11:50 p.m., John Pullokkaran wrote:
> > Ship It!

Conditional on cbo correctness test pass


- John


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


On Sept. 16, 2014, 5:23 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25700/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 5:23 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
>   ql/src/test/queries/clientpositive/create_func1.q ad924d3 
>   ql/src/test/results/clientpositive/create_func1.q.out 798f77f 
> 
> Diff: https://reviews.apache.org/r/25700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25700/#review53774
-----------------------------------------------------------

Ship it!


Ship It!

- John Pullokkaran


On Sept. 16, 2014, 5:23 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25700/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 5:23 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
>   ql/src/test/queries/clientpositive/create_func1.q ad924d3 
>   ql/src/test/results/clientpositive/create_func1.q.out 798f77f 
> 
> Diff: https://reviews.apache.org/r/25700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

Posted by John Pullokkaran <jp...@hortonworks.com>.

> On Sept. 16, 2014, 5:30 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java, line 646
> > <https://reviews.apache.org/r/25700/diff/1/?file=690720#file690720line646>
> >
> >     are all functions qualified in hive (w.r.t DB)
> >     How about built in functions like "toLower"?
> >     Could you say <DB NAME>.toLower()?

Also could you run few of the q test below and see if your change causes problems
authorization_create_func1.q	show_functions.q		vectorized_string_funcs.q
create_func1.q			vector_decimal_math_funcs.q	vectorized_timestamp_funcs.q
drop_function.q			vectorized_date_funcs.q
show_describe_func_quotes.q	vectorized_math_funcs.q


- John


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


On Sept. 16, 2014, 5:23 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25700/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 5:23 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
>   ql/src/test/queries/clientpositive/create_func1.q ad924d3 
>   ql/src/test/results/clientpositive/create_func1.q.out 798f77f 
> 
> Diff: https://reviews.apache.org/r/25700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Sept. 16, 2014, 5:30 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java, line 646
> > <https://reviews.apache.org/r/25700/diff/1/?file=690720#file690720line646>
> >
> >     are all functions qualified in hive (w.r.t DB)
> >     How about built in functions like "toLower"?
> >     Could you say <DB NAME>.toLower()?
> 
> John Pullokkaran wrote:
>     Also could you run few of the q test below and see if your change causes problems
>     authorization_create_func1.q	show_functions.q		vectorized_string_funcs.q
>     create_func1.q			vector_decimal_math_funcs.q	vectorized_timestamp_funcs.q
>     drop_function.q			vectorized_date_funcs.q
>     show_describe_func_quotes.q	vectorized_math_funcs.q
> 
> Sergey Shelukhin wrote:
>     this is covered by the 2nd part of the condition (if function is located w/o qualified name, just the name is returned)
>     
>     Will run the tests

Ran the tests; there are some out file changes, but they are the same as on current cbo branch.


- Sergey


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


On Sept. 16, 2014, 5:23 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25700/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 5:23 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
>   ql/src/test/queries/clientpositive/create_func1.q ad924d3 
>   ql/src/test/results/clientpositive/create_func1.q.out 798f77f 
> 
> Diff: https://reviews.apache.org/r/25700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Sept. 16, 2014, 5:30 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java, line 646
> > <https://reviews.apache.org/r/25700/diff/1/?file=690720#file690720line646>
> >
> >     are all functions qualified in hive (w.r.t DB)
> >     How about built in functions like "toLower"?
> >     Could you say <DB NAME>.toLower()?
> 
> John Pullokkaran wrote:
>     Also could you run few of the q test below and see if your change causes problems
>     authorization_create_func1.q	show_functions.q		vectorized_string_funcs.q
>     create_func1.q			vector_decimal_math_funcs.q	vectorized_timestamp_funcs.q
>     drop_function.q			vectorized_date_funcs.q
>     show_describe_func_quotes.q	vectorized_math_funcs.q
> 
> Sergey Shelukhin wrote:
>     this is covered by the 2nd part of the condition (if function is located w/o qualified name, just the name is returned)
>     
>     Will run the tests
> 
> Sergey Shelukhin wrote:
>     Ran the tests; there are some out file changes, but they are the same as on current cbo branch.
> 
> John Pullokkaran wrote:
>     it seems like the change would always use qualified function name.
>     If thats the case would built in functions work?
>     For example in the select statement could you always qualify functions with db name. What about arithmetic expressions, conjunctive/disjunctive functions (ad/or)?
>     It seems like your change would qualify thos functions with DB name.
>     
>     What is that i am missing?

the check is unctionUtils.isQualifiedFunctionName(fn) || mFunctions.get(fn) != null; 2nd part takes care of the built-in functions. It does exactly the same as getFunctionInfo


- Sergey


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


On Sept. 16, 2014, 5:23 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25700/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 5:23 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
>   ql/src/test/queries/clientpositive/create_func1.q ad924d3 
>   ql/src/test/results/clientpositive/create_func1.q.out 798f77f 
> 
> Diff: https://reviews.apache.org/r/25700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

Posted by John Pullokkaran <jp...@hortonworks.com>.

> On Sept. 16, 2014, 5:30 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java, line 646
> > <https://reviews.apache.org/r/25700/diff/1/?file=690720#file690720line646>
> >
> >     are all functions qualified in hive (w.r.t DB)
> >     How about built in functions like "toLower"?
> >     Could you say <DB NAME>.toLower()?
> 
> John Pullokkaran wrote:
>     Also could you run few of the q test below and see if your change causes problems
>     authorization_create_func1.q	show_functions.q		vectorized_string_funcs.q
>     create_func1.q			vector_decimal_math_funcs.q	vectorized_timestamp_funcs.q
>     drop_function.q			vectorized_date_funcs.q
>     show_describe_func_quotes.q	vectorized_math_funcs.q
> 
> Sergey Shelukhin wrote:
>     this is covered by the 2nd part of the condition (if function is located w/o qualified name, just the name is returned)
>     
>     Will run the tests
> 
> Sergey Shelukhin wrote:
>     Ran the tests; there are some out file changes, but they are the same as on current cbo branch.

it seems like the change would always use qualified function name.
If thats the case would built in functions work?
For example in the select statement could you always qualify functions with db name. What about arithmetic expressions, conjunctive/disjunctive functions (ad/or)?
It seems like your change would qualify thos functions with DB name.

What is that i am missing?


- John


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


On Sept. 16, 2014, 5:23 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25700/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 5:23 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
>   ql/src/test/queries/clientpositive/create_func1.q ad924d3 
>   ql/src/test/results/clientpositive/create_func1.q.out 798f77f 
> 
> Diff: https://reviews.apache.org/r/25700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

Posted by Sergey Shelukhin <se...@hortonworks.com>.

> On Sept. 16, 2014, 5:30 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java, line 644
> > <https://reviews.apache.org/r/25700/diff/1/?file=690720#file690720line644>
> >
> >     Is hive token case insensitive or all function names are in lower case?

see get... and register... methods in FunctionRegistry; when storing or retrieving, they are all made lower case


> On Sept. 16, 2014, 5:30 p.m., John Pullokkaran wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java, line 646
> > <https://reviews.apache.org/r/25700/diff/1/?file=690720#file690720line646>
> >
> >     are all functions qualified in hive (w.r.t DB)
> >     How about built in functions like "toLower"?
> >     Could you say <DB NAME>.toLower()?
> 
> John Pullokkaran wrote:
>     Also could you run few of the q test below and see if your change causes problems
>     authorization_create_func1.q	show_functions.q		vectorized_string_funcs.q
>     create_func1.q			vector_decimal_math_funcs.q	vectorized_timestamp_funcs.q
>     drop_function.q			vectorized_date_funcs.q
>     show_describe_func_quotes.q	vectorized_math_funcs.q

this is covered by the 2nd part of the condition (if function is located w/o qualified name, just the name is returned)

Will run the tests


- Sergey


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


On Sept. 16, 2014, 5:23 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25700/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 5:23 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
>   ql/src/test/queries/clientpositive/create_func1.q ad924d3 
>   ql/src/test/results/clientpositive/create_func1.q.out 798f77f 
> 
> Diff: https://reviews.apache.org/r/25700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>


Re: Review Request 25700: HIVE-8080 CBO: function name may not match UDF name during translation

Posted by John Pullokkaran <jp...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25700/#review53546
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java
<https://reviews.apache.org/r/25700/#comment93239>

    Is hive token case insensitive or all function names are in lower case?



ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java
<https://reviews.apache.org/r/25700/#comment93240>

    are all functions qualified in hive (w.r.t DB)
    How about built in functions like "toLower"?
    Could you say <DB NAME>.toLower()?


- John Pullokkaran


On Sept. 16, 2014, 5:23 p.m., Sergey Shelukhin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25700/
> -----------------------------------------------------------
> 
> (Updated Sept. 16, 2014, 5:23 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and John Pullokkaran.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java c503bbb 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/RexNodeConverter.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/optiq/translator/SqlFunctionConverter.java PRE-CREATION 
>   ql/src/test/queries/clientpositive/create_func1.q ad924d3 
>   ql/src/test/results/clientpositive/create_func1.q.out 798f77f 
> 
> Diff: https://reviews.apache.org/r/25700/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>