You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Rajat Khandelwal <ra...@gmail.com> on 2016/01/08 14:26:47 UTC

Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

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

Review request for lens.


Bugs: LENS-813
    https://issues.apache.org/jira/browse/LENS-813


Repository: lens


Description
-------


Diffs
-----

  lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 1fd1d17bbc05b2ca07d88237b4143ea3cd904d0e 
  lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 26514d86776ff55f5a674ae51d1066100e797e10 
  lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java bfb65c7bde8f6d9ec963714cf1cb9e292ed3ae9c 
  lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
  lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java ac5632823d8a55f7e25b5058167e40fc7e6aedaf 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 5b44f9581ab1d57239b86738963ff519c0e94a9a 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On Jan. 11, 2016, 10:01 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java, lines 191-198
> > <https://reviews.apache.org/r/42070/diff/1/?file=1187458#file1187458line191>
> >
> >     Why is additional loop required?
> 
> Rajat Khandelwal wrote:
>     Fact's select tree has original query's select tree and extra select expressisons picked from having expression. The first loop is over select expressions 0 to `cubeql.getselecttree.getchildcount`. This loop is over the extra columns picked from having.
> 
> Amareshwari Sriramadasu wrote:
>     Reopening this, as I dont see why this loop is required just after the loop above.

Removed now.


- Rajat


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


On Feb. 1, 2016, 6:12 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 6:12 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java c9aff5da2566e6a2c96e0e0592a533e5e504888a 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Amareshwari Sriramadasu <am...@apache.org>.

> On Jan. 11, 2016, 4:31 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java, lines 191-198
> > <https://reviews.apache.org/r/42070/diff/1/?file=1187458#file1187458line191>
> >
> >     Why is additional loop required?
> 
> Rajat Khandelwal wrote:
>     Fact's select tree has original query's select tree and extra select expressisons picked from having expression. The first loop is over select expressions 0 to `cubeql.getselecttree.getchildcount`. This loop is over the extra columns picked from having.

Reopening this, as I dont see why this loop is required just after the loop above.


- Amareshwari


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


On Jan. 27, 2016, 7:51 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2016, 7:51 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On Jan. 11, 2016, 10:01 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java, lines 363-374
> > <https://reviews.apache.org/r/42070/diff/1/?file=1187464#file1187464line363>
> >
> >     Having clauses should be pushed down to corresponding fact queries. Should not be converted to where clause.
> 
> Rajat Khandelwal wrote:
>     Right now we have correctness issue. Push down of having clause has certain requirements:
>     
>     1. The entire boolean expression of having clause should be a bunch of sub-clauses being `AND`ed together.
>     2. Each of those sub-clauses should be completely evaluable by one of the facts. ''
>     
>     Hence I've implemented a `correct` solution first, will take pushdown as an improvement.

I've added pushdown feature. Now, first push down is tried, if not possible of some parts of the clause, then it's converted to outer where while adding components to select expressions of inner sub-queries.


> On Jan. 11, 2016, 10:01 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java, line 893
> > <https://reviews.apache.org/r/42070/diff/1/?file=1187459#file1187459line893>
> >
> >     Why is having clause converted to where?
> 
> Rajat Khandelwal wrote:
>     Because aggregation is already done. Inner having conditions have to be written as where conditions in the outer query.

This is now only the not-push-downable having clauses.


- Rajat


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


On Jan. 25, 2016, 6:21 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2016, 6:21 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 3e930ded95b5b975373d1035be61bf8360a45807 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 100d7c9cceb5feb98c5e88a688e364298d6543be 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On Jan. 11, 2016, 10:01 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java, lines 191-198
> > <https://reviews.apache.org/r/42070/diff/1/?file=1187458#file1187458line191>
> >
> >     Why is additional loop required?

Fact's select tree has original query's select tree and extra select expressisons picked from having expression. The first loop is over select expressions 0 to `cubeql.getselecttree.getchildcount`. This loop is over the extra columns picked from having.


