Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
Hive >> mail # dev >> Review Request 15718: HIVE-5614: Subquery support: allow subquery expressions in having clause


+
Harish Butani 2013-11-20, 18:04
Copy link to this message
-
Re: Review Request 15718: HIVE-5614: Subquery support: allow subquery expressions in having clause

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

ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java
<https://reviews.apache.org/r/15718/#comment56454>

    It will be good to add a comment about various fields in Conjunct class.

ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java
<https://reviews.apache.org/r/15718/#comment56455>

    Can this constructor be package protected instead of public ?

ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java
<https://reviews.apache.org/r/15718/#comment56456>

    protected instead of public?

ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java
<https://reviews.apache.org/r/15718/#comment56457>

    It will be good to add a comment, how behavior of ConjunctAnalyzer changes when forHavingClause = true instead of false.

ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java
<https://reviews.apache.org/r/15718/#comment56458>

    Should this exception needs to be propagated up the stack. At the least, we should have LOG.warn() message here.

ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/15718/#comment56452>

    It will be good to add a comment here along the lines of
     there could be a subq in having clause, if so we need to generate subq plan followed by semi-join.

ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
<https://reviews.apache.org/r/15718/#comment56459>

    It will be good to add a comment how this new boolean changes behavior of this method.

ql/src/test/queries/clientpositive/subquery_in_having.q
<https://reviews.apache.org/r/15718/#comment56453>

    It will be good to add a test which has a subq in both where clause as well as having clause

ql/src/test/queries/clientpositive/subquery_in_having.q
<https://reviews.apache.org/r/15718/#comment56448>

    Same comment w.r.t map-join on. Also, if we support over clause in subq, it will be good to have a test for that.

ql/src/test/queries/clientpositive/subquery_notexists_having.q
<https://reviews.apache.org/r/15718/#comment56449>

    It will be good to add a negative test where subq and outer query both uses same table alias. It seems in such cases we may generate incorrect results, so we should disable those.

ql/src/test/results/clientpositive/subquery_in_having.q.out
<https://reviews.apache.org/r/15718/#comment56450>

    In this plan, we are first computing outq, then subq and then doing left semi-join on resultset of those two. As we discussed efficient way for this is to push filter conditions in subq to outer query to cut-down the output generated by outq. Though, I am not sure whether its better to do it in optimizer phase via Transformer or right here. Either ways, I think thats an optimization which we can do as a follow-up.

ql/src/test/results/clientpositive/subquery_notexists_having.q.out
<https://reviews.apache.org/r/15718/#comment56451>

    First expression in this filter is redundant. Thats not strictly required. However, since there is an active work going on for constant folding optimization, this may get optimized way via that optimization. Either way, this can be done in follow-up.
- Ashutosh Chauhan
On Nov. 20, 2013, 6:04 p.m., Harish Butani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15718/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2013, 6:04 p.m.)
>
>
> Review request for hive and Ashutosh Chauhan.
>
>
> Bugs: HIVE-5614
>     https://issues.apache.org/jira/browse/HIVE-5614
>
>
> Repository: hive-git
>
>
> Description
> -------
>
> support for subquery predicates in having clause. SubTask of HIVE-784
>
>
> Diffs
> -----
+
Harish Butani 2013-11-23, 00:17
+
Harish Butani 2013-11-23, 00:14
+
Ashutosh Chauhan 2013-11-23, 00:54