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/08 11:35:28 UTC
Review Request 29705: Lens-169 : Add distinct keyword with columns
projected for dimension only queries
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29705/
-----------------------------------------------------------
Review request for lens and Amareshwari Sriramadasu.
Bugs: LENS-169
https://issues.apache.org/jira/browse/LENS-169
Repository: lens
Description
-------
Currently dimension only queries give duplicate rows when joining with fact for specified date range. For such queries we should be adding distinct keyword in the projected column list.
Diffs
-----
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 2d063ef
lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 8afaf17
lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java e24e0bc
Diff: https://reviews.apache.org/r/29705/diff/
Testing
-------
Tested this by adding new test case. All unit tests are running fine.
Thanks,
Sushil Mohanty
Re: Review Request 29705: Lens-169 : Add distinct keyword with columns
projected for dimension only queries
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29705/#review67406
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111428>
add braces around if. Please take care of it in all ifs added in new code
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111429>
Fix formatting and add braces correctly
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111425>
Can you move hasAggregate method in GroupbyResolver to HQLParser and make it static and call that directly, instead of adding it here again?
lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111430>
Can you also add test for setting the ENABLE_ATTRFIELDS_ADD_DISTINCT to false and check no distinct gets added for this?
- Amareshwari Sriramadasu
On Jan. 9, 2015, 10:02 a.m., Sushil Mohanty wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29705/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 10:02 a.m.)
>
>
> Review request for lens and Amareshwari Sriramadasu.
>
>
> Bugs: LENS-169
> https://issues.apache.org/jira/browse/LENS-169
>
>
> Repository: lens
>
>
> Description
> -------
>
> Currently dimension only queries give duplicate rows when joining with fact for specified date range. For such queries we should be adding distinct keyword in the projected column list.
>
>
> Diffs
> -----
>
> lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 2d063ef
> lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 8afaf17
> lens-cube/src/main/resources/olap-query-conf.xml 42bd210
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java e24e0bc
> src/site/apt/user/olap-query-conf.apt 2b22e8a
>
> Diff: https://reviews.apache.org/r/29705/diff/
>
>
> Testing
> -------
>
> Tested this by adding new test case. All unit tests are running fine.
>
>
> Thanks,
>
> Sushil Mohanty
>
>
Re: Review Request 29705: Lens-169 : Add distinct keyword with columns
projected for dimension only queries
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29705/#review67425
-----------------------------------------------------------
Ship it!
Ship It!
- Amareshwari Sriramadasu
On Jan. 9, 2015, 12:32 p.m., Sushil Mohanty wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29705/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 12:32 p.m.)
>
>
> Review request for lens and Amareshwari Sriramadasu.
>
>
> Bugs: LENS-169
> https://issues.apache.org/jira/browse/LENS-169
>
>
> Repository: lens
>
>
> Description
> -------
>
> Currently dimension only queries give duplicate rows when joining with fact for specified date range. For such queries we should be adding distinct keyword in the projected column list.
>
>
> Diffs
> -----
>
> lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 2d063ef
> lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 8afaf17
> lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java ee61aaa
> lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 3c551c2
> lens-cube/src/main/resources/olap-query-conf.xml 42bd210
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java e24e0bc
> src/site/apt/user/olap-query-conf.apt 2b22e8a
>
> Diff: https://reviews.apache.org/r/29705/diff/
>
>
> Testing
> -------
>
> Tested this by adding new test case. All unit tests are running fine.
>
>
> Thanks,
>
> Sushil Mohanty
>
>
Re: Review Request 29705: Lens-169 : Add distinct keyword with columns
projected for dimension only queries
Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29705/
-----------------------------------------------------------
(Updated Jan. 9, 2015, 12:32 p.m.)
Review request for lens and Amareshwari Sriramadasu.
Changes
-------
Addressed review comments.
Bugs: LENS-169
https://issues.apache.org/jira/browse/LENS-169
Repository: lens
Description
-------
Currently dimension only queries give duplicate rows when joining with fact for specified date range. For such queries we should be adding distinct keyword in the projected column list.
Diffs (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 2d063ef
lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 8afaf17
lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java ee61aaa
lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 3c551c2
lens-cube/src/main/resources/olap-query-conf.xml 42bd210
lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java e24e0bc
src/site/apt/user/olap-query-conf.apt 2b22e8a
Diff: https://reviews.apache.org/r/29705/diff/
Testing
-------
Tested this by adding new test case. All unit tests are running fine.
Thanks,
Sushil Mohanty
Re: Review Request 29705: Lens-169 : Add distinct keyword with columns
projected for dimension only queries
Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29705/
-----------------------------------------------------------
(Updated Jan. 9, 2015, 10:02 a.m.)
Review request for lens and Amareshwari Sriramadasu.
Changes
-------
Dimension with aggregate functions were not considered earlier. Added a method to check them as well. Other review comments addressed. All test cases are passing.
Bugs: LENS-169
https://issues.apache.org/jira/browse/LENS-169
Repository: lens
Description
-------
Currently dimension only queries give duplicate rows when joining with fact for specified date range. For such queries we should be adding distinct keyword in the projected column list.
Diffs (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 2d063ef
lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 8afaf17
lens-cube/src/main/resources/olap-query-conf.xml 42bd210
lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java e24e0bc
src/site/apt/user/olap-query-conf.apt 2b22e8a
Diff: https://reviews.apache.org/r/29705/diff/
Testing
-------
Tested this by adding new test case. All unit tests are running fine.
Thanks,
Sushil Mohanty
Re: Review Request 29705: Lens-169 : Add distinct keyword with columns
projected for dimension only queries
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29705/#review67390
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111409>
can you add braces around if clauses?
lens-cube/src/main/resources/olap-query-conf.xml
<https://reviews.apache.org/r/29705/#comment111407>
Can we change the name to lens.cube.query.enable.attrfields.add.distinct? Makes more readable.
lens-cube/src/main/resources/olap-query-conf.xml
<https://reviews.apache.org/r/29705/#comment111408>
Change description to 'When the query has only attribute fields projected from cube and the flag is set to true, distinct clause will be added for the projection sothat no duplicate values will be projected. If flag is set to false, projection wont be changed, result might include duplicate values.'
lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111406>
Whats the rewritten query for 'select count(cityid) from testcube'? We shouldn't be adding distinct for that. Can you add test for the same?
- Amareshwari Sriramadasu
On Jan. 9, 2015, 7:24 a.m., Sushil Mohanty wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29705/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 7:24 a.m.)
>
>
> Review request for lens and Amareshwari Sriramadasu.
>
>
> Bugs: LENS-169
> https://issues.apache.org/jira/browse/LENS-169
>
>
> Repository: lens
>
>
> Description
> -------
>
> Currently dimension only queries give duplicate rows when joining with fact for specified date range. For such queries we should be adding distinct keyword in the projected column list.
>
>
> Diffs
> -----
>
> lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 2d063ef
> lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 8afaf17
> lens-cube/src/main/resources/olap-query-conf.xml 42bd210
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java e24e0bc
> src/site/apt/user/olap-query-conf.apt 2b22e8a
>
> Diff: https://reviews.apache.org/r/29705/diff/
>
>
> Testing
> -------
>
> Tested this by adding new test case. All unit tests are running fine.
>
>
> Thanks,
>
> Sushil Mohanty
>
>
Re: Review Request 29705: Lens-169 : Add distinct keyword with columns
projected for dimension only queries
Posted by Sushil Mohanty <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29705/
-----------------------------------------------------------
(Updated Jan. 9, 2015, 7:24 a.m.)
Review request for lens and Amareshwari Sriramadasu.
Changes
-------
Review comments addressed.
Bugs: LENS-169
https://issues.apache.org/jira/browse/LENS-169
Repository: lens
Description
-------
Currently dimension only queries give duplicate rows when joining with fact for specified date range. For such queries we should be adding distinct keyword in the projected column list.
Diffs (updated)
-----
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 2d063ef
lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 8afaf17
lens-cube/src/main/resources/olap-query-conf.xml 42bd210
lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java e24e0bc
src/site/apt/user/olap-query-conf.apt 2b22e8a
Diff: https://reviews.apache.org/r/29705/diff/
Testing
-------
Tested this by adding new test case. All unit tests are running fine.
Thanks,
Sushil Mohanty
Re: Review Request 29705: Lens-169 : Add distinct keyword with columns
projected for dimension only queries
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29705/#review67186
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111051>
We should not create a new configuration here. You should be property value from CubeQueryContext.getConf()
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111052>
isDistinctClauseUsed(cubeql.getSelectAST()) == false can be changed to !isDistinctClauseUsed(cubeql.getSelectAST())
lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111054>
We might have to check for node.getToken() != null here
lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java
<https://reviews.apache.org/r/29705/#comment111059>
Can you add conf olap-query-conf.xml with description?
lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java
<https://reviews.apache.org/r/29705/#comment111058>
Can you seperate this test into different method?
Can you add queries with more than one column, and also queries passing distinct in themselves and with function distincts in that method?
- Amareshwari Sriramadasu
On Jan. 8, 2015, 10:35 a.m., Sushil Mohanty wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29705/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 10:35 a.m.)
>
>
> Review request for lens and Amareshwari Sriramadasu.
>
>
> Bugs: LENS-169
> https://issues.apache.org/jira/browse/LENS-169
>
>
> Repository: lens
>
>
> Description
> -------
>
> Currently dimension only queries give duplicate rows when joining with fact for specified date range. For such queries we should be adding distinct keyword in the projected column list.
>
>
> Diffs
> -----
>
> lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 2d063ef
> lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 8afaf17
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestAggregateResolver.java e24e0bc
>
> Diff: https://reviews.apache.org/r/29705/diff/
>
>
> Testing
> -------
>
> Tested this by adding new test case. All unit tests are running fine.
>
>
> Thanks,
>
> Sushil Mohanty
>
>