> On Jan. 11, 2016, 10:01 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java, line 893
> > <https://reviews.apache.org/r/42070/diff/1/?file=1187459#file1187459line893>
> >
> >     Why is having clause converted to where?

Because aggregation is already done. Inner having conditions have to be written as where conditions in the outer query.


> On Jan. 11, 2016, 10:01 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java, lines 363-374
> > <https://reviews.apache.org/r/42070/diff/1/?file=1187464#file1187464line363>
> >
> >     Having clauses should be pushed down to corresponding fact queries. Should not be converted to where clause.

Right now we have correctness issue. Push down of having clause has certain requirements:

1. The entire boolean expression of having clause should be a bunch of sub-clauses being `AND`ed together.
2. Each of those sub-clauses should be completely evaluable by one of the facts. ''

Hence I've implemented a `correct` solution first, will take pushdown as an improvement.


- Rajat


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


On Jan. 8, 2016, 6:56 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2016, 6:56 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 1fd1d17bbc05b2ca07d88237b4143ea3cd904d0e 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 26514d86776ff55f5a674ae51d1066100e797e10 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java bfb65c7bde8f6d9ec963714cf1cb9e292ed3ae9c 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java ac5632823d8a55f7e25b5058167e40fc7e6aedaf 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 5b44f9581ab1d57239b86738963ff519c0e94a9a 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/#review113673
-----------------------------------------------------------



lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java (lines 188 - 195)
<https://reviews.apache.org/r/42070/#comment174426>

    Why is additional loop required?



lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java (lines 197 - 198)
<https://reviews.apache.org/r/42070/#comment174425>

    whereAST will be usually common for all facts. The TODO here can be removed.



lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java (line 200)
<https://reviews.apache.org/r/42070/#comment174427>

    Comment needs update if havingAST is getitng update elsewhere.



lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java (line 891)
<https://reviews.apache.org/r/42070/#comment174428>

    Why is having clause converted to where?



lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java (lines 363 - 374)
<https://reviews.apache.org/r/42070/#comment174429>

    Having clauses should be pushed down to corresponding fact queries. Should not be converted to where clause.


- Amareshwari Sriramadasu


On Jan. 8, 2016, 1:26 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2016, 1:26 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 1fd1d17bbc05b2ca07d88237b4143ea3cd904d0e 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 26514d86776ff55f5a674ae51d1066100e797e10 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java bfb65c7bde8f6d9ec963714cf1cb9e292ed3ae9c 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java ac5632823d8a55f7e25b5058167e40fc7e6aedaf 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 5b44f9581ab1d57239b86738963ff519c0e94a9a 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Amareshwari Sriramadasu <am...@apache.org>.

> On Feb. 1, 2016, 11:41 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java, line 582
> > <https://reviews.apache.org/r/42070/diff/5/?file=1222557#file1222557line582>
> >
> >     Add test with only one having clause
> 
> Rajat Khandelwal wrote:
>     I think such cases should already be covered in other tests. I'll add anyway.

I meant adding one having clause on a multi fact query. It would fail without this fix.


- Amareshwari


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


On Feb. 1, 2016, 12:42 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 12:42 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java c9aff5da2566e6a2c96e0e0592a533e5e504888a 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On Feb. 1, 2016, 5:11 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java, line 176
> > <https://reviews.apache.org/r/42070/diff/5/?file=1222555#file1222555line176>
> >
> >     Check if this can happen.

alias null will be in cases like round(sum(msr1)+avg(msr2)). In such cases we have to fall back to recursion. Changed the code to that effect.


> On Feb. 1, 2016, 5:11 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java, line 582
> > <https://reviews.apache.org/r/42070/diff/5/?file=1222557#file1222557line582>
> >
> >     Add test with only one having clause

I think such cases should already be covered in other tests. I'll add anyway.


- Rajat


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


On Feb. 1, 2016, 6:12 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 6:12 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java c9aff5da2566e6a2c96e0e0592a533e5e504888a 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/#review117195
-----------------------------------------------------------




lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java (line 131)
<https://reviews.apache.org/r/42070/#comment178309>

    Seems it should be equal instead of contains.
    
    Also, please check if ASTNode.equals is better than converting to string.



lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java (line 176)
<https://reviews.apache.org/r/42070/#comment178315>

    Check if this can happen.



lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java (line 582)
<https://reviews.apache.org/r/42070/#comment178316>

    Add test with only one having clause


- Amareshwari Sriramadasu


On Jan. 27, 2016, 7:51 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2016, 7:51 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On Jan. 29, 2016, 5:26 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java, line 500
> > <https://reviews.apache.org/r/42070/diff/5/?file=1222553#file1222553line500>
> >
> >     Why is this change required?

Removed, residual changes from when it was WIP.


- Rajat


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


On Feb. 1, 2016, 6:12 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 6:12 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java c9aff5da2566e6a2c96e0e0592a533e5e504888a 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/#review116942
-----------------------------------------------------------




lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java (line 500)
<https://reviews.apache.org/r/42070/#comment178024>

    Why is this change required?


- Amareshwari Sriramadasu


On Jan. 27, 2016, 7:51 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2016, 7:51 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/#review117206
-----------------------------------------------------------


Ship it!




Ship It!

- Amareshwari Sriramadasu


On Feb. 1, 2016, 12:42 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 12:42 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java c9aff5da2566e6a2c96e0e0592a533e5e504888a 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/
-----------------------------------------------------------

(Updated Feb. 1, 2016, 7:18 p.m.)


Review request for lens.


Bugs: LENS-813
    https://issues.apache.org/jira/browse/LENS-813


Repository: lens


Description
-------


Diffs (updated)
-----

  lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
  lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
  lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
  lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java c9aff5da2566e6a2c96e0e0592a533e5e504888a 
  lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
  lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/
-----------------------------------------------------------

(Updated Feb. 1, 2016, 6:12 p.m.)


Review request for lens.


Bugs: LENS-813
    https://issues.apache.org/jira/browse/LENS-813


Repository: lens


Description
-------


Diffs (updated)
-----

  lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
  lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
  lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
  lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java c9aff5da2566e6a2c96e0e0592a533e5e504888a 
  lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
  lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/
-----------------------------------------------------------

(Updated Jan. 27, 2016, 1:21 p.m.)


Review request for lens.


Bugs: LENS-813
    https://issues.apache.org/jira/browse/LENS-813


Repository: lens


Description
-------


Diffs (updated)
-----

  lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
  lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
  lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
  lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
  lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
  lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
  lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/
-----------------------------------------------------------

(Updated Jan. 27, 2016, 1:15 p.m.)


Review request for lens.


Bugs: LENS-813
    https://issues.apache.org/jira/browse/LENS-813


Repository: lens


Description
-------


Diffs (updated)
-----

  lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
  lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
  lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
  lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
  lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
  lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
  lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
  lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
  lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 100d7c9cceb5feb98c5e88a688e364298d6543be 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On Jan. 27, 2016, 10:30 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java, line 128
> > <https://reviews.apache.org/r/42070/diff/3/?file=1221272#file1221272line128>
> >
> >     Can you explain whats happening here?
> >     
> >     HQLParser.getString() is a costly operation on AST, should be called sparingly.

Trying to figure out whether the ast is in the select tree or not. So two trees have to be traversed and compared. Comparing strings allows me to compare the traversal outputs.


> On Jan. 27, 2016, 10:30 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java, line 186
> > <https://reviews.apache.org/r/42070/diff/3/?file=1221272#file1221272line186>
> >
> >     Why is it catching LensException ?

Removed.


- Rajat


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


On Jan. 26, 2016, 12:20 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2016, 12:20 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 100d7c9cceb5feb98c5e88a688e364298d6543be 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.

