----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29919/#review68897 -----------------------------------------------------------
Other changes look good. Can you update brief errors for examples you provided? - Amareshwari Sriramadasu On Jan. 21, 2015, 10:44 a.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29919/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2015, 10:44 a.m.) > > > Review request for lens. > > > Bugs: LENS-149 > https://issues.apache.org/jira/browse/LENS-149 > > > Repository: lens > > > Description > ------- > > 1. In the json output, showing [(list of facts) -> (error)] instead of (list > of [fact -> error]). Many facts have same error, makes the error somewhat > compact. > 2. converting getters/setters to annotations. > 3. Many testcases were using this pattern: > > ``` > SemanticException th = null; > try{ > rewrite(something, something); > Assert.fail("Should have thrown exception"); > } catch (SemanticException e) { > e.printStackTrace(); > th = e; > } > Assert.assertNotNull(th); > //Other asserts on th > ``` > > Introduced a function `getSemanticExceptionInRewrite` which abstracts out > this part and directly returns `SemanticException`. > 4. Abstracted out `Map<? extends AbstractCubeTable, > List<CandidateTablePruneCause>>` as a separate class(`PruneCauses`). In > `CubeQueryContext`, `PruneCauses` is used for both fact prune causes and > dimtable prune causes. Later on PruneCauses can be implemented as a guavava > multimap if need be. > 5. Modified test cases to check prune cause wherever possible. > 6. I don't think defining an order between Error codes is a good idea. > Consider the following scenario: > > > There are four Prune causes: C1, C2, C3, C4. C1 < C2 < C3 < C4. For a > particular query Tables T1, T2, T3, T4 are picked. T2 is rejected with cause > C2, T3 with C3, T4 with C4. T1 is the only one that gets picked in the > regular case. Now for some reason(e.g. trying the query after some server > config changes), T1 is rejected with C1. If we honor the ordering we'll only > show T4 was rejected with C4. This might be a wrong depiction of the actual > error and would be misleading for both the end user and the > developers/debuggers. > > > Diffs > ----- > > lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java > 33a4a7c7357206d48dbc8c91360eb57851f9572d > > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTablePruneCause.java > b346da6f2a0d18eb11a04a4c4e88d85ba4c2bff9 > > lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java > 4d472acfe2d0d0ef5d505bffc87c1261af76decc > lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java > 4a03701c73125a14f72378ab84234ad6563e6db8 > > lens-cube/src/main/java/org/apache/lens/cube/parse/DenormalizationResolver.java > 5643b361bc0fdd4571ad7ac1c628ce301b552cf3 > lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java > 0bcc55855e11d2ce55ff73ee10674f9664dcc484 > > lens-cube/src/main/java/org/apache/lens/cube/parse/LightestDimensionResolver.java > 6074952ec2d2d49d24dd227186f4213ddba2ad86 > lens-cube/src/main/java/org/apache/lens/cube/parse/PruneCauses.java > PRE-CREATION > > lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java > 06550b81988aa5cee06c86c9725770cadee58b3e > lens-cube/src/main/java/org/apache/lens/driver/cube/RewriteUtil.java > d2da97d3ad8d20690ff0d2e03d0ff860f6b70c90 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java > 5d1b632223692af5d0af6514fe9b3d76e2f32fad > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java > cb70b66fd966bfb71ea67725e15de454a4eb9e32 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java > 42d5a70e1f09289ad1c8ee5e4328e8a5d23c3a28 > > lens-cube/src/test/java/org/apache/lens/cube/parse/TestExpressionResolver.java > 771a4ce148ed44c7c7eae94f53e13e5ac8d2cd83 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java > 8787b801a7593f393203c54c0f7bb09aeed875c5 > lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryRewrite.java > a8cdb381235365db95e9113ab44181a9d3c6ceb9 > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java > ce62b65f35c610267d452b32e9a21c1673bb5c2f > > lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java > 19deea6a4a7bfee9d3e1a59542d556d7446e324c > > lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java > 28f6aa469bfe7c74a3c4da6bb402962f52933f49 > > Diff: https://reviews.apache.org/r/29919/diff/ > > > Testing > ------- > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [2.668s] > [INFO] Lens .............................................. SUCCESS [1.627s] > [INFO] Lens API .......................................... SUCCESS [6.922s] > [INFO] Lens API for server and extensions ................ SUCCESS [6.340s] > [INFO] Lens Cube ......................................... SUCCESS [7:15.327s] > [INFO] Lens DB storage ................................... SUCCESS [12.629s] > [INFO] Lens Query Library ................................ SUCCESS [5.685s] > [INFO] Lens Hive Driver .................................. SUCCESS [3:16.607s] > [INFO] Lens Driver for Cloudera Impala ................... SUCCESS [3.937s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [27.892s] > [INFO] Lens Server ....................................... SUCCESS [5:01.175s] > [INFO] Lens client ....................................... SUCCESS [27.307s] > [INFO] Lens CLI .......................................... SUCCESS [2:02.313s] > [INFO] Lens Examples ..................................... SUCCESS [1.011s] > [INFO] Lens Distribution ................................. SUCCESS [4.181s] > [INFO] Lens Client Distribution .......................... SUCCESS [7.506s] > [INFO] Lens ML Lib ....................................... SUCCESS [47.106s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 20:11.370s > [INFO] Finished at: Mon Jan 19 12:46:14 UTC 2015 > [INFO] Final Memory: 110M/1420M > [INFO] > ------------------------------------------------------------------------ > > Some examples of the error json: > > ``` > { > "testfact3_base,testfact3_raw_base": { > "cause": "COLUMN_NOT_FOUND", > "missingColumns": [ > "cityid" > ] > }, > "testfact2_raw_base,testfact2_base": { > "cause": "COLUMN_NOT_FOUND", > "missingColumns": [ > "msr3", > "msr13" > ] > } > } > ``` > > ``` > { > "summary2,testfact2_raw,summary3": { > "cause": "INVALID_DENORM_TABLE" > }, > "summary4": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "C2": "UNSUPPORTED" > } > }, > "summary1,cheapfact,testfactmonthly,testfact2,testfact": { > "cause": "COLUMN_NOT_FOUND", > "missingColumns": [ > "dim2big1", > "dim2" > ] > } > } > ``` > > > ``` > { > "cheapfact": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "C99": "UNSUPPORTED" > }, > "updatePeriodCauses": {} > }, > "summary1": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2_summary1": "PART_COL_DOES_NOT_EXIST" > } > }, > "summary2": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2_summary2": "PART_COL_DOES_NOT_EXIST" > } > }, > "summary3": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2_summary3": "PART_COL_DOES_NOT_EXIST" > } > }, > "summary4": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2_summary4": "PART_COL_DOES_NOT_EXIST" > } > }, > > "summary1,summary2,testfact2_raw,summary3,summary4,testfactmonthly,testfact2,testfact": > { > "cause": "NO_FACT_UPDATE_PERIODS_FOR_GIVEN_RANGE" > } > } > ``` > > ``` > { > "cheapfact": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "C99": "UNSUPPORTED" > } > }, > "summary1,summary2,testfact2_raw,summary3,summary4,testfact2": { > "cause": "MORE_WEIGHT" > }, > "testfact": { > "cause": "MISSING_PARTITIONS", > "missingPartitions": [ > "2015-01-18", > "2015-01-17-18", > "2015-01-17-19", > "2015-01-17-20", > "2015-01-17-21", > "2015-01-17-22", > "2015-01-17-23", > "2015-01-19-00", > "2015-01-19-01", > "2015-01-19-02", > "2015-01-19-03", > "2015-01-19-04", > "2015-01-19-05", > "2015-01-19-06", > "2015-01-19-07", > "2015-01-19-08", > "2015-01-19-09", > "2015-01-19-10", > "2015-01-19-11", > "2015-01-19-12", > "2015-01-19-13", > "2015-01-19-14", > "2015-01-19-15", > "2015-01-19-16", > "2015-01-19-17" > ] > }, > "testfactmonthly": { > "cause": "NO_FACT_UPDATE_PERIODS_FOR_GIVEN_RANGE" > } > } > ``` > > > ``` > { > "testfact2_raw,summary4,testfactmonthly,testfact2": { > "cause": "NO_FACT_UPDATE_PERIODS_FOR_GIVEN_RANGE" > }, > "cheapfact": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "C99": "UNSUPPORTED" > } > }, > "summary1": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2_summary1": "PART_COL_DOES_NOT_EXIST" > } > }, > "summary2": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2_summary2": "PART_COL_DOES_NOT_EXIST" > } > }, > "summary1,summary2,summary3,testfact": { > "cause": "MISSING_PARTITIONS", > "missingPartitions": [ > "2015-01-18", > "2015-01-17-19", > "2015-01-17-20", > "2015-01-17-21", > "2015-01-17-22", > "2015-01-17-23", > "2015-01-17-18-45", > "2015-01-17-18-46", > "2015-01-17-18-47", > "2015-01-17-18-48", > "2015-01-17-18-49", > "2015-01-17-18-50", > "2015-01-17-18-51", > "2015-01-17-18-52", > "2015-01-17-18-53", > "2015-01-17-18-54", > "2015-01-17-18-55", > "2015-01-17-18-56", > "2015-01-17-18-57", > "2015-01-17-18-58", > "2015-01-17-18-59" > ] > }, > "summary3": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2_summary3": "PART_COL_DOES_NOT_EXIST" > } > }, > "summary4": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2_summary4": "PART_COL_DOES_NOT_EXIST" > } > } > } > ``` > > > Thanks, > > Rajat Khandelwal > >
