Re: Review Request 38402: LENS-752 : Support flattening of columns selected through bridge-tables(many-to-many relationships)

2015-09-24 Thread Rajat Khandelwal 

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

Ship it!



lens-cube/src/main/java/org/apache/lens/cube/metadata/TableReference.java (line 
42)


We are serializing as `t1.f1->t2.f2[n]`. So `parseBoolean` might not work.


- Rajat Khandelwal


On Sept. 16, 2015, 2:33 p.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38402/
> ---
> 
> (Updated Sept. 16, 2015, 2:33 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-752
> https://issues.apache.org/jira/browse/LENS-752
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Changes include :
> - Ability to specify if the destination link in a join chain has 
> many-many-relationship
> - Ability to configure at query to say flatten the fields coming from 
> bridge-table relations
> - Enhance cube query rewriter (mainly the join clause) to add join for bridge 
> table to flatten them
> - Added tests for all new functionality
> - Updated documentation with feature addition
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/resources/cube-0.1.xsd 58f68f5 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensCubeCommands.java 
> 39441c9 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/JoinChain.java 
> e394e20 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 
> bf27b99 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/SchemaGraph.java 
> 1a37e80 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/TableReference.java 
> 31fd97b 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 
> aab2488 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java 
> c7f1e2a 
>   lens-cube/src/main/resources/olap-query-conf.xml c9b8d0f 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 
> 67f7ab9 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java 
> cb63fad 
>   lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 
> ed472f6 
>   
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java
>  f9be6e6 
>   src/site/apt/user/olap-cube.apt ac91955 
>   src/site/apt/user/olap-query-conf.apt 8204c34 
> 
> Diff: https://reviews.apache.org/r/38402/diff/
> 
> 
> Testing
> ---
> 
> [INFO] 
> 
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules . SUCCESS [2.015s]
> [INFO] Lens .. SUCCESS [3.049s]
> [INFO] Lens API .. SUCCESS [25.588s]
> [INFO] Lens API for server and extensions  SUCCESS [20.759s]
> [INFO] Lens Cube . SUCCESS [5:09.699s]
> [INFO] Lens DB storage ... SUCCESS [20.745s]
> [INFO] Lens Query Library  SUCCESS [16.889s]
> [INFO] Lens Hive Driver .. SUCCESS [2:48.677s]
> [INFO] Lens Driver for JDBC .. SUCCESS [37.134s]
> [INFO] Lens Elastic Search Driver  SUCCESS [16.155s]
> [INFO] Lens Server ... SUCCESS [5:40.844s]
> [INFO] Lens client ... SUCCESS [35.393s]
> [INFO] Lens CLI .. SUCCESS [2:36.188s]
> [INFO] Lens Examples . SUCCESS [8.233s]
> [INFO] Lens Distribution . SUCCESS [8.400s]
> [INFO] Lens ML Lib ... SUCCESS [1:18.649s]
> [INFO] Lens ML Ext Distribution .. SUCCESS [2.233s]
> [INFO] Lens Regression ... SUCCESS [10.572s]
> [INFO] 
> 
> [INFO] BUILD SUCCESS
> [INFO] 
> 
> [INFO] Total time: 21:02.117s
> [INFO] Finished at: Wed Sep 16 09:00:28 UTC 2015
> [INFO] Final Memory: 209M/2163M
> [INFO] 
> 
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>

Re: Review Request 38402: LENS-752 : Support flattening of columns selected through bridge-tables(many-to-many relationships)

2015-09-15 Thread Amareshwari Sriramadasu 

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



lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java (line 582)


We should make this function configurable and can pass different function 
names from drivers.


- Amareshwari Sriramadasu


On Sept. 15, 2015, 12:48 p.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38402/
> ---
> 
> (Updated Sept. 15, 2015, 12:48 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Changes include :
> - Ability to specify if the destination link in a join chain has 
> many-many-relationship
> - Ability to configure at query to say flatten the fields coming from 
> bridge-table relations
> - Enhance cube query rewriter (mainly the join clause) to add join for bridge 
> table to flatten them
> 
> Pending :
> - Add a test with multi fact query
> - Update documentation with feature addition
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/resources/cube-0.1.xsd 58f68f5 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/JoinChain.java 
> e394e20 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 
> bf27b99 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/SchemaGraph.java 
> 1a37e80 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/TableReference.java 
> 31fd97b 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 
> aab2488 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java 
> c7f1e2a 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 
> 7f56292 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java 
> cb63fad 
>   lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 
> ed472f6 
>   
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java
>  f9be6e6 
> 
> Diff: https://reviews.apache.org/r/38402/diff/
> 
> 
> Testing
> ---
> 
> Added testcases for bridge tables and queries on them.
> 
> All tests in lens-cube module pass. Will update full test results once done.
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>

Re: Review Request 38402: LENS-752 : Support flattening of columns selected through bridge-tables(many-to-many relationships)

2015-09-15 Thread Amareshwari Sriramadasu 


> On Sept. 15, 2015, 2 p.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java, line 
> > 579
> > 
> >
> > blankness of `bridgeFromClause`

from clause would never be empty. This is for appending userfilter if it is not 
null


> On Sept. 15, 2015, 2 p.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java, lines 
> > 586-587
> > 
> >
> > only null check needed.

why? we should check if the column are queried from that table as well.


> On Sept. 15, 2015, 2 p.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java, line 
> > 585
> > 
> >
> > Among the approaches: 
> > 
> > 1. Relying on a flag to append "select" or "join"
> > 2. Making a list and later doing "select" + StringUtils.join("join", 
> > list)
> > 
> > the second seems more intuitive. And would be easier to debug later on.

Did not understand the comment fully.

It is not simply prepending select or join to alist. If join needs to be 
converted to select from a bridge table - it becomes a full fledged query with 
select, from, groupby, filters and optionally more joins.


- Amareshwari


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


On Sept. 15, 2015, 12:48 p.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38402/
> ---
> 
> (Updated Sept. 15, 2015, 12:48 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Changes include :
> - Ability to specify if the destination link in a join chain has 
> many-many-relationship
> - Ability to configure at query to say flatten the fields coming from 
> bridge-table relations
> - Enhance cube query rewriter (mainly the join clause) to add join for bridge 
> table to flatten them
> 
> Pending :
> - Add a test with multi fact query
> - Update documentation with feature addition
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/resources/cube-0.1.xsd 58f68f5 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/JoinChain.java 
> e394e20 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 
> bf27b99 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/SchemaGraph.java 
> 1a37e80 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/TableReference.java 
> 31fd97b 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 
> aab2488 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java 
> c7f1e2a 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 
> 7f56292 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java 
> cb63fad 
>   lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 
> ed472f6 
>   
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java
>  f9be6e6 
> 
> Diff: https://reviews.apache.org/r/38402/diff/
> 
> 
> Testing
> ---
> 
> Added testcases for bridge tables and queries on them.
> 
> All tests in lens-cube module pass. Will update full test results once done.
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>

Re: Review Request 38402: LENS-752 : Support flattening of columns selected through bridge-tables(many-to-many relationships)

2015-09-15 Thread Rajat Khandelwal 

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



lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java (line 556)


Let's check blankness of `bridgeFilterClause` instead of checking blankness 
of the only thing appended to `bridgeFilterClause`.



lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java (line 572)


blankness of `bridgeFromClause`



lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java (line 578)


Among the approaches: 

1. Relying on a flag to append "select" or "join"
2. Making a list and later doing "select" + StringUtils.join("join", list)

the second seems more intuitive. And would be easier to debug later on.



lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java (lines 579 
- 580)


only null check needed.


- Rajat Khandelwal


On Sept. 15, 2015, 6:18 p.m., Amareshwari Sriramadasu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38402/
> ---
> 
> (Updated Sept. 15, 2015, 6:18 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Repository: lens
> 
> 
> Description
> ---
> 
> Changes include :
> - Ability to specify if the destination link in a join chain has 
> many-many-relationship
> - Ability to configure at query to say flatten the fields coming from 
> bridge-table relations
> - Enhance cube query rewriter (mainly the join clause) to add join for bridge 
> table to flatten them
> 
> Pending :
> - Add a test with multi fact query
> - Update documentation with feature addition
> 
> 
> Diffs
> -
> 
>   lens-api/src/main/resources/cube-0.1.xsd 58f68f5 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/JoinChain.java 
> e394e20 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 
> bf27b99 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/SchemaGraph.java 
> 1a37e80 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/TableReference.java 
> 31fd97b 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 
> aab2488 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java 
> c7f1e2a 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 
> 7f56292 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java 
> cb63fad 
>   lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 
> ed472f6 
>   
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java
>  f9be6e6 
> 
> Diff: https://reviews.apache.org/r/38402/diff/
> 
> 
> Testing
> ---
> 
> Added testcases for bridge tables and queries on them.
> 
> All tests in lens-cube module pass. Will update full test results once done.
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>