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