----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35821/#review90353 -----------------------------------------------------------
lens-api/src/main/java/org/apache/lens/api/query/FactPartitionBasedQueryCost.java (line 27) <https://reviews.apache.org/r/35821/#comment143368> @Data makes lot of things including public getter/setter method on member variables which violates data hiding. Please use @EqualsAndHashCode and @ToString only. I guess this class does not need @Getter as well. The only public methods present in this implementation should be of the implemented interface. Exceptions are equals, hashcode and toString which are already inherited from Object class. lens-api/src/main/java/org/apache/lens/api/query/FactPartitionBasedQueryCost.java (line 29) <https://reviews.apache.org/r/35821/#comment143369> Please make this final lens-api/src/main/java/org/apache/lens/api/query/FactPartitionBasedQueryCost.java (line 31) <https://reviews.apache.org/r/35821/#comment143370> Please add a final to argument. lens-api/src/main/java/org/apache/lens/api/query/FactPartitionBasedQueryCost.java (line 32) <https://reviews.apache.org/r/35821/#comment143374> Shall we have a check that partitionCost cannot be negative ? checkArgument(partitionCost >= 0, "Partition cost cannot be positive") lens-api/src/main/java/org/apache/lens/api/query/FactPartitionBasedQueryCost.java (line 36) <https://reviews.apache.org/r/35821/#comment143373> If @NonNull is not present and a null value is passed, code here will throw a NullPointerException at line 37 without going into any inconsistent state. In such cases, a precondition of @NonNull which throws a NullPointerException can be avoided. The written code will automatically fall into same behaviour without any extra check. lens-api/src/main/java/org/apache/lens/api/query/FactPartitionBasedQueryCost.java (line 56) <https://reviews.apache.org/r/35821/#comment143371> partitionCost gives an estimation of resource usage. In light of this, will it be ok to change return type of getEstimatedResourceUsage to float and return partition cost from getEstimatedResourceUsage. This will allow us to remove getEstimatedCost from the interface and also remove throws UnsupportedOperationException from estimatedResourceUsage ? lens-api/src/main/java/org/apache/lens/api/query/FactPartitionBasedQueryCost.java (line 61) <https://reviews.apache.org/r/35821/#comment143372> How about adding a final to argument ? - Himanshu Gahlaut On July 3, 2015, 12:08 p.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35821/ > ----------------------------------------------------------- > > (Updated July 3, 2015, 12:08 p.m.) > > > Review request for lens. > > > Bugs: LENS-630 > https://issues.apache.org/jira/browse/LENS-630 > > > Repository: lens > > > Description > ------- > > Along with this, we can add a new field in QueryCost to return the query cost > calculated by the implementation. normalizedQueryCost could be one name for > that field. > > > Diffs > ----- > > > lens-api/src/main/java/org/apache/lens/api/query/FactPartitionBasedQueryCost.java > PRE-CREATION > lens-api/src/main/java/org/apache/lens/api/query/QueryCost.java > 1a37c20f408dac8d007e917a3b1107c96acb51f1 > lens-api/src/main/java/org/apache/lens/api/query/QueryCostType.java > PRE-CREATION > lens-api/src/main/java/org/apache/lens/api/query/QueryPlan.java > a836b1e433b50e3137aba71ac8c8288ca7a2f5bf > lens-api/src/main/java/org/apache/lens/api/query/QuerySubmitResult.java > 2177c7887ff7cae395f9ef1a8dcf1f1489bcf6ca > lens-api/src/main/java/org/apache/lens/api/result/QueryCostTO.java > PRE-CREATION > lens-cube/src/main/java/org/apache/lens/cube/metadata/FactPartition.java > 3465513c10a13fb062a6b64e5589862d3d0141dd > lens-cube/src/main/java/org/apache/lens/cube/metadata/UpdatePeriod.java > 3c9268bc7f23687bd450cf01a44272df50a2df26 > lens-cube/src/main/java/org/apache/lens/driver/cube/RewriterPlan.java > 841946d2cc8ef9f1a590acd1693404fbdf60ac49 > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/HiveDriver.java > b7a3be72352cba201f7244822c08e59f298fc64c > > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/HiveQueryPlan.java > b0a04dfc59e2301aac336a97fe52024c4ebf76df > > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/priority/DurationBasedQueryPriorityDecider.java > bc1ec1c619f980d7a18314cb93d5438fc1d11321 > > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/priority/FactPartitionBasedQueryCostCalculator.java > PRE-CREATION > > lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestHiveDriver.java > 8355f29df831fad25348b51bd22b0cc45c968f76 > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java > bda6572adfcc4834197608fa43f68dbaabb2d852 > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java > be34164dd49b734b94fc2c121130613555402d50 > lens-examples/src/main/resources/city-local-part.xml > 528b8808ab7c9e508e4bf00c158a8b319d957f93 > lens-examples/src/main/resources/sales-aggr-fact1.xml > 728c77548ddcf22002c8927ba57a9fad8e01cf31 > > lens-regression/src/main/java/org/apache/lens/regression/core/helpers/QueryHelper.java > 8957aefec894485fc3d1e26e3f3b4f79eb01915e > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/DriverQueryPlan.java > 51b12177a3b7c6db5beb4b98a892bc83cc94fcdf > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java > 3dc4e734eac979d7bd8f94ee3cb4eb986d4d5081 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/MinQueryCostSelector.java > 43237f76a75e5c020534548bb9ec5e423e2b360c > > lens-server-api/src/main/java/org/apache/lens/server/api/priority/CostRangePriorityDecider.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/priority/QueryCostCalculator.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/priority/QueryPriorityDecider.java > 09a8ace5e4738b338ec241c83e297d071949a4bf > > lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java > c398215f415c20b2bfcd3d743316f7300ac2baa1 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/DriverSelectorQueryContext.java > 09452226337bbacb12216efd43f15a2b957c0cf4 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryExecutionService.java > 3c0f7876d96e2f7dd9a0468ec94a7abcec41f819 > > lens-server-api/src/test/java/org/apache/lens/server/api/driver/MockDriver.java > 4b80918f3d1cf9f3f683fbdb4d1a89bc55e4b0f9 > > lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java > ec97b8df25f57a283d4d539d635c011cd06819d6 > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 655549b25aa2e6aef507c164c0e87d69e856961e > > Diff: https://reviews.apache.org/r/35821/diff/ > > > Testing > ------- > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [2.387s] > [INFO] Lens .............................................. SUCCESS [3.277s] > [INFO] Lens API .......................................... SUCCESS [20.678s] > [INFO] Lens API for server and extensions ................ SUCCESS [20.428s] > [INFO] Lens Cube ......................................... SUCCESS [3:37.068s] > [INFO] Lens DB storage ................................... SUCCESS [19.018s] > [INFO] Lens Query Library ................................ SUCCESS [15.157s] > [INFO] Lens Hive Driver .................................. SUCCESS [2:56.027s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [36.331s] > [INFO] Lens Server ....................................... SUCCESS [5:45.727s] > [INFO] Lens client ....................................... SUCCESS [38.252s] > [INFO] Lens CLI .......................................... SUCCESS [2:35.316s] > [INFO] Lens Examples ..................................... SUCCESS [9.162s] > [INFO] Lens Distribution ................................. SUCCESS [8.978s] > [INFO] Lens ML Lib ....................................... SUCCESS [1:21.324s] > [INFO] Lens ML Ext Distribution .......................... SUCCESS [1.613s] > [INFO] Lens Regression ................................... SUCCESS [10.724s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 19:22.407s > [INFO] Finished at: Wed Jun 24 09:32:15 UTC 2015 > [INFO] Final Memory: 173M/1360M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Rajat Khandelwal > >
