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



lens-cube/src/main/java/org/apache/lens/cube/metadata/Dimension.java
<https://reviews.apache.org/r/34399/#comment135793>

    No need to cast, I think.



lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java
<https://reviews.apache.org/r/34399/#comment135794>

    both new static methods have cubeql as argument. I wonder if keeping them 
inside CubeQueryContext will make more sense?



lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java
<https://reviews.apache.org/r/34399/#comment135795>

    Is this ever null?



lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java
<https://reviews.apache.org/r/34399/#comment135796>

    Is there a use case where the last two arguments are differenet?



lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java
<https://reviews.apache.org/r/34399/#comment135797>

    typo: evaluable



lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java
<https://reviews.apache.org/r/34399/#comment135798>

    Can you explain why the interface is needed.


- Rajat Khandelwal


On May 20, 2015, 2:14 p.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34399/
> -----------------------------------------------------------
> 
> (Updated May 20, 2015, 2:14 p.m.)
> 
> 
> Review request for lens, Jaideep dhok and Rajat Khandelwal.
> 
> 
> Bugs: LENS-174
>     https://issues.apache.org/jira/browse/LENS-174
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Changes are the following :
> 
> - Fix Dimension.getColumnByName() to return expression column as well. This 
> was existing bug, got fixed now
> - Adds ExpressionContext (new inner class) for each expression queried in 
> ExpressionResolver. And 
> - ExpressionContext is updated with ExpressionSpecContext (new inner class) 
> for each expression in expressioncolumn. It adds newer ExpressionSpecContext 
> by expanding all nested expressions.
> - Some methods in AliasReplacer and ColumnResolver are made static sothat 
> they can called from ExpressionResolver as well.
> - AggregateResolver wraps appropriate aggregate around fields in expressions, 
> along with ASTs
> - CandidateTableResolver picks all facts if expression column is directly 
> available. if not, it checks if the expression is evaluatable by the 
> candidate table.
> - Finally CubeQueryContext.toHQL() picks appropriate expressions for 
> candidates picked and replaces them in AST. A picked expression can add more 
> tables to joined.
> - FieldValidator to validate derived cube corresponding to expression along 
> other fields qeuried
> - TimerangeResolver to remove expressions which not valid in the range
> 
> Pending:
> - Add more testcases to cover cornercases
> 
> 
> Diffs
> -----
> 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/Dimension.java 
> 1a4b581 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/ExprColumn.java 
> a7f711f 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 
> a25fae6 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java 
> 9d367c3 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateFact.java 
> 84e5341 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java
>  a1fea16 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java
>  6b6a09b 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ColumnResolver.java 
> 1aa33db 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 
> 38b6429 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java 
> b7a92e7 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/DenormalizationResolver.java
>  7857868 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 
> 5355049 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/FieldValidator.java 
> fd8568f 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java 
> 3e3534c 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java 
> e5e7c56 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/TrackQueriedColumns.java 
> PRE-CREATION 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 
> 5737057 
>   
> lens-cube/src/test/java/org/apache/lens/cube/parse/FieldsCannotBeQueriedTogetherTest.java
>  1e0dbf8 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 
> 2a8f082 
>   
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionContext.java 
> PRE-CREATION 
>   
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java
>  a78bcbd 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java 
> 2c8dab3 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java 
> 4304f94 
>   lens-cube/src/test/resources/log4j.properties f514648 
> 
> Diff: https://reviews.apache.org/r/34399/diff/
> 
> 
> Testing
> -------
> 
> Almost lens-cube tests pass, a couple of them are failing. Working on fixing 
> them.
> 
> - Added tests for picking different expressions of expression column
> - Added tests for picking materialized expression value soemtimes and 
> expression sometimes
> - Added tetss testing all nested expressions are added
> - Added tests FieldNotQueryableTogether to test for expression fields.
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>

Reply via email to