> On Dec. 2, 2014, 9:58 a.m., Rajat Khandelwal wrote: > > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java, > > line 210 > > <https://reviews.apache.org/r/28596/diff/1/?file=779977#file779977line210> > > > > `{` should be on the same line.
It is on the same line. review board is wrapping and showing > On Dec. 2, 2014, 9:58 a.m., Rajat Khandelwal wrote: > > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java, > > line 251 > > <https://reviews.apache.org/r/28596/diff/1/?file=779976#file779976line251> > > > > Won't we have discarded all facts if no fact contains a particular join > > chain's src column? Yes. This is for pruning the paths in join chain, which contain columns that are not there in any fact. For example : Fact1 : col1, col2 Fact2 : col3 Fact3 : col5 JoinChain: path1 : col1 -> dim path2 : col2 -> dim path3 : col3 -> dim path4 : col4 -> dim In the first pruning Fact3 gets pruned. In the second pruning - path4 (containgin col4) gets pruned Will add the above example in code > On Dec. 2, 2014, 9:58 a.m., Rajat Khandelwal wrote: > > lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java, line > > 287 > > <https://reviews.apache.org/r/28596/diff/1/?file=779975#file779975line287> > > > > I hope this is properly tested. Changes look okay but better to be sure. yes. Test cases are added > On Dec. 2, 2014, 9:58 a.m., Rajat Khandelwal wrote: > > lens-cube/src/main/java/org/apache/lens/cube/metadata/JoinChain.java, line > > 150 > > <https://reviews.apache.org/r/28596/diff/1/?file=779973#file779973line150> > > > > rename to getIntermediateDimensions ? Sure > On Dec. 2, 2014, 9:58 a.m., Rajat Khandelwal wrote: > > lens-cube/src/main/java/org/apache/lens/cube/metadata/JoinChain.java, line > > 121 > > <https://reviews.apache.org/r/28596/diff/1/?file=779973#file779973line121> > > > > Should we make Path a logical entity. Path would extend from > > List<TableReference> and have additional methods like sourceColumn, > > destTable, destColumn etc. Would make this class a lot more readable. True. Will try to put the changes - Amareshwari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28596/#review63504 ----------------------------------------------------------- On Dec. 2, 2014, 7:25 a.m., Amareshwari Sriramadasu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28596/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2014, 7:25 a.m.) > > > Review request for lens and Jaideep dhok. > > > Bugs: LENS-41 > https://issues.apache.org/jira/browse/LENS-41 > > > Repository: lens > > > Description > ------- > > Includes changes required for query rewriter to understand chainName.column > and resolve the join corresponding to chain. > > Still pending: If query involves two different chains leading same > destination tables. Will raise a followup issue and put patch there. > > > Diffs > ----- > > lens-cube/src/main/java/org/apache/lens/cube/metadata/JoinChain.java > ebdf952 > > lens-cube/src/main/java/org/apache/lens/cube/metadata/ReferencedDimAtrribute.java > 51a9528 > lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java > d255566 > > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java > 8b9e604 > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java > 2409db8 > > lens-cube/src/main/java/org/apache/lens/cube/parse/DenormalizationResolver.java > 93bcabe > lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java > bc15890 > lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java > 485ff46 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java > 114bbc4 > > Diff: https://reviews.apache.org/r/28596/diff/ > > > Testing > ------- > > > Thanks, > > Amareshwari Sriramadasu > >
