----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31154/#review75476 -----------------------------------------------------------
lens-client/src/main/java/org/apache/lens/client/LensClientSingletonWrapper.java <https://reviews.apache.org/r/31154/#comment122539> Can you remove system.out statements? I think they will be handled in LENS-391 lens-cube/src/main/java/org/apache/lens/cube/metadata/CaseInsensitiveStringHashMap.java <https://reviews.apache.org/r/31154/#comment122540> Isn't the type of key String here? lens-cube/src/main/java/org/apache/lens/cube/metadata/CaseInsensitiveStringHashMap.java <https://reviews.apache.org/r/31154/#comment122541> type of key - String? lens-cube/src/main/java/org/apache/lens/cube/metadata/CaseInsensitiveStringHashMap.java <https://reviews.apache.org/r/31154/#comment122542> type of key - String? lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122543> Right now the non of the classes in org.apache.lens.cube.metadata depend on org.apache.lens.cube.parse. Its only the other way. Should FactPartition be moved to metadata? lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122544> Why are we not populating the cache if not already populated? It might give wrong values because cache is populated for one of the storage and not for other storage. lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122545> can the third param be only partCol, instead of latestPartCol? lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122546> Lets name the variable partCols instead of cols. Can we move variables timeParts and partCols outside the loop? They are used not just in this for loop, but in other blocks of this method as well. They can be fetched once and used in all blocks. lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122547> could not understand the description. Is it updating the PartitionTimeline for all time partition columns of added partition? If so, please update description and the method name as well. Should method name be updatePartitionTimelineForAddPartition lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122548> Do we need similar method for drop as well? lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122549> Shall we call the method partitionTimeExists? lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122551> Can we check usage of this method? lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartition.java <https://reviews.apache.org/r/31154/#comment122552> Please include unit tests for all the helper methods, trucation and date format use cases. lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java <https://reviews.apache.org/r/31154/#comment122553> what does excluding edges mean? Can you include example here? lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java <https://reviews.apache.org/r/31154/#comment122554> What is the significance of return value here? Is it saying true when updated and false othewise? lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java <https://reviews.apache.org/r/31154/#comment122556> Why is result value boolean & here? lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java <https://reviews.apache.org/r/31154/#comment122555> Again, whats the significance of return value here? lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/StoreAllPartitionTimeline.java <https://reviews.apache.org/r/31154/#comment122557> I hope this is not the default timeline right now. Added only for exploration purpose. If so, can you add a comment? lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java <https://reviews.apache.org/r/31154/#comment122558> Please go ahead and remove commented code lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java <https://reviews.apache.org/r/31154/#comment122559> Can we use constant strings for properties here? lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java <https://reviews.apache.org/r/31154/#comment122560> Can we enable asserts now? It should be easy to construct expected queries now, since all partitions are getting added. - Amareshwari Sriramadasu On March 5, 2015, 3:36 p.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31154/ > ----------------------------------------------------------- > > (Updated March 5, 2015, 3:36 p.m.) > > > Review request for lens. > > > Bugs: LENS-281 > https://issues.apache.org/jira/browse/LENS-281 > > > Repository: lens > > > Description > ------- > > finished with changes. > > > Diffs > ----- > > > lens-client/src/main/java/org/apache/lens/client/LensClientSingletonWrapper.java > 05964e1021910038f46a2ca141d1bf56ee2f4e03 > > lens-cube/src/main/java/org/apache/lens/cube/metadata/CaseInsensitiveStringHashMap.java > PRE-CREATION > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java > b3ed6efd102ed800519ebb9b5d15f81a2a32d5d5 > > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java > 7e6fe7e93a07ddb6ae4369b78e7856dc2b0a65b5 > > lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreConstants.java > 979de6d7f1d43f0fb5621e3287f3a6eef3dcdaef > lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java > c21c72a5ceb0271134423b8a9caf928e524cf150 > lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java > 240516d6911ecdab6a07ef77b194c013aba42f4e > lens-cube/src/main/java/org/apache/lens/cube/metadata/StorageConstants.java > c4b3c3a4d469f5cc609db67437b00855d907c51d > lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartition.java > PRE-CREATION > > lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java > PRE-CREATION > > lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/PartitionTimeline.java > PRE-CREATION > > lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/PartitionTimelineFactory.java > PRE-CREATION > > lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/StoreAllPartitionTimeline.java > PRE-CREATION > > lens-cube/src/main/java/org/apache/lens/cube/parse/AbridgedTimeRangeWriter.java > b1a85adfb229fa0f6a708a047a7629b2faa28e30 > lens-cube/src/main/java/org/apache/lens/cube/parse/FactPartition.java > a60041bf3df0fa03091674b602a78a2323f88d55 > > lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java > 7eaf597bac879430dff086914865d34102003f0f > lens-cube/src/main/java/org/apache/lens/cube/parse/TimeRange.java > 550df6c99b7ffe7372d319b8c4b101cdd5a4f830 > > lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java > 96ba240404d1f21fbcac572ced0a4e3c911a5731 > lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java > 609b7f3a2750a0272f649d4cce3181a0588013e9 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java > 3c960622c4bc7d95bcbd9c66f3f64a8c7eff2cd2 > > lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java > 4047e0c8b2098929cd6f1d320f7e52db40a280aa > > lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java > 9f4595f96354eacabd758053b57183026de116c0 > > Diff: https://reviews.apache.org/r/31154/diff/ > > > Testing > ------- > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [3.118s] > [INFO] Lens .............................................. SUCCESS [3.121s] > [INFO] Lens API .......................................... SUCCESS [7.252s] > [INFO] Lens API for server and extensions ................ SUCCESS [7.777s] > [INFO] Lens Cube ......................................... SUCCESS [2:25.228s] > [INFO] Lens DB storage ................................... SUCCESS [9.340s] > [INFO] Lens Query Library ................................ SUCCESS [5.294s] > [INFO] Lens Hive Driver .................................. SUCCESS [2:33.994s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [27.057s] > [INFO] Lens Server ....................................... SUCCESS [4:36.485s] > [INFO] Lens client ....................................... SUCCESS [21.727s] > [INFO] Lens CLI .......................................... SUCCESS [1:58.127s] > [INFO] Lens Examples ..................................... SUCCESS [0.785s] > [INFO] Lens Distribution ................................. SUCCESS [9.733s] > [INFO] Lens ML Lib ....................................... SUCCESS [43.829s] > [INFO] Lens Regression ................................... SUCCESS [0.487s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 13:54.728s > [INFO] Finished at: Thu Mar 05 11:01:59 UTC 2015 > [INFO] Final Memory: 108M/1102M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Rajat Khandelwal > >
