> On Sept. 22, 2015, 8:07 a.m., Rajat Khandelwal wrote: > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, > > lines 306-308 > > <https://reviews.apache.org/r/38521/diff/4/?file=1079585#file1079585line306> > > > > can use `expectedJoinList.containsAll(actualQueryParts)` instead of the > > loop. > > > > And the check should also be done in the reverse order.
Checking the sizes of the lists already. Checking in the reverse does not required. Will do containsAll() > On Sept. 22, 2015, 8:07 a.m., Rajat Khandelwal wrote: > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java, > > line 90 > > <https://reviews.apache.org/r/38521/diff/4/?file=1079586#file1079586line90> > > > > is it needed? It is needed as both storages C1 and C2 can answer the query. We are checking for only tables on C2 storage in assert. > On Sept. 22, 2015, 8:07 a.m., Rajat Khandelwal wrote: > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java, > > line 292 > > <https://reviews.apache.org/r/38521/diff/4/?file=1079587#file1079587line292> > > > > only c2? what was the default? Earlier we were testing against two storages C1 and C2. Now default one is C1. > On Sept. 22, 2015, 8:07 a.m., Rajat Khandelwal wrote: > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java, > > line 202 > > <https://reviews.apache.org/r/38521/diff/4/?file=1079588#file1079588line202> > > > > Is some behaviour different in java 8 than java 7? > > Or is there some other code change that requires this portion to be > > commented? There are multiple sets(3 sets) can cover maximum number of partitions. Changed the assert statement accordingly. Earlier which is in java7, we always used to get same set but in java8 it is giving different sets. Iterator on set is not giving elements in different order every time in java8 > On Sept. 22, 2015, 8:07 a.m., Rajat Khandelwal wrote: > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestColumnarSQLRewriter.java, > > line 88 > > <https://reviews.apache.org/r/38521/diff/4/?file=1079591#file1079591line88> > > > > This `compareJoinQueries` is different than the other > > `compareJoinQueries`? Actually I can remove this method. Initially, I have added logic for comparing the join strings separately. Now we are directly comparing the queries. No separate logic required in comparing those join strings. > On Sept. 22, 2015, 8:07 a.m., Rajat Khandelwal wrote: > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestColumnarSQLRewriter.java, > > line 459 > > <https://reviews.apache.org/r/38521/diff/4/?file=1079591#file1079591line459> > > > > +1 for changing underlying structures so that both java7 and java8 > > behave in similar manner. After using the LinkedHashSet in ColumnarSQLRewriter, elements are coming in the same order in both java7 and java8. Changed the order accordingly. > On Sept. 22, 2015, 8:07 a.m., Rajat Khandelwal wrote: > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java, > > line 167 > > <https://reviews.apache.org/r/38521/diff/4/?file=1079586#file1079586line167> > > > > For existing test cases doing this might seem fine, but when someone is > > writing a new test case, we can't expect them to > > > > 1. either write test cases like this. (split and set check for each new > > test case) > > 2. or run with both java 7 and java 8 just to figure out there'll be a > > difference and needs special handling > > > > > > It would be better if the underlying structure supports comparison so > > that the comparison logic is not: > > > > 1. outside of the structure and > > 2. repeated several times Agree. Iterator on sets are giving elements in different order in java7 and java8. We are forming the key by joining the fact tables. The key which we are getting in java7 and jav8 differnt. > On Sept. 22, 2015, 8:07 a.m., Rajat Khandelwal wrote: > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java, > > line 313 > > <https://reviews.apache.org/r/38521/diff/4/?file=1079585#file1079585line313> > > > > Instead of making a new method, I'd say make a class `Query`. Then in > > the old compare method, take two strings and make to objects. Inside the > > constructor, the clauses can be extracted. Then `equals` can be defined on > > that class. that combined with `toString` will give nice output when we > > directly use `Assert.assertEquals(query1, query2)` instead of doing all > > string processing outside. Sure. Will make changes accrodingly. - Raju ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38521/#review99944 ----------------------------------------------------------- On Sept. 22, 2015, 1:19 a.m., Raju Bairishetti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38521/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2015, 1:19 a.m.) > > > Review request for lens, Amareshwari Sriramadasu and Rajat Khandelwal. > > > Bugs: LENS-607 > https://issues.apache.org/jira/browse/LENS-607 > > > Repository: lens > > > Description > ------- > > Iterators in java7 and java8 are returning elements in different order. All > most all failures are because of this case. > > Changes: > TestBaseCubeQueries: Changed driver supported storages to single storage > instead of multiple > TestCubeRewriter: Returning diffrent orders in the joins. Separated join part > from the actaul query part and verified join part and remaining query part > separately. > TestStorageUtil: Multiple sets can answer the given partitions. Changed > asserts accordingly. > TestTimeRangeResolver: Changed asserts to cover all the errors. > > *ColumnSQLRewriter* : Changed all data structures to *Linked* datastructures > (Arraylist --> LinkedList, HashSet to LinkedHashSet, HashMap to > LinkedHashMap) to return the elements in insertion order. > > TestColumnSQLRewriter: Changed the order of columns in queries. > > > Diffs > ----- > > lens-cli/src/test/java/org/apache/lens/cli/TestLensCubeCommands.java > 39441c9 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java > 0f76c76 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java > a58f5fe > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java > bde4edd > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java > 493b8d6 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestStorageUtil.java > 81f515b > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestTimeRangeResolver.java > cb27d50 > > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/ColumnarSQLRewriter.java > 295b476 > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestColumnarSQLRewriter.java > db09a4b > > Diff: https://reviews.apache.org/r/38521/diff/ > > > Testing > ------- > > Tested on both java7 and java8. Build is successful. Will post the build > artifact summary soon. > > c1mng0pxdty3:lens raju.bairishetti$ java -version > **java version "1.8.0_40"** > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules .............................. SUCCESS [ 4.849 > s] > [INFO] Lens ............................................... SUCCESS [ 5.806 > s] > [INFO] Lens API ........................................... SUCCESS [ 31.562 > s] > [INFO] Lens API for server and extensions ................. SUCCESS [ 23.200 > s] > [INFO] Lens Cube .......................................... SUCCESS [06:19 > min] > [INFO] Lens DB storage .................................... SUCCESS [ 23.015 > s] > [INFO] Lens Query Library ................................. SUCCESS [ 18.249 > s] > [INFO] Lens Hive Driver ................................... SUCCESS [03:11 > min] > [INFO] Lens Driver for JDBC ............................... SUCCESS [ 41.094 > s] > [INFO] Lens Elastic Search Driver ......................... SUCCESS [ 26.188 > s] > [INFO] Lens Server ........................................ SUCCESS [12:38 > min] > [INFO] Lens client ........................................ SUCCESS [ 44.951 > s] > [INFO] Lens CLI ........................................... SUCCESS [04:57 > min] > [INFO] Lens Examples ...................................... SUCCESS [ 11.491 > s] > [INFO] Lens Distribution .................................. SUCCESS [ 19.483 > s] > [INFO] Lens ML Lib ........................................ SUCCESS [02:35 > min] > [INFO] Lens ML Ext Distribution ........................... SUCCESS [ 7.919 > s] > [INFO] Lens Regression .................................... SUCCESS [ 11.523 > s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 34:13 min > [INFO] Finished at: 2015-09-19T12:13:27+05:30 > [INFO] Final Memory: 158M/1483M > [INFO] > ------------------------------------------------------------------------ > > > JDK-7: **sun-jdk-1.7.0_55** > > regression-2.4.0-beta-SNAPSHOT-tests.jar > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [1.999s] > [INFO] Lens .............................................. SUCCESS [2.816s] > [INFO] Lens API .......................................... SUCCESS [23.685s] > [INFO] Lens API for server and extensions ................ SUCCESS [20.093s] > [INFO] Lens Cube ......................................... SUCCESS [5:02.073s] > [INFO] Lens DB storage ................................... SUCCESS [19.937s] > [INFO] Lens Query Library ................................ SUCCESS [15.858s] > [INFO] Lens Hive Driver .................................. SUCCESS [2:50.353s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [37.076s] > [INFO] Lens Elastic Search Driver ........................ SUCCESS [15.348s] > [INFO] Lens Server ....................................... SUCCESS [5:42.566s] > [INFO] Lens client ....................................... SUCCESS [34.983s] > [INFO] Lens CLI .......................................... SUCCESS [2:34.585s] > [INFO] Lens Examples ..................................... SUCCESS [8.688s] > [INFO] Lens Distribution ................................. SUCCESS [8.809s] > [INFO] Lens ML Lib ....................................... SUCCESS [1:28.010s] > [INFO] Lens ML Ext Distribution .......................... SUCCESS [2.393s] > [INFO] Lens Regression ................................... SUCCESS [14.258s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 21:04.437s > [INFO] Finished at: Sat Sep 19 06:48:15 UTC 2015 > [INFO] Final Memory: 178M/1113M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Raju Bairishetti > >
