You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Amareshwari Sriramadasu <am...@apache.org> on 2015/05/19 10:53:12 UTC

Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

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.

Pending:

Fillin TODOs marked
- FieldValidator to validate derived cube corresponding to expression along other fields qeuried
- TimerangeResolver to remove expressions which not valid in the range
- 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/TestBaseCubeQueries.java 2a8f082 
  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 new tests for picking different expressions of expression column
- Added new tests for picking materialized expression value soemtimes and expression sometimes


Thanks,

Amareshwari Sriramadasu


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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



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

    Will be adding unit test to verify all nested expressions are expanded and added


- Amareshwari Sriramadasu


On May 19, 2015, 8:53 a.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34399/
> -----------------------------------------------------------
> 
> (Updated May 19, 2015, 8:53 a.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.
> 
> Pending:
> 
> Fillin TODOs marked
> - FieldValidator to validate derived cube corresponding to expression along other fields qeuried
> - TimerangeResolver to remove expressions which not valid in the range
> - 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/TestBaseCubeQueries.java 2a8f082 
>   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 new tests for picking different expressions of expression column
> - Added new tests for picking materialized expression value soemtimes and expression sometimes
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

> On May 26, 2015, 8:13 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java, line 544
> > <https://reviews.apache.org/r/34399/diff/8/?file=969025#file969025line544>
> >
> >     first `updateEvaluable` then `isEvaluable` with exactly same arguments. Seems weird.

updateEvaluable is called once to update all evaluable expressions. isEvaluable is called multiple times as a boolean check. If we want to avoid the call updateEvaluable, we need to have a cache for each candidateTable which expression is evaluable and so on. so, not doing changes here.


> On May 26, 2015, 8:13 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java, line 807
> > <https://reviews.apache.org/r/34399/diff/8/?file=969027#file969027line807>
> >
> >     or size zero

Not required


> On May 26, 2015, 8:13 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java, line 930
> > <https://reviews.apache.org/r/34399/diff/8/?file=969027#file969027line930>
> >
> >     `isHasMeasures()` -> `hasMeasures()`

hasMeasures() is something i want, but lombak is not giving such a name.


> On May 26, 2015, 8:13 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/test/resources/log4j.properties, line 36
> > <https://reviews.apache.org/r/34399/diff/8/?file=969043#file969043line36>
> >
> >     Why is this changed?

INFO was better for debugging. Removed console logging for not seeing huge logs.


> On May 26, 2015, 8:13 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/TrackQueriedColumns.java, line 24
> > <https://reviews.apache.org/r/34399/diff/8/?file=969034#file969034line24>
> >
> >     public?

Not required


> On May 26, 2015, 8:13 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java, line 751
> > <https://reviews.apache.org/r/34399/diff/8/?file=969030#file969030line751>
> >
> >     Unit test for this?

TestExpressionContext$testNestedExpressions covers this.


- Amareshwari


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


On May 22, 2015, 7:54 a.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34399/
> -----------------------------------------------------------
> 
> (Updated May 22, 2015, 7:54 a.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
> - Add more testcases to cover cornercases
> - Allow expressions to have denorm columns and added tests
> 
> 
> 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 76b5729 
>   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 52bf9aa 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 8c009b2 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java 40561ad 
>   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 3964c1a 
>   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 e0f7bea 
>   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 17d2eed 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java 936faa1 
>   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 5c776cc 
>   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 2fd0a46 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java 71ed1a8 
>   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
> -------
> 
> - 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.
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules ............................. SUCCESS [3.214s]
> [INFO] Lens .............................................. SUCCESS [3.542s]
> [INFO] Lens API .......................................... SUCCESS [20.894s]
> [INFO] Lens API for server and extensions ................ SUCCESS [18.985s]
> [INFO] Lens Cube ......................................... SUCCESS [3:34.331s]
> [INFO] Lens DB storage ................................... SUCCESS [20.569s]
> [INFO] Lens Query Library ................................ SUCCESS [16.552s]
> [INFO] Lens Hive Driver .................................. SUCCESS [2:56.814s]
> [INFO] Lens Driver for JDBC .............................. SUCCESS [47.560s]
> [INFO] Lens Server ....................................... SUCCESS [6:54.878s]
> [INFO] Lens client ....................................... SUCCESS [44.291s]
> [INFO] Lens CLI .......................................... SUCCESS [3:36.281s]
> [INFO] Lens Examples ..................................... SUCCESS [11.696s]
> [INFO] Lens Distribution ................................. SUCCESS [8.245s]
> [INFO] Lens ML Lib ....................................... SUCCESS [1:19.084s]
> [INFO] Lens ML Ext Distribution .......................... SUCCESS [1.708s]
> [INFO] Lens Regression ................................... SUCCESS [8.307s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 21:47.838s
> [INFO] Finished at: Fri May 22 07:10:12 UTC 2015
> [INFO] Final Memory: 165M/1275M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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



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

    @Getter



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

    We can refactor it to three functions returning their own `toRemove`. Then in the original function: `if(a() or b() or c()) { iter.remove }`



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

    first `updateEvaluable` then `isEvaluable` with exactly same arguments. Seems weird.



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

    I don't think a blank line is needed.



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

    `exprDimensions` ?



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

    or size zero



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

    `isHasMeasures()` -> `hasMeasures()`



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

    Both if and else (over split.length) are doing the same thing with different arguments.



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

    Let's also call this `isCubeMeasure` since it's returning the result of `isCubeMeasure` and doing essencially the same thing. Having different names makes it confusing to understand and use by callers.



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

    Can't we merge the two for loops?



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

    passing both `cubeql` and `cubeql.something()`. Let's pass only `cubeql`.



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

    use getter. `esc.getTblAliasToColumns()`



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

    commented?



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

    Unit test for this?



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

    public?



lens-cube/src/test/resources/log4j.properties
<https://reviews.apache.org/r/34399/#comment136655>

    Why is this changed?


- Rajat Khandelwal


On May 22, 2015, 1:24 p.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34399/
> -----------------------------------------------------------
> 
> (Updated May 22, 2015, 1:24 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
> - Add more testcases to cover cornercases
> - Allow expressions to have denorm columns and added tests
> 
> 
> 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 76b5729 
>   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 52bf9aa 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 8c009b2 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java 40561ad 
>   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 3964c1a 
>   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 e0f7bea 
>   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 17d2eed 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java 936faa1 
>   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 5c776cc 
>   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 2fd0a46 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java 71ed1a8 
>   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
> -------
> 
> - 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.
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules ............................. SUCCESS [3.214s]
> [INFO] Lens .............................................. SUCCESS [3.542s]
> [INFO] Lens API .......................................... SUCCESS [20.894s]
> [INFO] Lens API for server and extensions ................ SUCCESS [18.985s]
> [INFO] Lens Cube ......................................... SUCCESS [3:34.331s]
> [INFO] Lens DB storage ................................... SUCCESS [20.569s]
> [INFO] Lens Query Library ................................ SUCCESS [16.552s]
> [INFO] Lens Hive Driver .................................. SUCCESS [2:56.814s]
> [INFO] Lens Driver for JDBC .............................. SUCCESS [47.560s]
> [INFO] Lens Server ....................................... SUCCESS [6:54.878s]
> [INFO] Lens client ....................................... SUCCESS [44.291s]
> [INFO] Lens CLI .......................................... SUCCESS [3:36.281s]
> [INFO] Lens Examples ..................................... SUCCESS [11.696s]
> [INFO] Lens Distribution ................................. SUCCESS [8.245s]
> [INFO] Lens ML Lib ....................................... SUCCESS [1:19.084s]
> [INFO] Lens ML Ext Distribution .......................... SUCCESS [1.708s]
> [INFO] Lens Regression ................................... SUCCESS [8.307s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 21:47.838s
> [INFO] Finished at: Fri May 22 07:10:12 UTC 2015
> [INFO] Final Memory: 165M/1275M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

Ship it!


Ship It!

- Rajat Khandelwal


On May 22, 2015, 1:24 p.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34399/
> -----------------------------------------------------------
> 
> (Updated May 22, 2015, 1:24 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
> - Add more testcases to cover cornercases
> - Allow expressions to have denorm columns and added tests
> 
> 
> 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 76b5729 
>   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 52bf9aa 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 8c009b2 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java 40561ad 
>   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 3964c1a 
>   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 e0f7bea 
>   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 17d2eed 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java 936faa1 
>   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 5c776cc 
>   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 2fd0a46 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java 71ed1a8 
>   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
> -------
> 
> - 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.
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules ............................. SUCCESS [3.214s]
> [INFO] Lens .............................................. SUCCESS [3.542s]
> [INFO] Lens API .......................................... SUCCESS [20.894s]
> [INFO] Lens API for server and extensions ................ SUCCESS [18.985s]
> [INFO] Lens Cube ......................................... SUCCESS [3:34.331s]
> [INFO] Lens DB storage ................................... SUCCESS [20.569s]
> [INFO] Lens Query Library ................................ SUCCESS [16.552s]
> [INFO] Lens Hive Driver .................................. SUCCESS [2:56.814s]
> [INFO] Lens Driver for JDBC .............................. SUCCESS [47.560s]
> [INFO] Lens Server ....................................... SUCCESS [6:54.878s]
> [INFO] Lens client ....................................... SUCCESS [44.291s]
> [INFO] Lens CLI .......................................... SUCCESS [3:36.281s]
> [INFO] Lens Examples ..................................... SUCCESS [11.696s]
> [INFO] Lens Distribution ................................. SUCCESS [8.245s]
> [INFO] Lens ML Lib ....................................... SUCCESS [1:19.084s]
> [INFO] Lens ML Ext Distribution .......................... SUCCESS [1.708s]
> [INFO] Lens Regression ................................... SUCCESS [8.307s]
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 21:47.838s
> [INFO] Finished at: Fri May 22 07:10:12 UTC 2015
> [INFO] Final Memory: 165M/1275M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Amareshwari Sriramadasu
> 
>


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

(Updated May 22, 2015, 7:54 a.m.)


Review request for lens, Jaideep dhok and Rajat Khandelwal.


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


Repository: lens


Description (updated)
-------

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
- Add more testcases to cover cornercases
- Allow expressions to have denorm columns and added tests


Diffs (updated)
-----

  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 76b5729 
  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 52bf9aa 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java 8c009b2 
  lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java 40561ad 
  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 3964c1a 
  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 e0f7bea 
  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 17d2eed 
  lens-cube/src/main/java/org/apache/lens/cube/parse/TimerangeResolver.java 936faa1 
  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 5c776cc 
  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 2fd0a46 
  lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java 71ed1a8 
  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 (updated)
-------

- 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.

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Lens Checkstyle Rules ............................. SUCCESS [3.214s]
[INFO] Lens .............................................. SUCCESS [3.542s]
[INFO] Lens API .......................................... SUCCESS [20.894s]
[INFO] Lens API for server and extensions ................ SUCCESS [18.985s]
[INFO] Lens Cube ......................................... SUCCESS [3:34.331s]
[INFO] Lens DB storage ................................... SUCCESS [20.569s]
[INFO] Lens Query Library ................................ SUCCESS [16.552s]
[INFO] Lens Hive Driver .................................. SUCCESS [2:56.814s]
[INFO] Lens Driver for JDBC .............................. SUCCESS [47.560s]
[INFO] Lens Server ....................................... SUCCESS [6:54.878s]
[INFO] Lens client ....................................... SUCCESS [44.291s]
[INFO] Lens CLI .......................................... SUCCESS [3:36.281s]
[INFO] Lens Examples ..................................... SUCCESS [11.696s]
[INFO] Lens Distribution ................................. SUCCESS [8.245s]
[INFO] Lens ML Lib ....................................... SUCCESS [1:19.084s]
[INFO] Lens ML Ext Distribution .......................... SUCCESS [1.708s]
[INFO] Lens Regression ................................... SUCCESS [8.307s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 21:47.838s
[INFO] Finished at: Fri May 22 07:10:12 UTC 2015
[INFO] Final Memory: 165M/1275M
[INFO] ------------------------------------------------------------------------


Thanks,

Amareshwari Sriramadasu


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

(Updated May 21, 2015, 12:15 p.m.)


Review request for lens, Jaideep dhok and Rajat Khandelwal.


Changes
-------

Tests fixed


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


Repository: lens


Description (updated)
-------

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
- Add more testcases to cover cornercases


Diffs (updated)
-----

  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 (updated)
-------

- 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.

All lens-cube tests pass


Thanks,

Amareshwari Sriramadasu


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

(Updated May 20, 2015, 11:16 a.m.)


Review request for lens, Jaideep dhok and Rajat Khandelwal.


Changes
-------

Review comments fixed


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 (updated)
-----

  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


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

> On May 20, 2015, 9:38 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java, line 325
> > <https://reviews.apache.org/r/34399/diff/1/?file=963556#file963556line325>
> >
> >     both new static methods have cubeql as argument. I wonder if keeping them inside CubeQueryContext will make more sense?

Moved isMeasure method, as it is generic enough, but hasMeasuresNotInDefaultAggregates need not be static, reverting the change to do with making static


- Amareshwari


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


On May 20, 2015, 8:44 a.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34399/
> -----------------------------------------------------------
> 
> (Updated May 20, 2015, 8:44 a.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
> 
>


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

> On May 20, 2015, 9:38 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/Dimension.java, line 174
> > <https://reviews.apache.org/r/34399/diff/1/?file=963554#file963554line174>
> >
> >     No need to cast, I think.

Yes. Removing.


> On May 20, 2015, 9:38 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java, line 68
> > <https://reviews.apache.org/r/34399/diff/1/?file=963557#file963557line68>
> >
> >     Is this ever null?

I also saw that. But earlier code was checking for null. Removing now.


> On May 20, 2015, 9:38 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java, line 138
> > <https://reviews.apache.org/r/34399/diff/1/?file=963557#file963557line138>
> >
> >     Is there a use case where the last two arguments are differenet?

Yes, I'm calling it from ExpressionResolver, passing ExprSpecContext to replaces aliases in expressions.


> On May 20, 2015, 9:38 a.m., Rajat Khandelwal wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java, line 48
> > <https://reviews.apache.org/r/34399/diff/1/?file=963562#file963562line48>
> >
> >     Can you explain why the interface is needed.

Added the interface, because wanted to keep track of columns and iterate over them for each as well Expression. Especially for reusing code of finding all columns in AST and replacing AST columns with aliases in ColumnResolver and AliasReplacer


- Amareshwari


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


On May 20, 2015, 8:44 a.m., Amareshwari Sriramadasu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34399/
> -----------------------------------------------------------
> 
> (Updated May 20, 2015, 8:44 a.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
> 
>


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

Posted by Rajat Khandelwal <ra...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

(Updated May 20, 2015, 8:44 a.m.)


Review request for lens, Jaideep dhok and Rajat Khandelwal.


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


Repository: lens


Description (updated)
-------

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 (updated)
-----

  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 (updated)
-------

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


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

(Updated May 19, 2015, 11:12 a.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.

Pending:

Fillin TODOs marked
- FieldValidator to validate derived cube corresponding to expression along other fields qeuried
- TimerangeResolver to remove expressions which not valid in the range
- Add more testcases to cover cornercases


Diffs (updated)
-----

  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/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 new tests for picking different expressions of expression column
- Added new tests for picking materialized expression value soemtimes and expression sometimes


Thanks,

Amareshwari Sriramadasu


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

(Updated May 19, 2015, 10:42 a.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.

Pending:

Fillin TODOs marked
- FieldValidator to validate derived cube corresponding to expression along other fields qeuried
- TimerangeResolver to remove expressions which not valid in the range
- Add more testcases to cover cornercases


Diffs (updated)
-----

  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/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 new tests for picking different expressions of expression column
- Added new tests for picking materialized expression value soemtimes and expression sometimes


Thanks,

Amareshwari Sriramadasu


Re: Review Request 34399: LENS-174 : Add cube query rewriter changes for picking eligible expression from multiple expressions available.

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

(Updated May 19, 2015, 9:19 a.m.)


Review request for lens, Jaideep dhok and Rajat Khandelwal.


Changes
-------

Fix comments


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.

Pending:

Fillin TODOs marked
- FieldValidator to validate derived cube corresponding to expression along other fields qeuried
- TimerangeResolver to remove expressions which not valid in the range
- Add more testcases to cover cornercases


Diffs (updated)
-----

  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/TestBaseCubeQueries.java 2a8f082 
  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 new tests for picking different expressions of expression column
- Added new tests for picking materialized expression value soemtimes and expression sometimes


Thanks,

Amareshwari Sriramadasu