Re: Review Request 34696: HIVE-686 add UDF substring_index

2015-05-29 Thread Sergio Pena

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

Ship it!


Looks good. Code is simple to read.

- Sergio Pena


On May 27, 2015, 3:35 a.m., Alexander Pivovarov wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34696/
 ---
 
 (Updated May 27, 2015, 3:35 a.m.)
 
 
 Review request for hive, Hao Cheng, Jason Dere, namit jain, and Thejas Nair.
 
 
 Bugs: HIVE-686
 https://issues.apache.org/jira/browse/HIVE-686
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 HIVE-686 add UDF substring_index
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
 94a3b1787e2b3571eb7a8102c28f7334ae3fa829 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java
  PRE-CREATION 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSubstringIndex.java
  PRE-CREATION 
   ql/src/test/queries/clientpositive/udf_substring_index.q PRE-CREATION 
   ql/src/test/results/clientpositive/show_functions.q.out 
 16820ca887320da13a42bebe0876f29eec373c8f 
   ql/src/test/results/clientpositive/udf_substring_index.q.out PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alexander Pivovarov
 




Re: Review Request 34696: HIVE-686 add UDF substring_index

2015-05-27 Thread Alexander Pivovarov


 On May 27, 2015, 4:42 a.m., Swarnim Kulkarni wrote:
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java,
   line 45
  https://reviews.apache.org/r/34696/diff/1/?file=972489#file972489line45
 
  Worth mentinoning in your example what the expected output would look 
  like?
 
 Alexander Pivovarov wrote:
 Not sure I got the issue...
 
 --- desc output
 hive desc function extended substring_index;
 OK
 ...
 Example:
   SELECT substring_index('www.apache.org', '.', 2);
  'www.apache'
 
 
 -- actual select
 hive SELECT substring_index('www.apache.org', '.', 2);
 OK
 www.apache
 
 Swarnim Kulkarni wrote:
 My point was just that why not also include a sample result what the 
 users could expect to see after this command is executed. Might improve the 
 readability a bit.

it's included. The result is 'www.apache' - right adter \n symbol


- Alexander


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


On May 27, 2015, 3:35 a.m., Alexander Pivovarov wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34696/
 ---
 
 (Updated May 27, 2015, 3:35 a.m.)
 
 
 Review request for hive, Hao Cheng, Jason Dere, namit jain, and Thejas Nair.
 
 
 Bugs: HIVE-686
 https://issues.apache.org/jira/browse/HIVE-686
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 HIVE-686 add UDF substring_index
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
 94a3b1787e2b3571eb7a8102c28f7334ae3fa829 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java
  PRE-CREATION 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSubstringIndex.java
  PRE-CREATION 
   ql/src/test/queries/clientpositive/udf_substring_index.q PRE-CREATION 
   ql/src/test/results/clientpositive/show_functions.q.out 
 16820ca887320da13a42bebe0876f29eec373c8f 
   ql/src/test/results/clientpositive/udf_substring_index.q.out PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alexander Pivovarov
 




Re: Review Request 34696: HIVE-686 add UDF substring_index

2015-05-27 Thread Swarnim Kulkarni


 On May 27, 2015, 4:42 a.m., Swarnim Kulkarni wrote:
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java,
   line 45
  https://reviews.apache.org/r/34696/diff/1/?file=972489#file972489line45
 
  Worth mentinoning in your example what the expected output would look 
  like?
 
 Alexander Pivovarov wrote:
 Not sure I got the issue...
 
 --- desc output
 hive desc function extended substring_index;
 OK
 ...
 Example:
   SELECT substring_index('www.apache.org', '.', 2);
  'www.apache'
 
 
 -- actual select
 hive SELECT substring_index('www.apache.org', '.', 2);
 OK
 www.apache

My point was just that why not also include a sample result what the users 
could expect to see after this command is executed. Might improve the 
readability a bit.


- Swarnim


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