> On Jan. 27, 2016, 10:30 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java, line 128
> > <https://reviews.apache.org/r/42070/diff/3/?file=1221272#file1221272line128>
> >
> >     Can you explain whats happening here?
> >     
> >     HQLParser.getString() is a costly operation on AST, should be called sparingly.
> 
> Rajat Khandelwal wrote:
>     Trying to figure out whether the ast is in the select tree or not. So two trees have to be traversed and compared. Comparing strings allows me to compare the traversal outputs.

Doing AST comparisons now instead of string comparisons.


- Rajat


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


On Feb. 1, 2016, 6:12 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2016, 6:12 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java c9aff5da2566e6a2c96e0e0592a533e5e504888a 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/#review116536
-----------------------------------------------------------




lens-api/src/main/resources/lens-errors.conf (line 326)
<https://reviews.apache.org/r/42070/#comment177543>

    Is this a metastore error? Does not look like. If not, it should moved to query errors i.e. < 3100.



lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java (line 65)
<https://reviews.apache.org/r/42070/#comment177544>

    Should be 3034



lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java (line 128)
<https://reviews.apache.org/r/42070/#comment177545>

    Can you explain whats happening here?
    
    HQLParser.getString() is a costly operation on AST, should be called sparingly.



lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java (line 183)
<https://reviews.apache.org/r/42070/#comment177546>

    Why is it catching LensException ?


- Amareshwari Sriramadasu


On Jan. 26, 2016, 6:50 a.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42070/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2016, 6:50 a.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-813
>     https://issues.apache.org/jira/browse/LENS-813
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
>   lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 100d7c9cceb5feb98c5e88a688e364298d6543be 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 
> 
> Diff: https://reviews.apache.org/r/42070/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/
-----------------------------------------------------------

(Updated Jan. 26, 2016, 12:20 p.m.)


Review request for lens.


Bugs: LENS-813
    https://issues.apache.org/jira/browse/LENS-813


Repository: lens


Description
-------


Diffs (updated)
-----

  lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
  lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
  lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
  lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 79dd88c8d2f94b0bb2a9c12fe3ccc3e39e655972 
  lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
  lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
  lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
  lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
  lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 100d7c9cceb5feb98c5e88a688e364298d6543be 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 

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


Testing
-------


Thanks,

Rajat Khandelwal


Re: Review Request 42070: LENS-813: For multifact queries, having clauses are getting added to both sub queries.

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42070/
-----------------------------------------------------------

(Updated Jan. 25, 2016, 6:21 p.m.)


Review request for lens.


Bugs: LENS-813
    https://issues.apache.org/jira/browse/LENS-813


Repository: lens


Description
-------


Diffs (updated)
-----

  lens-api/src/main/resources/lens-errors.conf 9087fcd09f67c462c68f5385a58360bb8a16374b 
  lens-client/src/main/java/org/apache/lens/client/LensClient.java f7f99c7c20b97e9f2b53c39ee147d5cc682f93dc 
  lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 61d08b2769deb554eb3dc2d992b7a6b8ba3484b8 
  lens-cube/src/main/java/org/apache/lens/cube/parse/AliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java c305244c98ff38a0acaa24812ccec9d2910e0095 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 3e930ded95b5b975373d1035be61bf8360a45807 
  lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultAliasDecider.java PRE-CREATION 
  lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 5ff265db155363938f3b657256bb232056e50f78 
  lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java b1deb0753fe36d2908ed4c8a98178a93644fab64 
  lens-cube/src/main/java/org/apache/lens/cube/parse/MultiFactHQLContext.java 1a729f8eab5cb3f8ab2eeb9bf32dc0c3efc05620 
  lens-cube/src/main/java/org/apache/lens/cube/parse/SingleFactMultiStorageHQLContext.java 7e3a0bfa703d27f38c5b2a5935d7d2da07125b7c 
  lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 100d7c9cceb5feb98c5e88a688e364298d6543be 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 57a15e23a39cf09e1f167740083b43bb154334eb 

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


Testing
-------


Thanks,

Rajat Khandelwal