----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29919/#review69159 -----------------------------------------------------------
Changes look fine. Can you update the brief errors for examples? - Amareshwari Sriramadasu On Jan. 22, 2015, 10:51 a.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29919/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2015, 10:51 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 > > Diff: https://reviews.apache.org/r/29919/diff/ > > > Testing > ------- > > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [2.671s] > [INFO] Lens .............................................. SUCCESS [1.890s] > [INFO] Lens API .......................................... SUCCESS [6.569s] > [INFO] Lens API for server and extensions ................ SUCCESS [7.130s] > [INFO] Lens Cube ......................................... SUCCESS [7:08.405s] > [INFO] Lens DB storage ................................... SUCCESS [12.492s] > [INFO] Lens Query Library ................................ SUCCESS [5.427s] > [INFO] Lens Hive Driver .................................. SUCCESS [3:16.284s] > [INFO] Lens Driver for Cloudera Impala ................... SUCCESS [3.694s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [29.635s] > [INFO] Lens Server ....................................... SUCCESS [5:12.396s] > [INFO] Lens client ....................................... SUCCESS [22.350s] > [INFO] Lens CLI .......................................... SUCCESS [1:58.029s] > [INFO] Lens Examples ..................................... SUCCESS [1.054s] > [INFO] Lens Distribution ................................. SUCCESS [4.236s] > [INFO] Lens Client Distribution .......................... SUCCESS [7.791s] > [INFO] Lens ML Lib ....................................... SUCCESS [47.096s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 20:08.261s > [INFO] Finished at: Thu Jan 22 10:26:55 UTC 2015 > [INFO] Final Memory: 110M/1221M > [INFO] > ------------------------------------------------------------------------ > > Some examples of the error json: > > ``` > { > "brief": "Column Sets: [[msr3, msr13], [cityid]] are not queriable > together", > "details": { > "testfact3_base,testfact3_raw_base": { > "cause": "COLUMN_NOT_FOUND", > "missingColumns": [ > "cityid" > ] > }, > "testfact2_raw_base,testfact2_base": { > "cause": "COLUMN_NOT_FOUND", > "missingColumns": [ > "msr3", > "msr13" > ] > } > } > } > ``` > > ``` > { > "brief": "No candidate storages for any table", > "details": { > "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" > ] > } > } > } > ``` > ``` > { > "brief": "No fact update periods for given range", > "details": { > "cheapfact": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "C99": "UNSUPPORTED" > } > }, > "summary1,summary2,summary3,summary4": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c2": "PART_COL_DOES_NOT_EXIST" > } > }, > > "summary1,summary2,testfact2_raw,summary3,summary4,testfactmonthly,testfact2,testfact": > { > "cause": "NO_FACT_UPDATE_PERIODS_FOR_GIVEN_RANGE" > } > } > } > ``` > > ``` > { > "brief": "No candidate storages for any table", > "details": { > "statetable": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "c1": "NO_PARTITIONS" > } > } > } > } > ``` > > ``` > { > "brief": "Missing partitions for the cube table: [[2015-01-21, > 2015-01-20-15, 2015-01-20-16, 2015-01-20-17, 2015-01-20-18, 2015-01-20-19, > 2015-01-20-20, 2015-01-20-21, 2015-01-20-22, 2015-01-20-23, 2015-01-22-00, > 2015-01-22-01, 2015-01-22-02, 2015-01-22-03, 2015-01-22-04, 2015-01-22-05, > 2015-01-22-06, 2015-01-22-07, 2015-01-22-08, 2015-01-22-09, 2015-01-22-10, > 2015-01-22-11, 2015-01-22-12, 2015-01-22-13, 2015-01-22-14]]", > "details": { > "summary1,summary2,testfact2_raw,summary3,summary4,testfact2": { > "cause": "MORE_WEIGHT" > }, > "cheapfact": { > "cause": "NO_CANDIDATE_STORAGES", > "storageCauses": { > "C99": "UNSUPPORTED" > } > }, > "testfact": { > "cause": "MISSING_PARTITIONS", > "missingPartitions": [ > "2015-01-21", > "2015-01-20-15", > "2015-01-20-16", > "2015-01-20-17", > "2015-01-20-18", > "2015-01-20-19", > "2015-01-20-20", > "2015-01-20-21", > "2015-01-20-22", > "2015-01-20-23", > "2015-01-22-00", > "2015-01-22-01", > "2015-01-22-02", > "2015-01-22-03", > "2015-01-22-04", > "2015-01-22-05", > "2015-01-22-06", > "2015-01-22-07", > "2015-01-22-08", > "2015-01-22-09", > "2015-01-22-10", > "2015-01-22-11", > "2015-01-22-12", > "2015-01-22-13", > "2015-01-22-14" > ] > }, > "testfactmonthly": { > "cause": "NO_FACT_UPDATE_PERIODS_FOR_GIVEN_RANGE" > } > } > } > ``` > > > Thanks, > > Rajat Khandelwal > >