On May 27, 2015, 3:35 a.m., Alexander Pivovarov wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34696/
 ---
 
 (Updated May 27, 2015, 3:35 a.m.)
 
 
 Review request for hive, Hao Cheng, Jason Dere, namit jain, and Thejas Nair.
 
 
 Bugs: HIVE-686
 https://issues.apache.org/jira/browse/HIVE-686
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 HIVE-686 add UDF substring_index
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
 94a3b1787e2b3571eb7a8102c28f7334ae3fa829 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java
  PRE-CREATION 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSubstringIndex.java
  PRE-CREATION 
   ql/src/test/queries/clientpositive/udf_substring_index.q PRE-CREATION 
   ql/src/test/results/clientpositive/show_functions.q.out 
 16820ca887320da13a42bebe0876f29eec373c8f 
   ql/src/test/results/clientpositive/udf_substring_index.q.out PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alexander Pivovarov
 




Re: Review Request 34696: HIVE-686 add UDF substring_index

2015-05-27 Thread Swarnim Kulkarni


 On May 27, 2015, 4:42 a.m., Swarnim Kulkarni wrote:
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java,
   line 45
  https://reviews.apache.org/r/34696/diff/1/?file=972489#file972489line45
 
  Worth mentinoning in your example what the expected output would look 
  like?
 
 Alexander Pivovarov wrote:
 Not sure I got the issue...
 
 --- desc output
 hive desc function extended substring_index;
 OK
 ...
 Example:
   SELECT substring_index('www.apache.org', '.', 2);
  'www.apache'
 
 
 -- actual select
 hive SELECT substring_index('www.apache.org', '.', 2);
 OK
 www.apache
 
 Swarnim Kulkarni wrote:
 My point was just that why not also include a sample result what the 
 users could expect to see after this command is executed. Might improve the 
 readability a bit.
 
 Alexander Pivovarov wrote:
 it's included. The result is 'www.apache' - right adter \n symbol

Ah ok. Sorry missed that.


- Swarnim


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


On May 27, 2015, 3:35 a.m., Alexander Pivovarov wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34696/
 ---
 
 (Updated May 27, 2015, 3:35 a.m.)
 
 
 Review request for hive, Hao Cheng, Jason Dere, namit jain, and Thejas Nair.
 
 
 Bugs: HIVE-686
 https://issues.apache.org/jira/browse/HIVE-686
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 HIVE-686 add UDF substring_index
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
 94a3b1787e2b3571eb7a8102c28f7334ae3fa829 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java
  PRE-CREATION 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSubstringIndex.java
  PRE-CREATION 
   ql/src/test/queries/clientpositive/udf_substring_index.q PRE-CREATION 
   ql/src/test/results/clientpositive/show_functions.q.out 
 16820ca887320da13a42bebe0876f29eec373c8f 
   ql/src/test/results/clientpositive/udf_substring_index.q.out PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alexander Pivovarov
 




Re: Review Request 34696: HIVE-686 add UDF substring_index

2015-05-26 Thread Alexander Pivovarov


 On May 27, 2015, 4:42 a.m., Swarnim Kulkarni wrote:
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java,
   line 45
  https://reviews.apache.org/r/34696/diff/1/?file=972489#file972489line45
 
  Worth mentinoning in your example what the expected output would look 
  like?

Not sure I got the issue...

--- desc output
hive desc function extended substring_index;
OK
...
Example:
  SELECT substring_index('www.apache.org', '.', 2);
 'www.apache'


-- actual select
hive SELECT substring_index('www.apache.org', '.', 2);
OK
www.apache


- Alexander


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


