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