You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Sushil Mohanty <su...@gmail.com> on 2015/01/14 12:29:07 UTC

Review Request 29881: LENS-131 : Query fails when constants projected

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

Review request for lens and Amareshwari Sriramadasu.


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


Repository: lens


Description
-------

When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.


Diffs
-----

  lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 

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


Testing
-------

Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.


Thanks,

Sushil Mohanty


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.

> On Jan. 20, 2015, 7:04 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, line 577
> > <https://reviews.apache.org/r/29881/diff/2/?file=824925#file824925line577>
> >
> >     Why are we putting NULL and TRUE in quotes? Arent they becoming strings with quotes?

NULL projected as column gives parsing error. Booleans work fine.


- Sushil


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


On Jan. 19, 2015, 7:24 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2015, 7:24 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment113062>

    Why are we putting NULL and TRUE in quotes? Arent they becoming strings with quotes?


- Amareshwari Sriramadasu


On Jan. 19, 2015, 7:24 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2015, 7:24 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.

> On Jan. 20, 2015, 8:45 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, line 50
> > <https://reviews.apache.org/r/29881/diff/3/?file=826092#file826092line50>
> >
> >     Can we undo this change?

This was due to the same.


> On Jan. 20, 2015, 8:45 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java, line 114
> > <https://reviews.apache.org/r/29881/diff/3/?file=826091#file826091line114>
> >
> >     I'm still thinking we would need check for column as part of single select expression available.
> >     
> >     Can we add test with constant + col? Especially constant first.

Okay. Added the checks.


- Sushil


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


On Jan. 20, 2015, 8:37 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 8:37 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.

> On Jan. 20, 2015, 8:45 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, lines 22-36
> > <https://reviews.apache.org/r/29881/diff/3/?file=826092#file826092line22>
> >
> >     Do we need all these imports? If so, can you combine them to .*

My IDE has Organize imports enabled,  which was exploding the "*" while saving.


- Sushil


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


On Jan. 20, 2015, 8:37 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 8:37 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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



lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java
<https://reviews.apache.org/r/29881/#comment113085>

    I'm still thinking we would need check for column as part of single select expression available.
    
    Can we add test with constant + col? Especially constant first.



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment113083>

    Do we need all these imports? If so, can you combine them to .*



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment113084>

    Can we undo this change?


- Amareshwari Sriramadasu


On Jan. 20, 2015, 8:37 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 8:37 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.

> On Jan. 21, 2015, 11:10 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java, line 118
> > <https://reviews.apache.org/r/29881/diff/6/?file=828190#file828190line118>
> >
> >     Do we need to iterate over child nodes? Since node passed here is single expression AST, above if check of hasTableOrColumn and returning true or false accordingly should work.
> >     
> >     Also, the boolean isConstant should not be required in the method param

iterate over child nodes not required, as its taken care in the hasTableOrColumn(new name).


- Sushil


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


On Jan. 21, 2015, 11:41 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 11:41 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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



lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java
<https://reviews.apache.org/r/29881/#comment113425>

    Do we need to iterate over child nodes? Since node passed here is single expression AST, above if check of hasTableOrColumn and returning true or false accordingly should work.
    
    Also, the boolean isConstant should not be required in the method param



lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java
<https://reviews.apache.org/r/29881/#comment113424>

    call it hasTableOrColumn ?


- Amareshwari Sriramadasu


On Jan. 21, 2015, 10:55 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 10:55 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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

Ship it!


Ship It!

- Amareshwari Sriramadasu


On Jan. 21, 2015, 11:41 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 11:41 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29881/
-----------------------------------------------------------

(Updated Jan. 21, 2015, 11:41 a.m.)


Review request for lens and Amareshwari Sriramadasu.


Changes
-------

Addressed review comments. Unit tests are running fine, complete build test is in progress.


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


Repository: lens


Description
-------

When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.


Diffs (updated)
-----

  lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 

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


Testing
-------

Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.


Thanks,

Sushil Mohanty


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29881/
-----------------------------------------------------------

(Updated Jan. 21, 2015, 10:55 a.m.)


Review request for lens and Amareshwari Sriramadasu.


Changes
-------