On May 27, 2015, 3:35 a.m., Alexander Pivovarov wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34696/
 ---
 
 (Updated May 27, 2015, 3:35 a.m.)
 
 
 Review request for hive, Hao Cheng, Jason Dere, namit jain, and Thejas Nair.
 
 
 Bugs: HIVE-686
 https://issues.apache.org/jira/browse/HIVE-686
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 HIVE-686 add UDF substring_index
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
 94a3b1787e2b3571eb7a8102c28f7334ae3fa829 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java
  PRE-CREATION 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSubstringIndex.java
  PRE-CREATION 
   ql/src/test/queries/clientpositive/udf_substring_index.q PRE-CREATION 
   ql/src/test/results/clientpositive/show_functions.q.out 
 16820ca887320da13a42bebe0876f29eec373c8f 
   ql/src/test/results/clientpositive/udf_substring_index.q.out PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alexander Pivovarov
 




Review Request 34696: HIVE-686 add UDF substring_index

2015-05-26 Thread Alexander Pivovarov

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

Review request for hive, Hao Cheng, Jason Dere, namit jain, and Thejas Nair.


Bugs: HIVE-686
https://issues.apache.org/jira/browse/HIVE-686


Repository: hive-git


Description
---

HIVE-686 add UDF substring_index


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
94a3b1787e2b3571eb7a8102c28f7334ae3fa829 
  
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java 
PRE-CREATION 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSubstringIndex.java
 PRE-CREATION 
  ql/src/test/queries/clientpositive/udf_substring_index.q PRE-CREATION 
  ql/src/test/results/clientpositive/show_functions.q.out 
16820ca887320da13a42bebe0876f29eec373c8f 
  ql/src/test/results/clientpositive/udf_substring_index.q.out PRE-CREATION 

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


Testing
---


Thanks,

Alexander Pivovarov



Re: Review Request 34696: HIVE-686 add UDF substring_index

2015-05-26 Thread Swarnim Kulkarni

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



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java
https://reviews.apache.org/r/34696/#comment136875

Worth mentinoning in your example what the expected output would look like?


- Swarnim Kulkarni


On May 27, 2015, 3:35 a.m., Alexander Pivovarov wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34696/
 ---
 
 (Updated May 27, 2015, 3:35 a.m.)
 
 
 Review request for hive, Hao Cheng, Jason Dere, namit jain, and Thejas Nair.
 
 
 Bugs: HIVE-686
 https://issues.apache.org/jira/browse/HIVE-686
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 HIVE-686 add UDF substring_index
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
 94a3b1787e2b3571eb7a8102c28f7334ae3fa829 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java
  PRE-CREATION 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSubstringIndex.java
  PRE-CREATION 
   ql/src/test/queries/clientpositive/udf_substring_index.q PRE-CREATION 
   ql/src/test/results/clientpositive/show_functions.q.out 
 16820ca887320da13a42bebe0876f29eec373c8f 
   ql/src/test/results/clientpositive/udf_substring_index.q.out PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alexander Pivovarov
 




Re: Review Request 34696: HIVE-686 add UDF substring_index

2015-05-26 Thread Swarnim Kulkarni

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

Ship it!


Ship It!

- Swarnim Kulkarni


On May 27, 2015, 3:35 a.m., Alexander Pivovarov wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34696/
 ---
 
 (Updated May 27, 2015, 3:35 a.m.)
 
 
 Review request for hive, Hao Cheng, Jason Dere, namit jain, and Thejas Nair.
 
 
 Bugs: HIVE-686
 https://issues.apache.org/jira/browse/HIVE-686
 
 
 Repository: hive-git
 
 
 Description
 ---
 
 HIVE-686 add UDF substring_index
 
 
 Diffs
 -
 
   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
 94a3b1787e2b3571eb7a8102c28f7334ae3fa829 
   
 ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSubstringIndex.java
  PRE-CREATION 
   
 ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSubstringIndex.java
  PRE-CREATION 
   ql/src/test/queries/clientpositive/udf_substring_index.q PRE-CREATION 
   ql/src/test/results/clientpositive/show_functions.q.out 
 16820ca887320da13a42bebe0876f29eec373c8f 
   ql/src/test/results/clientpositive/udf_substring_index.q.out PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34696/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Alexander Pivovarov