> On April 11, 2017, 3:51 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/UnionQueryWriter.java
> > Line 700 (original), 701 (patched)
> > <https://reviews.apache.org/r/57947/diff/9/?file=1685324#file1685324line712>
> >
> >     Not sure if removing factDimMap can cause wrong queries

While writing union queries, we don't perform any joins outside the union. The 
joins are to be performed for Storage Candidates. So since 
`StorageCandidateHQLContext` already has `factDimMap`, we don't need anything 
outside.


> On April 11, 2017, 3:51 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/join/AutoJoinContext.java
> > Lines 164 (patched)
> > <https://reviews.apache.org/r/57947/diff/9/?file=1685325#file1685325line175>
> >
> >     qdims can be different from dimsToQuery list for a fact.

That is somehow getting taken care by the changes. Can you explain one such 
scenario and then I might be able to better reason about how/why it's working. 
I'll still somewhat explain the changes. 

So we were keeping two data structures for handling dimensions: one 
`Set<Dimension>` and one `Map<Dimension, CandidateDim>`. I wanted to reduce the 
duplications if any. It turned out the first one can be removed and keys of the 
map can be used everywhere, since whenever we used to update the set, we used 
to put entries in the map as well. So the invariant "the set is always equal to 
the keyset" holds.


> On April 11, 2017, 3:51 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java
> > Line 88 (original), 88 (patched)
> > <https://reviews.apache.org/r/57947/diff/9/?file=1685330#file1685330line88>
> >
> >     Revisit this error change

As explained in another reply, CandidateTableResolver no longer throws 
exception when it doesn't find any candidate.


> On April 11, 2017, 3:51 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java
> > Lines 1620-1628 (original), 1614-1622 (patched)
> > <https://reviews.apache.org/r/57947/diff/9/?file=1685331#file1685331line1620>
> >
> >     We might have to revisit these error changes.

Same issue as above comment


> On April 11, 2017, 3:51 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeSegmentationRewriter.java
> > Lines 113 (patched)
> > <https://reviews.apache.org/r/57947/diff/9/?file=1685332#file1685332line113>
> >
> >     Finish this test?

Yeah, dependent on sushil's changes to convert `sum(0.0) -> 0.0`. Didn't want 
to write asserts and have to modify them later. Although I've done a manual 
inspection of the generated query and it looked fine.


> On April 11, 2017, 3:51 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeSegmentationRewriter.java
> > Lines 198-211 (patched)
> > <https://reviews.apache.org/r/57947/diff/9/?file=1685332#file1685332line198>
> >
> >     Add asserts for these tests.

Same as above comment.


> On April 11, 2017, 3:51 p.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java
> > Line 679 (original), 671 (patched)
> > <https://reviews.apache.org/r/57947/diff/9/?file=1685334#file1685334line679>
> >
> >     Why is this changed to NO_CANDIDATE_FACT_AVAILABLE?

Reason is changes done in CandidateTableResolver.


- Rajat


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57947/#review171541
-----------------------------------------------------------


