You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Vineet Garg <vg...@hortonworks.com> on 2017/04/04 17:27:11 UTC
Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct from'
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/
-----------------------------------------------------------
Review request for hive, Ashutosh Chauhan and Jes�s Camacho Rodr�guez.
Bugs: HIVE-15986
https://issues.apache.org/jira/browse/HIVE-15986
Repository: hive-git
Description
-------
This patch adds support for 'is distinct from' and 'is not distinct from'.
Diffs
-----
itests/src/test/resources/testconfiguration.properties 7a70c9c
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPIsDistinctFrom.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFOPIsNotDistinctFrom.java PRE-CREATION
ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/58181/diff/1/
Testing
-------
Added new tests
Pre-commit testing
Thanks,
Vineet Garg
Re: Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct
from'
Posted by Vineet Garg <vg...@hortonworks.com>.
> On April 5, 2017, 3:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g
> > Lines 590-592 (patched)
> > <https://reviews.apache.org/r/58181/diff/2/?file=1684715#file1684715line590>
> >
> > It will be good to not add extra tokens in grammar as it increases the size of state machine. How about:
> >
> >
> > KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
> > -> {$a !=null}? ^(EQUAL_NS)
> > -> ^(KW_NOT EQUAL_NS)
I agree and I tried not to add new tokens but I couldn't figure out how to write grammar in such a way to avoid adding it.
For is distinct from we want AST as follows
KW_NOT
EQUAL_NS
Expr1
Expr2
isDistinctFrom rule is invoked from precedenceEqualOperator which is suppose to return an AST for operator. This AST is further used by precedenceEqualOperator's invoker to make an AST with returned AST as root and with two expression as it's children. So if isDistinctFrom return this AST
KW_NOT
EQUAL_NS
invoker of precedenceEqualOperator will end up creating
KW_NOT
EQUAL_NS
Expr1
Expr2
which is not what we want.
Your above suggestion throws an exception while parsing FAILED: RewriteEmptyStreamException token KW_NOT.
I am not sure why
- Vineet
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/#review171125
-----------------------------------------------------------
On April 4, 2017, 11:05 p.m., Vineet Garg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58181/
> -----------------------------------------------------------
>
> (Updated April 4, 2017, 11:05 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan and Jes�s Camacho Rodr�guez.
>
>
> Bugs: HIVE-15986
> https://issues.apache.org/jira/browse/HIVE-15986
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This patch adds support for 'is distinct from' and 'is not distinct from'.
>
>
> Diffs
> -----
>
> itests/src/test/resources/testconfiguration.properties 7a70c9c
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
> ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
> ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
> ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
> ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58181/diff/2/
>
>
> Testing
> -------
>
> Added new tests
> Pre-commit testing
>
>
> Thanks,
>
> Vineet Garg
>
>
Re: Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct
from'
Posted by pengcheng xiong <px...@hortonworks.com>.
> On April 5, 2017, 3:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g
> > Lines 590-592 (patched)
> > <https://reviews.apache.org/r/58181/diff/2/?file=1684715#file1684715line590>
> >
> > It will be good to not add extra tokens in grammar as it increases the size of state machine. How about:
> >
> >
> > KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
> > -> {$a !=null}? ^(EQUAL_NS)
> > -> ^(KW_NOT EQUAL_NS)
>
> Vineet Garg wrote:
> I agree and I tried not to add new tokens but I couldn't figure out how to write grammar in such a way to avoid adding it.
>
> For is distinct from we want AST as follows
> KW_NOT
> EQUAL_NS
> Expr1
> Expr2
>
> isDistinctFrom rule is invoked from precedenceEqualOperator which is suppose to return an AST for operator. This AST is further used by precedenceEqualOperator's invoker to make an AST with returned AST as root and with two expression as it's children. So if isDistinctFrom return this AST
> KW_NOT
> EQUAL_NS
>
> invoker of precedenceEqualOperator will end up creating
> KW_NOT
> EQUAL_NS
> Expr1
> Expr2
>
> which is not what we want.
>
> Your above suggestion throws an exception while parsing FAILED: RewriteEmptyStreamException token KW_NOT.
>
> I am not sure why
can u try
KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
-> {$a !=null}? ^(EQUAL_NS)
-> ^(KW_NOT ^EQUAL_NS)
- pengcheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/#review171125
-----------------------------------------------------------
On April 6, 2017, 8:02 p.m., Vineet Garg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58181/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 8:02 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Jes�s Camacho Rodr�guez, and Pengcheng Xu.
>
>
> Bugs: HIVE-15986
> https://issues.apache.org/jira/browse/HIVE-15986
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This patch adds support for 'is distinct from' and 'is not distinct from'.
>
>
> Diffs
> -----
>
> itests/src/test/resources/testconfiguration.properties 7a70c9c
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
> ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
> ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java f979c14
> ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
> ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out 68e248a
> ql/src/test/results/clientpositive/udf_equal.q.out 52bd843
>
>
> Diff: https://reviews.apache.org/r/58181/diff/4/
>
>
> Testing
> -------
>
> Added new tests
> Pre-commit testing
>
>
> Thanks,
>
> Vineet Garg
>
>
Re: Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct
from'
Posted by pengcheng xiong <px...@hortonworks.com>.
> On April 5, 2017, 3:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g
> > Lines 590-592 (patched)
> > <https://reviews.apache.org/r/58181/diff/2/?file=1684715#file1684715line590>
> >
> > It will be good to not add extra tokens in grammar as it increases the size of state machine. How about:
> >
> >
> > KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
> > -> {$a !=null}? ^(EQUAL_NS)
> > -> ^(KW_NOT EQUAL_NS)
>
> Vineet Garg wrote:
> I agree and I tried not to add new tokens but I couldn't figure out how to write grammar in such a way to avoid adding it.
>
> For is distinct from we want AST as follows
> KW_NOT
> EQUAL_NS
> Expr1
> Expr2
>
> isDistinctFrom rule is invoked from precedenceEqualOperator which is suppose to return an AST for operator. This AST is further used by precedenceEqualOperator's invoker to make an AST with returned AST as root and with two expression as it's children. So if isDistinctFrom return this AST
> KW_NOT
> EQUAL_NS
>
> invoker of precedenceEqualOperator will end up creating
> KW_NOT
> EQUAL_NS
> Expr1
> Expr2
>
> which is not what we want.
>
> Your above suggestion throws an exception while parsing FAILED: RewriteEmptyStreamException token KW_NOT.
>
> I am not sure why
>
> pengcheng xiong wrote:
> can u try
> KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
> -> {$a !=null}? ^(EQUAL_NS)
> -> ^(KW_NOT ^EQUAL_NS)
>
> Vineet Garg wrote:
> I get compilation error
> error(100): IdentifiersParser.g:592:17: syntax error: antlr: MismatchedTokenException
sorry, how about this?
KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
-> {$a !=null}? ^(EQUAL_NS)
-> ^(KW_NOT ^(EQUAL_NS))
- pengcheng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/#review171125
-----------------------------------------------------------
On April 6, 2017, 8:02 p.m., Vineet Garg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58181/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 8:02 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Jes�s Camacho Rodr�guez, and Pengcheng Xu.
>
>
> Bugs: HIVE-15986
> https://issues.apache.org/jira/browse/HIVE-15986
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This patch adds support for 'is distinct from' and 'is not distinct from'.
>
>
> Diffs
> -----
>
> itests/src/test/resources/testconfiguration.properties 7a70c9c
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
> ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
> ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java f979c14
> ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
> ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out 68e248a
> ql/src/test/results/clientpositive/udf_equal.q.out 52bd843
>
>
> Diff: https://reviews.apache.org/r/58181/diff/4/
>
>
> Testing
> -------
>
> Added new tests
> Pre-commit testing
>
>
> Thanks,
>
> Vineet Garg
>
>
Re: Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct
from'
Posted by Vineet Garg <vg...@hortonworks.com>.
> On April 5, 2017, 3:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g
> > Lines 590-592 (patched)
> > <https://reviews.apache.org/r/58181/diff/2/?file=1684715#file1684715line590>
> >
> > It will be good to not add extra tokens in grammar as it increases the size of state machine. How about:
> >
> >
> > KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
> > -> {$a !=null}? ^(EQUAL_NS)
> > -> ^(KW_NOT EQUAL_NS)
>
> Vineet Garg wrote:
> I agree and I tried not to add new tokens but I couldn't figure out how to write grammar in such a way to avoid adding it.
>
> For is distinct from we want AST as follows
> KW_NOT
> EQUAL_NS
> Expr1
> Expr2
>
> isDistinctFrom rule is invoked from precedenceEqualOperator which is suppose to return an AST for operator. This AST is further used by precedenceEqualOperator's invoker to make an AST with returned AST as root and with two expression as it's children. So if isDistinctFrom return this AST
> KW_NOT
> EQUAL_NS
>
> invoker of precedenceEqualOperator will end up creating
> KW_NOT
> EQUAL_NS
> Expr1
> Expr2
>
> which is not what we want.
>
> Your above suggestion throws an exception while parsing FAILED: RewriteEmptyStreamException token KW_NOT.
>
> I am not sure why
>
> pengcheng xiong wrote:
> can u try
> KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
> -> {$a !=null}? ^(EQUAL_NS)
> -> ^(KW_NOT ^EQUAL_NS)
I get compilation error
error(100): IdentifiersParser.g:592:17: syntax error: antlr: MismatchedTokenException
- Vineet
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/#review171125
-----------------------------------------------------------
On April 6, 2017, 8:02 p.m., Vineet Garg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58181/
> -----------------------------------------------------------
>
> (Updated April 6, 2017, 8:02 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan, Jes�s Camacho Rodr�guez, and Pengcheng Xu.
>
>
> Bugs: HIVE-15986
> https://issues.apache.org/jira/browse/HIVE-15986
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This patch adds support for 'is distinct from' and 'is not distinct from'.
>
>
> Diffs
> -----
>
> itests/src/test/resources/testconfiguration.properties 7a70c9c
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
> ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
> ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
> ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java f979c14
> ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
> ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
> ql/src/test/results/clientpositive/show_functions.q.out 68e248a
> ql/src/test/results/clientpositive/udf_equal.q.out 52bd843
>
>
> Diff: https://reviews.apache.org/r/58181/diff/4/
>
>
> Testing
> -------
>
> Added new tests
> Pre-commit testing
>
>
> Thanks,
>
> Vineet Garg
>
>
Re: Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct
from'
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/#review171125
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g
Lines 590-592 (patched)
<https://reviews.apache.org/r/58181/#comment244016>
It will be good to not add extra tokens in grammar as it increases the size of state machine. How about:
KW_IS (a=KW_NOT)? KW_DISTINCT KW_FROM
-> {$a !=null}? ^(EQUAL_NS)
-> ^(KW_NOT EQUAL_NS)
- Ashutosh Chauhan
On April 4, 2017, 11:05 p.m., Vineet Garg wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58181/
> -----------------------------------------------------------
>
> (Updated April 4, 2017, 11:05 p.m.)
>
>
> Review request for hive, Ashutosh Chauhan and Jes�s Camacho Rodr�guez.
>
>
> Bugs: HIVE-15986
> https://issues.apache.org/jira/browse/HIVE-15986
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> This patch adds support for 'is distinct from' and 'is not distinct from'.
>
>
> Diffs
> -----
>
> itests/src/test/resources/testconfiguration.properties 7a70c9c
> ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
> ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
> ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
> ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
> ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58181/diff/2/
>
>
> Testing
> -------
>
> Added new tests
> Pre-commit testing
>
>
> Thanks,
>
> Vineet Garg
>
>
Re: Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct
from'
Posted by Vineet Garg <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/
-----------------------------------------------------------
(Updated April 6, 2017, 5:38 a.m.)
Review request for hive, Ashutosh Chauhan and Jes�s Camacho Rodr�guez.
Bugs: HIVE-15986
https://issues.apache.org/jira/browse/HIVE-15986
Repository: hive-git
Description
-------
This patch adds support for 'is distinct from' and 'is not distinct from'.
Diffs (updated)
-----
itests/src/test/resources/testconfiguration.properties 7a70c9c
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java f979c14
ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
ql/src/test/results/clientpositive/show_functions.q.out 68e248a
ql/src/test/results/clientpositive/udf_equal.q.out 52bd843
Diff: https://reviews.apache.org/r/58181/diff/4/
Changes: https://reviews.apache.org/r/58181/diff/3-4/
Testing
-------
Added new tests
Pre-commit testing
Thanks,
Vineet Garg
Re: Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct
from'
Posted by Vineet Garg <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/
-----------------------------------------------------------
(Updated April 5, 2017, 9:53 p.m.)
Review request for hive, Ashutosh Chauhan and Jes�s Camacho Rodr�guez.
Changes
-------
Test fix
Bugs: HIVE-15986
https://issues.apache.org/jira/browse/HIVE-15986
Repository: hive-git
Description
-------
This patch adds support for 'is distinct from' and 'is not distinct from'.
Diffs (updated)
-----
itests/src/test/resources/testconfiguration.properties 7a70c9c
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java f979c14
ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
ql/src/test/results/clientpositive/show_functions.q.out 68e248a
ql/src/test/results/clientpositive/udf_equal.q.out 52bd843
Diff: https://reviews.apache.org/r/58181/diff/3/
Changes: https://reviews.apache.org/r/58181/diff/2-3/
Testing
-------
Added new tests
Pre-commit testing
Thanks,
Vineet Garg
Re: Review Request 58181: HIVE-15986 Support for 'is [NOT] distinct
from'
Posted by Vineet Garg <vg...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58181/
-----------------------------------------------------------
(Updated April 4, 2017, 11:05 p.m.)
Review request for hive, Ashutosh Chauhan and Jes�s Camacho Rodr�guez.
Changes
-------
See JIRA for the description
Bugs: HIVE-15986
https://issues.apache.org/jira/browse/HIVE-15986
Repository: hive-git
Description
-------
This patch adds support for 'is distinct from' and 'is not distinct from'.
Diffs (updated)
-----
itests/src/test/resources/testconfiguration.properties 7a70c9c
ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java ccfb455
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 85450c9
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g d98a663
ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 8c4ee8a
ql/src/test/queries/clientpositive/is_distinct_from.q PRE-CREATION
ql/src/test/results/clientpositive/llap/is_distinct_from.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/58181/diff/2/
Changes: https://reviews.apache.org/r/58181/diff/1-2/
Testing
-------
Added new tests
Pre-commit testing
Thanks,
Vineet Garg