Added a method to check column or constants and add them to group by accordingly. Addressed the new review comments.

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Lens Checkstyle Rules ............................. SUCCESS [5.255s]
[INFO] Lens .............................................. SUCCESS [0.403s]
[INFO] Lens API .......................................... SUCCESS [9.272s]
[INFO] Lens API for server and extensions ................ SUCCESS [9.158s]
[INFO] Lens Cube ......................................... SUCCESS [10:22.629s]
[INFO] Lens DB storage ................................... SUCCESS [21.294s]
[INFO] Lens Query Library ................................ SUCCESS [7.524s]
[INFO] Lens Hive Driver .................................. SUCCESS [3:55.945s]
[INFO] Lens Driver for Cloudera Impala ................... SUCCESS [6.326s]
[INFO] Lens Driver for JDBC .............................. SUCCESS [35.656s]
[INFO] Lens Server ....................................... SUCCESS [9:23.056s]
[INFO] Lens client ....................................... SUCCESS [36.529s]
[INFO] Lens CLI .......................................... SUCCESS [2:36.283s]
[INFO] Lens Examples ..................................... SUCCESS [2.232s]
[INFO] Lens Distribution ................................. SUCCESS [2.076s]
[INFO] Lens Client Distribution .......................... SUCCESS [2.893s]
[INFO] Lens ML Lib ....................................... SUCCESS [1:22.679s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 30:00.035s
[INFO] Finished at: Wed Jan 21 16:05:43 GMT+05:30 2015
[INFO] Final Memory: 78M/1078M
[INFO] ------------------------------------------------------------------------


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


Repository: lens


Description
-------

When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.


Diffs (updated)
-----

  lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 

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


Testing
-------

Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.


Thanks,

Sushil Mohanty


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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

> On Jan. 21, 2015, 6:34 a.m., Rajat Khandelwal wrote:
> >

Please update the result of complete build.


- Rajat


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


On Jan. 20, 2015, 12:46 p.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 12:46 p.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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



lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java
<https://reviews.apache.org/r/29881/#comment113386>

    Make another method `isConstantsUsed(exprAST)` which calls `isConstantsUsed(exprAST, false)`. The reason being the method with 2 arguments is recursive, The calling function which is just going to start recursion should need no info about arguments passed between recursive calls.



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment113388>

    +1



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment113390>

    test cases for constants inside measures too?


- Rajat Khandelwal


On Jan. 20, 2015, 12:46 p.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 12:46 p.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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



lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java
<https://reviews.apache.org/r/29881/#comment113403>

    This check seems very specific to solving one case. What if it is multiplication or division?
    
     why are we doing this childcount == 2, check? What happens if the expression is 100 + 200, with code put here it is not a constant.


- Amareshwari Sriramadasu


On Jan. 20, 2015, 12:46 p.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 12:46 p.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29881/
-----------------------------------------------------------

(Updated Jan. 20, 2015, 12:46 p.m.)


Review request for lens and Amareshwari Sriramadasu.


Changes
-------

Last upload has some issue. Did it again.


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


Repository: lens


Description
-------

When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.


Diffs (updated)
-----

  lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 

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


Testing
-------

Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.


Thanks,

Sushil Mohanty


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29881/
-----------------------------------------------------------

(Updated Jan. 20, 2015, 11:48 a.m.)


Review request for lens and Amareshwari Sriramadasu.


Changes
-------

Addressed review comments.


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


Repository: lens


Description
-------

When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.


Diffs (updated)
-----

  lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 

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


Testing
-------

Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.


Thanks,

Sushil Mohanty


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29881/
-----------------------------------------------------------

(Updated Jan. 20, 2015, 8:37 a.m.)


Review request for lens and Amareshwari Sriramadasu.


Changes
-------

Addressed review comments.


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


Repository: lens


Description
-------

When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.


Diffs (updated)
-----

  lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 

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


Testing
-------

Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.


Thanks,

Sushil Mohanty


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29881/
-----------------------------------------------------------

(Updated Jan. 19, 2015, 7:24 a.m.)


Review request for lens and Amareshwari Sriramadasu.


Changes
-------

Addressed review comments.


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


Repository: lens


Description
-------

When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.


Diffs (updated)
-----

  lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 

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


Testing
-------

Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.


Thanks,

Sushil Mohanty


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.

> On Jan. 15, 2015, 10:16 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java, line 112
> > <https://reviews.apache.org/r/29881/diff/1/?file=820873#file820873line112>
> >
> >     We might have to return false for TABLE_OR_COL also?

HiveParser.TOK_FUNCTION will be good enough for the single row functions(input one row and output one row) like case, foramt etc used on columns. Columns with out function will be added to to "group by" by default. Test query "hqlQuery2" has both column and function.


- Sushil


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


On Jan. 14, 2015, 11:29 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 11:29 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.

> On Jan. 15, 2015, 10:16 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java, line 114
> > <https://reviews.apache.org/r/29881/diff/1/?file=820873#file820873line114>
> >
> >     Simple return false?

To exit recursion for true or false based on check, used the variable.


> On Jan. 15, 2015, 10:16 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, line 557
> > <https://reviews.apache.org/r/29881/diff/1/?file=820874#file820874line557>
> >
> >     Can you add more fields like column +100, boolean constants like true, NULL and etc ?

Okay, will add test cases to address other possible cases.


> On Jan. 15, 2015, 10:16 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java, line 83
> > <https://reviews.apache.org/r/29881/diff/1/?file=820873#file820873line83>
> >
> >     Move the !isConstantsUsed(exprAST, false)) here to avoid in both if and else?

