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/06/03 11:01:32 UTC
Re: Review Request 34399: LENS-174 : Add cube query rewriter changes
for picking eligible expression from multiple expressions available.
> 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
>
>