> On Dec. 26, 2014, 3:46 p.m., Amareshwari Sriramadasu wrote: > > lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java, > > line 219 > > <https://reviews.apache.org/r/29422/diff/3/?file=801333#file801333line219> > > > > Is there any node which is not SelectASTNode node? > > then is the if check required? Then the method > > isSelectASTNode in HQLParser required?
A method normally has following stages: 1) Precondition 2) Logic (Assumes precondtion is met) 3) Postconditions (cleanup) 4) Return The Logic of filterNonMsrNonAggSelectASTChildren assumes that a precondition is met by node input into the function. This check is to ensure the precondition. Currently the method is trying to give expected results and is returning an empty list when input node is not a select AST Node. If we remove this check, than we'll have to put following comment in javadoc of this method: "If input node is not a select ASTNode then return value is unexpected." > On Dec. 26, 2014, 3:46 p.m., Amareshwari Sriramadasu wrote: > > lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java, > > line 197 > > <https://reviews.apache.org/r/29422/diff/3/?file=801333#file801333line197> > > > > There wont be any ASTNode passed here which is not select expr node, > > right? Is this if check required? > > > > Also the methods isSelectExprASTNode in HQLParser If we are sure that a SELECT ASTNode can only have SELET_EXPR AST Nodes as children, then we can remove this check here. Is this declared in the contract of Hive Parser that SELECT AST Node will only have SELECT_EXPR as children ? - Himanshu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29422/#review66156 ----------------------------------------------------------- On Dec. 26, 2014, 12:58 p.m., Himanshu Gahlaut wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29422/ > ----------------------------------------------------------- > > (Updated Dec. 26, 2014, 12:58 p.m.) > > > Review request for lens. > > > Repository: lens > > > Description > ------- > > (1) getExpressionWithoutAlias in GroupbyResolver.class is unable to handle > aliases with spaces in them. Fixed the same. > (2) Also improved log4j.properties of lens-cube to send logs to both console > appender and test log files. > (3) Used Slf4j annotation in Test classes for getting a reference to Slf4j > logger. Added required dependencies in lens-cube/pom.xml to facilitate the > same. > > > Diffs > ----- > > lens-cube/pom.xml 1bcb4dc > lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java > c2fef7e > lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 8e41830 > lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java > f515df1 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java > ad4abcf > lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryRewrite.java > fd939e9 > lens-cube/src/test/resources/log4j.properties 9729de0 > > Diff: https://reviews.apache.org/r/29422/diff/ > > > Testing > ------- > > Added new unit test cases for the changes. > All existing Unit Test Cases passed. > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [14.068s] > [INFO] Lens .............................................. SUCCESS [5.717s] > [INFO] Lens API .......................................... SUCCESS [9.369s] > [INFO] Lens API for server and extensions ................ SUCCESS [8.624s] > [INFO] Lens Cube ......................................... SUCCESS [7:10.440s] > [INFO] Lens DB storage ................................... SUCCESS [23.631s] > [INFO] Lens Query Library ................................ SUCCESS [9.386s] > [INFO] Lens Hive Driver .................................. SUCCESS [3:34.922s] > [INFO] Lens Driver for Cloudera Impala ................... SUCCESS [10.798s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [36.429s] > [INFO] Lens Server ....................................... SUCCESS > [14:29.334s] > [INFO] Lens client ....................................... SUCCESS [45.711s] > [INFO] Lens CLI .......................................... SUCCESS [3:10.102s] > [INFO] Lens Examples ..................................... SUCCESS [3.064s] > [INFO] Lens Distribution ................................. SUCCESS [11.315s] > [INFO] Lens Client Distribution .......................... SUCCESS [7.496s] > [INFO] Lens ML Lib ....................................... SUCCESS [2:32.231s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 34:03.600s > [INFO] Finished at: Fri Dec 26 18:21:01 IST 2014 > [INFO] Final Memory: 100M/332M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Himanshu Gahlaut > >