On April 5, 2017, 7:25 p.m., Rajat Khandelwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57947/
> -----------------------------------------------------------
> 
> (Updated April 5, 2017, 7:25 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-974
>     https://issues.apache.org/jira/browse/LENS-974
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> With cube segmentation a cube can have multiple cubes and all these child 
> cubes together will make the cube complete. 
> 
> CubeSegmentation and  CubeFactTable will sit together, which means it can 
> belong to only one base cube. A base cube can have one or more cube 
> segmentations. Fields of segmentation will be intersection of all columns of 
> its cubes. Segmentation will have weight to compare with its buddies (facts 
> or other segmentations). Also it can have start and end time defined or it 
> can derive from its underline facts. 
> 
> eg: 
> base_cube
>   |_fact1
>   |_fact2
>   |_cube_segment1
>      |_cube1
>         |_fact_11
>         |_fact_12
>         ... 
>       ...
>   |_cube_segment2
>      |_cube2
>         |_fact_21
>         |_fact_22
>         ... 
>       ...
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/java/org/apache/lens/api/ds/Tuple2.java PRE-CREATION 
>   lens-api/src/main/resources/cube-0.1.xsd 
> 1d8a6241a85066f111b8490e8e03516e4848cf41 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/Cube.java 
> b376aaf463f991c540b7c711a90271551848a6e2 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/DateUtil.java 
> d10d72e1dcbd68ded9c09e04144b728b9755ef5d 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 
> 57d4502d0429c355bdf4127406512b6a967585e1 
>   
> lens-cube/src/main/java/org/apache/lens/cube/metadata/TimePartitionRange.java 
> 2e85111583eb8d80427df91607411581d4ccd38c 
>   lens-cube/src/main/java/org/apache/lens/cube/metadata/TimeRange.java 
> 242d3ba803b5266f351bdf15da90469a3a7c5cdc 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AggregateResolver.java 
> 30b1a904ddf2dcc4a3cd66732059fb146acc0c20 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/AliasReplacer.java 
> da342426924a54d4b53a3fcf83dda61193ed86d6 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/Candidate.java 
> f241cb3111426f820beffe7c63209162bfb50f39 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateCoveringSetsResolver.java
>  0aafda619c20e893ae99abdc76b6797c9da6705f 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateSegmentResolver.java
>  PRE-CREATION 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java
>  6d61f1fc152eb89efa71ac7a1a26f81f9eadafbf 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateUtil.java 
> 5db1344ed5de6b1f36a1f877d8c5a2906c7eb99c 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ColumnResolver.java 
> 21cdd26a965b6ae2f63a3c96ade55e7773a7d13d 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryConfUtil.java 
> 300d798f9dd5760853d86d106ec2b9fc1e91afb5 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryContext.java 
> 193bf440cbb52844711fba63622aead2721feab2 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/CubeQueryRewriter.java 
> 57130697c7ad3c3038b70994cdd61a7c70574f63 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DefaultQueryAST.java 
> 29da0a2cf64f28cd1dbd2d52cc6609aaefc07a50 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/DenormalizationResolver.java
>  e5cf916648da1bd9a8095bd004b8fd3b2cfe2e47 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DimHQLContext.java 
> 95d65728457d24ac58fcaedc0d4c7a9b2e71fd71 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/DimOnlyHQLContext.java 
> 6f6572ea3ba5ae5c6cb7ad2bc0eb9ce058f0ccc2 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/ExpressionResolver.java 
> aaa183b6de2e98d83f34769b7cad947450fa4930 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/GroupbyResolver.java 
> c9dc7b2351875c85318971b003516ae0888d59af 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLContextInterface.java 
> 78d448ab1d145925ebfe27bb86333c8a971339ae 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/HQLParser.java 
> 8a70535203f038b07f387b20adf7343e593d899d 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/JoinCandidate.java 
> 6334062701c35733e2a0f15a69c8e9fee77eccca 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/JoinResolver.java 
> 03709646b2f9fe7d26d524ef744ee07316f04684 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/LeastPartitionResolver.java
>  153df245cd2b79ee29991f98df36f57e4e029d2f 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/MaxCoveringFactResolver.java
>  4f4e3ab4d811db4971f69b0cf2cc2a62f620ec29 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/MultiCandidateQueryWriterContext.java
>  PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/PruneCauses.java 
> 50ccab5896fa1a2e291e4aaaff1d20af93b1a64e 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/QueriedPhraseContext.java 
> 310a655cd2102ce7c8da19ae3301cf9b241caff4 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/QueryWriter.java 
> PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/QueryWriterContext.java 
> PRE-CREATION 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/SegmentationCandidate.java 
> PRE-CREATION 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/SimpleHQLContext.java 
> 77ebe82fa62d85af03d55d6a1772b08171274fd1 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidate.java 
> 628e9aaf9f6aeda8697f4018e729156edd261333 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidateHQLContext.java
>  PRE-CREATION 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java 
> 1a2d9a99fc896086f8d664023233c472c2a27eb6 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/StorageUtil.java 
> f5cd540e46a68e5fb2fc4dd59f1ee6a9ec380693 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/UnionCandidate.java 
> 62ebf71b09d5845a04f03c4f770112c0d25659c5 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/UnionQueryWriter.java 
> f2325f18de1469b8186bdbeb1e7294988e9492b2 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/join/AutoJoinContext.java 
> aab671e2385cd92b50b344bd1a60871cf8c24173 
>   
> lens-cube/src/main/java/org/apache/lens/cube/parse/join/BridgeTableJoinContext.java
>  ab5c4f9bda147385c65ee9ce1bf9349383df2021 
>   lens-cube/src/main/java/org/apache/lens/cube/parse/join/JoinClause.java 
> 432525270966a203ab0f75e47f8d1aa48704d835 
>   lens-cube/src/test/java/org/apache/lens/cube/metadata/TestDateUtil.java 
> 8b3b4baacd2b7b08db37c321f5aff06e28ca8df1 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 
> 62d73861e01a7110dc5130f613c6e090d9fb2d06 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestBaseCubeQueries.java 
> ba8a5e41490024b641ad55fc4bff2f2ce65f36ca 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java 
> bf1c151a096994a3b549e4e40b7e9c62e25cfc50 
>   
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeSegmentationRewriter.java
>  PRE-CREATION 
>   
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestDenormalizationResolver.java
>  7874a6613656deb58b7e2c8db36759a8f459eb5f 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestJoinResolver.java 
> f5ddf7bba365a6313860191a0609ab0e729d4d67 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryMetrics.java 
> 3883beed7bf123d01ffc31d9d27b0b1d0a79331a 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/TestQueryRewrite.java 
> 17a8b0f75a777f55a5c7ba930c3e441351b7c434 
>   lens-cube/src/test/resources/schema/cubes/base/b1cube.xml PRE-CREATION 
>   lens-cube/src/test/resources/schema/cubes/base/b2cube.xml PRE-CREATION 
>   lens-cube/src/test/resources/schema/cubes/base/basecube.xml 
> b1fea1c3399bac1d3bbc3f18337de7c7e615b76c 
>   lens-cube/src/test/resources/schema/cubes/base/testcube.xml 
> 0338f550291b081a1251a986fe1f92c29f9a42e0 
>   lens-cube/src/test/resources/schema/facts/b1b2fact1.xml PRE-CREATION 
>   lens-cube/src/test/resources/schema/facts/b1fact1.xml PRE-CREATION 
>   lens-cube/src/test/resources/schema/facts/b2fact1.xml PRE-CREATION 
>   lens-cube/src/test/resources/schema/facts/testfact2.xml 
> d6006c65a52e3c3f5e44cd942b147572a31c9fc6 
>   lens-cube/src/test/resources/schema/facts/union_join_ctx_fact1.xml 
> d07393d6717ea5350855786c26ee7e750bfe48ed 
>   lens-cube/src/test/resources/schema/segmentations/seg1.xml 
> 7ed48a1ca97176e59b5481ad57c864e3675d89bd 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/query/comparators/ChainedComparator.java
>  2cff8d80fcfff16446f30bc53be2a84d9b95dc83 
>   
> lens-server-api/src/test/java/org/apache/lens/server/api/query/comparators/ChainedComparatorTest.java
>  cc587519dda64036ba56c15690d12c9b57c0afc6 
>   
> lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
>  c6fbedab81779583fee4c5b7ea4c0ba69fd93b8c 
> 
> 
> Diff: https://reviews.apache.org/r/57947/diff/9/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rajat Khandelwal
> 
>

Reply via email to