Re: Review Request 52912: HIVE-13557: Make interval keyword optional while specifying DAY in interval arithmetic

2023-08-06 Thread Jude Dike 

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


Ship it!




Ship It!

- Jude Dike


On Nov. 15, 2016, 11:13 p.m., Zoltan Haindrich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52912/
> ---
> 
> (Updated Nov. 15, 2016, 11:13 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and pengcheng xiong.
> 
> 
> Bugs: HIVE-13557
> https://issues.apache.org/jira/browse/HIVE-13557
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * added intervalExpression
> * moved intervalLiteral into intervalExpression
> * added days/months/seconds/years
> * moved interval support into an udf, this way: it may accept column refs or 
> expressions as input (sql2011)
> 
> supported:
> 
> * 1 day
> * interval 1 day
> * (1+x) day
> * interval (1+x) day
> * '1-1' year to month
> * interval '1-1' year to month
> * interval ('1-' || x) year to month
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
> 0dbbc1d6226330bcb8271abba8b2675adaeb6f0f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 4357328c48dd721d02773557ad01e4ee572d6dd1 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> a82083be94b8ca7875c84f78a365fa12ba0f033f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 
> 5e708d3876cbe4bf14384b1798770311ae86401e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFInternalInterval.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestParseDriverIntervals.java 
> PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFInternalInterval.java
>  PRE-CREATION 
>   ql/src/test/queries/clientpositive/interval_alt.q PRE-CREATION 
>   ql/src/test/results/clientpositive/interval_alt.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/vector_interval_1.q.out 
> d8003baa5fa2ed0410d98d124ebf9bcee095598e 
>   ql/src/test/results/clientpositive/llap/vector_interval_arithmetic.q.out 
> 13a8b356cc1825fec2cdd4731040dd025c42dc54 
>   ql/src/test/results/clientpositive/show_functions.q.out 
> d2f3b97add4563b97a5f581efd6a0295a9a7db56 
>   ql/src/test/results/clientpositive/vector_interval_1.q.out 
> 373a6dec223be9c8257cdfeca2acb641426c13b5 
>   ql/src/test/results/clientpositive/vector_interval_arithmetic.q.out 
> ff16b3b62372f530ed570b5728c8ca06bfab6b98 
> 
> 
> Diff: https://reviews.apache.org/r/52912/diff/2/
> 
> 
> Testing
> ---
> 
> * small test to check parsable formats
> * small unit test to check exceptions
> * added qtest for use case tests
> 
> 
> Thanks,
> 
> Zoltan Haindrich
> 
>

Re: Review Request 52912: HIVE-13557: Make interval keyword optional while specifying DAY in interval arithmetic

2016-11-15 Thread Zoltan Haindrich 


> On Nov. 10, 2016, 12:21 a.m., pengcheng xiong wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g, line 348
> > 
> >
> > Then "select date '2012-01-01' + 1 days from t;" can not be supported 
> > as there is no parenthese.

i've moved closer to the standard..and currently this is supported


> On Nov. 10, 2016, 12:21 a.m., pengcheng xiong wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g, line 295
> > 
> >
> > I would rather put the new rewrite rule here for intervalLiteral. Here 
> > you can branch out, and say it can also match the new requirement. 
> > for example, expr ... etc.

I've tried to do this...and I've bumped into problems because; intervalLiteral 
became a constant later on..
...but because constant is used in skewed column definitions - and intervals 
contain expressions - this opens up a whole lot of antlr troubles.

I feel that "INTERVAL x days", where x is a column reference is more 
sophisticated than a constant

I can restore a similar intervalLiteral/intervalExpression as in patch#1, but 
in the current version there is only intervalExpression - I think this is 
better.


- Zoltan


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


On Nov. 15, 2016, 11:13 p.m., Zoltan Haindrich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52912/
> ---
> 
> (Updated Nov. 15, 2016, 11:13 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and pengcheng xiong.
> 
> 
> Bugs: HIVE-13557
> https://issues.apache.org/jira/browse/HIVE-13557
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * added intervalExpression
> * moved intervalLiteral into intervalExpression
> * added days/months/seconds/years
> * moved interval support into an udf, this way: it may accept column refs or 
> expressions as input (sql2011)
> 
> supported:
> 
> * 1 day
> * interval 1 day
> * (1+x) day
> * interval (1+x) day
> * '1-1' year to month
> * interval '1-1' year to month
> * interval ('1-' || x) year to month
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
> 0dbbc1d6226330bcb8271abba8b2675adaeb6f0f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 4357328c48dd721d02773557ad01e4ee572d6dd1 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> a82083be94b8ca7875c84f78a365fa12ba0f033f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 
> 5e708d3876cbe4bf14384b1798770311ae86401e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFInternalInterval.java
>  PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/parse/TestParseDriverIntervals.java 
> PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFInternalInterval.java
>  PRE-CREATION 
>   ql/src/test/queries/clientpositive/interval_alt.q PRE-CREATION 
>   ql/src/test/results/clientpositive/interval_alt.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/vector_interval_1.q.out 
> d8003baa5fa2ed0410d98d124ebf9bcee095598e 
>   ql/src/test/results/clientpositive/llap/vector_interval_arithmetic.q.out 
> 13a8b356cc1825fec2cdd4731040dd025c42dc54 
>   ql/src/test/results/clientpositive/show_functions.q.out 
> d2f3b97add4563b97a5f581efd6a0295a9a7db56 
>   ql/src/test/results/clientpositive/vector_interval_1.q.out 
> 373a6dec223be9c8257cdfeca2acb641426c13b5 
>   ql/src/test/results/clientpositive/vector_interval_arithmetic.q.out 
> ff16b3b62372f530ed570b5728c8ca06bfab6b98 
> 
> Diff: https://reviews.apache.org/r/52912/diff/
> 
> 
> Testing
> ---
> 
> * small test to check parsable formats
> * small unit test to check exceptions
> * added qtest for use case tests
> 
> 
> Thanks,
> 
> Zoltan Haindrich
> 
>

Re: Review Request 52912: HIVE-13557: Make interval keyword optional while specifying DAY in interval arithmetic

2016-11-15 Thread Zoltan Haindrich 

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

(Updated Nov. 15, 2016, 11:13 p.m.)


Review request for hive, Ashutosh Chauhan and pengcheng xiong.


Changes
---

remove parentheses requirement in simple cases


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


Repository: hive-git


Description (updated)
---

* added intervalExpression
* moved intervalLiteral into intervalExpression
* added days/months/seconds/years
* moved interval support into an udf, this way: it may accept column refs or 
expressions as input (sql2011)

supported:

* 1 day
* interval 1 day
* (1+x) day
* interval (1+x) day
* '1-1' year to month
* interval '1-1' year to month
* interval ('1-' || x) year to month


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
0dbbc1d6226330bcb8271abba8b2675adaeb6f0f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
4357328c48dd721d02773557ad01e4ee572d6dd1 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
a82083be94b8ca7875c84f78a365fa12ba0f033f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 
5e708d3876cbe4bf14384b1798770311ae86401e 
  
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFInternalInterval.java
 PRE-CREATION 
  ql/src/test/org/apache/hadoop/hive/ql/parse/TestParseDriverIntervals.java 
PRE-CREATION 
  
ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFInternalInterval.java
 PRE-CREATION 
  ql/src/test/queries/clientpositive/interval_alt.q PRE-CREATION 
  ql/src/test/results/clientpositive/interval_alt.q.out PRE-CREATION 
  ql/src/test/results/clientpositive/llap/vector_interval_1.q.out 
d8003baa5fa2ed0410d98d124ebf9bcee095598e 
  ql/src/test/results/clientpositive/llap/vector_interval_arithmetic.q.out 
13a8b356cc1825fec2cdd4731040dd025c42dc54 
  ql/src/test/results/clientpositive/show_functions.q.out 
d2f3b97add4563b97a5f581efd6a0295a9a7db56 
  ql/src/test/results/clientpositive/vector_interval_1.q.out 
373a6dec223be9c8257cdfeca2acb641426c13b5 
  ql/src/test/results/clientpositive/vector_interval_arithmetic.q.out 
ff16b3b62372f530ed570b5728c8ca06bfab6b98 

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


Testing (updated)
---

* small test to check parsable formats
* small unit test to check exceptions
* added qtest for use case tests


Thanks,

Zoltan Haindrich

Re: Review Request 52912: HIVE-13557: Make interval keyword optional while specifying DAY in interval arithmetic

2016-11-09 Thread pengcheng xiong 

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




ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g (line 294)


I would rather put the new rewrite rule here for intervalLiteral. Here you 
can branch out, and say it can also match the new requirement. 
for example, expr ... etc.



ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g (line 347)


Then "select date '2012-01-01' + 1 days from t;" can not be supported as 
there is no parenthese.


- pengcheng xiong


On Oct. 15, 2016, 9:27 p.m., Zoltan Haindrich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52912/
> ---
> 
> (Updated Oct. 15, 2016, 9:27 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-13557
> https://issues.apache.org/jira/browse/HIVE-13557
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> * added intervalExpression
> * syntax: INTERVAL (c) DAY
> * added days/months/seconds/years
> * kept the existing (interval '8' days) as is
> * INTERVAL keyword usage is optional
> * moving this into an udf seemed like a good thing, this way: it may accept 
> column refs or expressions as input (sql2011)
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 
> 0dbbc1d6226330bcb8271abba8b2675adaeb6f0f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 025ea10cda5b126f46226809884088caadc0a3d0 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> 13f68796e4fe262098eaf6c5f09bc7f963d0f8cf 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFInternalInterval.java
>  PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFInternalInterval.java
>  PRE-CREATION 
>   ql/src/test/queries/clientpositive/interval_alt.q PRE-CREATION 
>   ql/src/test/results/clientpositive/interval_alt.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52912/diff/
> 
> 
> Testing
> ---
> 
> * small unit test to check exceptions
> * added qtest
> 
> 
> Thanks,
> 
> Zoltan Haindrich
> 
>