Okay, will make the change.


- Sushil


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


On Jan. 14, 2015, 11:29 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 11:29 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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



lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java
<https://reviews.apache.org/r/29881/#comment112375>

    Move the !isConstantsUsed(exprAST, false)) here to avoid in both if and else?



lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java
<https://reviews.apache.org/r/29881/#comment112378>

    We might have to return false for TABLE_OR_COL also?



lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java
<https://reviews.apache.org/r/29881/#comment112376>

    Simple return false?



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment112377>

    Can you add more fields like column +100, boolean constants like true, NULL and etc ?


- Amareshwari Sriramadasu


On Jan. 14, 2015, 11:29 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 11:29 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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

> On Jan. 15, 2015, 9:35 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, line 23
> > <https://reviews.apache.org/r/29881/diff/1/?file=820874#file820874line23>
> >
> >     All the imports can be clubbed together with `*`

What is the criteria for using * vs expanded import? I dont think we put any hard rules around it right now, which causing this to and fro across multiple patches. Will create jira to decide on and update contributor doc with same


- Amareshwari


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


On Jan. 14, 2015, 11:29 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 11:29 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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

> On Jan. 15, 2015, 9:35 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, line 23
> > <https://reviews.apache.org/r/29881/diff/1/?file=820874#file820874line23>
> >
> >     All the imports can be clubbed together with `*`
> 
> Amareshwari Sriramadasu wrote:
>     What is the criteria for using * vs expanded import? I dont think we put any hard rules around it right now, which causing this to and fro across multiple patches. Will create jira to decide on and update contributor doc with same

Raised https://issues.apache.org/jira/browse/LENS-205 for the same


- Amareshwari


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


On Jan. 14, 2015, 11:29 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 11:29 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.

> On Jan. 15, 2015, 9:35 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, line 23
> > <https://reviews.apache.org/r/29881/diff/1/?file=820874#file820874line23>
> >
> >     All the imports can be clubbed together with `*`
> 
> Amareshwari Sriramadasu wrote:
>     What is the criteria for using * vs expanded import? I dont think we put any hard rules around it right now, which causing this to and fro across multiple patches. Will create jira to decide on and update contributor doc with same
> 
> Amareshwari Sriramadasu wrote:
>     Raised https://issues.apache.org/jira/browse/LENS-205 for the same

For me IDE does this magic.


- Sushil


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


On Jan. 14, 2015, 11:29 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 11:29 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

Posted by Sushil Mohanty <su...@gmail.com>.

> On Jan. 15, 2015, 9:35 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, line 557
> > <https://reviews.apache.org/r/29881/diff/1/?file=820874#file820874line557>
> >
> >     1. Test case where constant is used along with a column. e.g. col + 99
> >     2. project a column `case when stateid='za' then 99 else -1001 end`

Okay, will add test cases to address these.


- Sushil


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


On Jan. 14, 2015, 11:29 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 11:29 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>


Re: Review Request 29881: LENS-131 : Query fails when constants projected

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



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment112379>

    All the imports can be clubbed together with `*`



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment112380>

    Same thing here.



lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
<https://reviews.apache.org/r/29881/#comment112381>

    1. Test case where constant is used along with a column. e.g. col + 99
    2. project a column `case when stateid='za' then 99 else -1001 end`


- Rajat Khandelwal


On Jan. 14, 2015, 11:29 a.m., Sushil Mohanty wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29881/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 11:29 a.m.)
> 
> 
> Review request for lens and Amareshwari Sriramadasu.
> 
> 
> Bugs: LENS-131
>     https://issues.apache.org/jira/browse/LENS-131
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> When constants get projected the queries by default they were being added to group by. Such queries fail with No Such Column error.
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 2531c49 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java cb70b66 
> 
> Diff: https://reviews.apache.org/r/29881/diff/
> 
> 
> Testing
> -------
> 
> Tested with queries with different types of constants projected and rewriter skipping them in group by. Added unit test for the same.
> 
> 
> Thanks,
> 
> Sushil Mohanty
> 
>