You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Harish Butani <rh...@gmail.com> on 2013/10/22 22:52:22 UTC
Review Request 14856: HIVE-5552: Merging of QBJoinTrees doesn't handle
filter pushdowns correctly
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14856/
-----------------------------------------------------------
Review request for hive and Ashutosh Chauhan.
Bugs: HIVE-5552
https://issues.apache.org/jira/browse/HIVE-5552
Repository: hive-git
Description
-------
Hive currently only support views in the FROM-clause, some Facebook use cases suggest that Hive should support subqueries such as those connected by IN/EXISTS in the WHERE-clause.
Diffs
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java 9834e73
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 063aa65
ql/src/test/queries/clientpositive/join_merging.q PRE-CREATION
ql/src/test/results/clientpositive/join_merging.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/14856/diff/
Testing
-------
ran all join tests.
added join_merging.q test
Thanks,
Harish Butani
Re: Review Request 14856: HIVE-5552: Merging of QBJoinTrees doesn't handle
filter pushdowns correctly
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14856/#review27478
-----------------------------------------------------------
Ship it!
Ship It!
- Ashutosh Chauhan
On Oct. 24, 2013, 6:16 p.m., Harish Butani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14856/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2013, 6:16 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Bugs: HIVE-5552
> https://issues.apache.org/jira/browse/HIVE-5552
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Merging of QBJoinTrees doesn't handle filter pushdowns correctly
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 12a0a69
> ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java 9834e73
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java c886760
> ql/src/test/queries/clientpositive/join_merging.q PRE-CREATION
> ql/src/test/results/clientpositive/join_merging.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14856/diff/
>
>
> Testing
> -------
>
> ran all join tests.
> added join_merging.q test
>
>
> Thanks,
>
> Harish Butani
>
>
Re: Review Request 14856: HIVE-5552: Merging of QBJoinTrees doesn't handle
filter pushdowns correctly
Posted by Harish Butani <rh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14856/
-----------------------------------------------------------
(Updated Oct. 24, 2013, 6:16 p.m.)
Review request for hive and Ashutosh Chauhan.
Bugs: HIVE-5552
https://issues.apache.org/jira/browse/HIVE-5552
Repository: hive-git
Description (updated)
-------
Merging of QBJoinTrees doesn't handle filter pushdowns correctly
Diffs (updated)
-----
ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 12a0a69
ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java 9834e73
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java c886760
ql/src/test/queries/clientpositive/join_merging.q PRE-CREATION
ql/src/test/results/clientpositive/join_merging.q.out PRE-CREATION
Diff: https://reviews.apache.org/r/14856/diff/
Testing
-------
ran all join tests.
added join_merging.q test
Thanks,
Harish Butani
Re: Review Request 14856: HIVE-5552: Merging of QBJoinTrees doesn't handle
filter pushdowns correctly
Posted by Harish Butani <rh...@gmail.com>.
> On Oct. 24, 2013, 5:45 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 1403
> > <https://reviews.apache.org/r/14856/diff/1/?file=369330#file369330line1403>
> >
> > Also, I think following may be needed:
> > * HiveParser.KW_ARRAY
> > * HiveParser.KW_STRUCT
> > * HiveParser.KW_MAP
these are created as Function Nodes in the AST. So get handled as any Function, by the default case in checkJoinFilterRefersOneAlias
- Harish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14856/#review27465
-----------------------------------------------------------
On Oct. 24, 2013, 6:16 p.m., Harish Butani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14856/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2013, 6:16 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Bugs: HIVE-5552
> https://issues.apache.org/jira/browse/HIVE-5552
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Merging of QBJoinTrees doesn't handle filter pushdowns correctly
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 12a0a69
> ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java 9834e73
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java c886760
> ql/src/test/queries/clientpositive/join_merging.q PRE-CREATION
> ql/src/test/results/clientpositive/join_merging.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14856/diff/
>
>
> Testing
> -------
>
> ran all join tests.
> added join_merging.q test
>
>
> Thanks,
>
> Harish Butani
>
>
Re: Review Request 14856: HIVE-5552: Merging of QBJoinTrees doesn't handle
filter pushdowns correctly
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14856/#review27465
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/14856/#comment53386>
Also, I think following may be needed:
* HiveParser.KW_ARRAY
* HiveParser.KW_STRUCT
* HiveParser.KW_MAP
- Ashutosh Chauhan
On Oct. 22, 2013, 8:52 p.m., Harish Butani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14856/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2013, 8:52 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Bugs: HIVE-5552
> https://issues.apache.org/jira/browse/HIVE-5552
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Hive currently only support views in the FROM-clause, some Facebook use cases suggest that Hive should support subqueries such as those connected by IN/EXISTS in the WHERE-clause.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java 9834e73
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 063aa65
> ql/src/test/queries/clientpositive/join_merging.q PRE-CREATION
> ql/src/test/results/clientpositive/join_merging.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14856/diff/
>
>
> Testing
> -------
>
> ran all join tests.
> added join_merging.q test
>
>
> Thanks,
>
> Harish Butani
>
>
Re: Review Request 14856: HIVE-5552: Merging of QBJoinTrees doesn't handle
filter pushdowns correctly
Posted by Harish Butani <rh...@gmail.com>.
> On Oct. 24, 2013, 3:57 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 1397
> > <https://reviews.apache.org/r/14856/diff/1/?file=369330#file369330line1397>
> >
> > I am bit confused. Is it tablename.colname or colname ? Looks like former for test cases you added, so how come we are able to determine correct index for it in joinTree.baseSrc list, since that I presume will only contain table aliases?
this gets handled by a recursive call in checkJoinFilterRefersOneAlias
- Harish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14856/#review27457
-----------------------------------------------------------
On Oct. 24, 2013, 6:16 p.m., Harish Butani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14856/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2013, 6:16 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Bugs: HIVE-5552
> https://issues.apache.org/jira/browse/HIVE-5552
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Merging of QBJoinTrees doesn't handle filter pushdowns correctly
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 12a0a69
> ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java 9834e73
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java c886760
> ql/src/test/queries/clientpositive/join_merging.q PRE-CREATION
> ql/src/test/results/clientpositive/join_merging.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14856/diff/
>
>
> Testing
> -------
>
> ran all join tests.
> added join_merging.q test
>
>
> Thanks,
>
> Harish Butani
>
>
Re: Review Request 14856: HIVE-5552: Merging of QBJoinTrees doesn't handle
filter pushdowns correctly
Posted by Ashutosh Chauhan <ha...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14856/#review27457
-----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java
<https://reviews.apache.org/r/14856/#comment53352>
Better name: postJoinFilters ?
ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java
<https://reviews.apache.org/r/14856/#comment53353>
Instead of this lazy initialization, its just better to initialize eagerly and not do this null check. Makes code more succinct.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/14856/#comment53354>
Better to move this function as a static function in ParseUtils to avoid further lengthening of SemanticAnalyzer.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/14856/#comment53364>
Also, instead of QBJoinTree, this method just need to accept String[] aliases from it, that is joinTree.getBaseSrc(). No need to pass QBJoinTree in it.
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/14856/#comment53356>
I am bit confused. Is it tablename.colname or colname ? Looks like former for test cases you added, so how come we are able to determine correct index for it in joinTree.baseSrc list, since that I presume will only contain table aliases?
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/14856/#comment53360>
Do we also need to add following ?
* HiveParser.TOK_NULL:
* HiveParser.TOK_DATELITERAL:
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/14856/#comment53361>
Per comment above, we could have this null check, if this list is always initialized with 0 size. There is nothing perf sensitive here, so code clarity is preferred.
- Ashutosh Chauhan
On Oct. 22, 2013, 8:52 p.m., Harish Butani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14856/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2013, 8:52 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Bugs: HIVE-5552
> https://issues.apache.org/jira/browse/HIVE-5552
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> Hive currently only support views in the FROM-clause, some Facebook use cases suggest that Hive should support subqueries such as those connected by IN/EXISTS in the WHERE-clause.
>
>
> Diffs
> -----
>
> ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java 9834e73
> ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 063aa65
> ql/src/test/queries/clientpositive/join_merging.q PRE-CREATION
> ql/src/test/results/clientpositive/join_merging.q.out PRE-CREATION
>
> Diff: https://reviews.apache.org/r/14856/diff/
>
>
> Testing
> -------
>
> ran all join tests.
> added join_merging.q test
>
>
> Thanks,
>
> Harish Butani
>
>