You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Amareshwari Sriramadasu <am...@apache.org> on 2017/05/17 07:18:18 UTC
Re: Review Request 58796: CRUD and Query rewrite changes for Virtual
Fact and Unit test cases [Lens-1412]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58796/#review175209
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
Lines 95 (patched)
<https://reviews.apache.org/r/58796/#comment248618>
Why would we need this mapping? Can you add a comment on why this mapping is required?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
Line 194 (original), 188 (patched)
<https://reviews.apache.org/r/58796/#comment248617>
Should caller make sure not pass virtualFact name for this method? Or even we can do a check here and throw proper error.
If not check done from call and here, it can result 500 errors on api.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
Lines 2550 (patched)
<https://reviews.apache.org/r/58796/#comment248619>
It is dropping tables not just from cache, but persisted tables as well, might need rename.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
Line 2459 (original), 2599 (patched)
<https://reviews.apache.org/r/58796/#comment248620>
Can we make sure all callers for the methods where getCubeFact() is getting casted to (CubeFactTable), virtual fact table will never be passed?
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java
Lines 416 (patched)
<https://reviews.apache.org/r/58796/#comment248623>
can we put back the error?
lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidate.java
Line 607 (original), 597 (patched)
<https://reviews.apache.org/r/58796/#comment248624>
Seems getFact() can be virtual fact as well. Casting would result in error
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java
Lines 1264 (patched)
<https://reviews.apache.org/r/58796/#comment248625>
Can we make virtual fact table to belong to a different cube?
lens-server-api/src/main/java/org/apache/lens/server/api/metastore/CubeMetastoreService.java
Lines 469 (patched)
<https://reviews.apache.org/r/58796/#comment248626>
remove commented code
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java
Lines 490 (patched)
<https://reviews.apache.org/r/58796/#comment248627>
remove commented code
lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java
Lines 186 (patched)
<https://reviews.apache.org/r/58796/#comment248628>
returning all facts? Instead should we say not implemented
- Amareshwari Sriramadasu
On May 16, 2017, 12:21 p.m., Rajitha R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58796/
> -----------------------------------------------------------
>
> (Updated May 16, 2017, 12:21 p.m.)
>
>
> Review request for lens, Rajat Khandelwal and Puneet Gupta.
>
>
> Repository: lens
>
>
> Description
> -------
>
> CRUD changes and Unit test cases for Virtual Fact
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd 8158e6d
> lens-cube/src/main/java/org/apache/lens/cube/metadata/AbstractCubeTable.java 67aaff8
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java 88bc1fc
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java b5c4c89
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeVirtualFactTable.java PRE-CREATION
> lens-cube/src/main/java/org/apache/lens/cube/metadata/FactTable.java PRE-CREATION
> lens-cube/src/main/java/org/apache/lens/cube/metadata/JAXBUtils.java e1e3d16
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreConstants.java 88500fd
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 40f766b
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Segmentation.java 1e3cef3
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java a8d6fbd
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateUtil.java 6ba46d6
> lens-cube/src/main/java/org/apache/lens/cube/parse/DimHQLContext.java 43510dd
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidate.java 1e54f13
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidateHQLContext.java 730b802
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java 291712b
> lens-cube/src/test/java/org/apache/lens/cube/metadata/CubeFactTableTest.java 42e32cb
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java a76ef13
> lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 2f300d3
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java 7d1f80d
> lens-cube/src/test/resources/schema/cubes/base/virtualcube.xml PRE-CREATION
> lens-cube/src/test/resources/schema/facts/virtualfact.xml PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/metastore/CubeMetastoreService.java 28b9d22
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 194b380
> lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7451aa9
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 6d32d23
>
>
> Diff: https://reviews.apache.org/r/58796/diff/6/
>
>
> Testing
> -------
>
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Lens Checkstyle Rules ............................. SUCCESS [1.467s]
> [INFO] Lens .............................................. SUCCESS [3.449s]
> [INFO] Lens API .......................................... SUCCESS [23.572s]
> [INFO] Lens API for server and extensions ................ SUCCESS [20.051s]
> [INFO] Lens Cube ......................................... SUCCESS [5:41.995s]
> [INFO] Lens DB storage ................................... SUCCESS [17.013s]
> [INFO] Lens Query Library ................................ SUCCESS [13.632s]
> [INFO] Lens Hive Driver .................................. SUCCESS [1:16.131s]
> [INFO] Lens Driver for JDBC .............................. SUCCESS [48.127s]
> [INFO] Lens Elastic Search Driver ........................ SUCCESS [21.542s]
> [INFO] Lens Server ....................................... SUCCESS [16:38.220s]
> [INFO] Lens client ....................................... SUCCESS [1:25.658s]
> [INFO] Lens CLI .......................................... SUCCESS [1:16.332s]
> [INFO] Lens Examples ..................................... SUCCESS [8.432s]
> [INFO] Lens Ship Jars to Distributed Cache ............... SUCCESS [0.766s]
> [INFO] Lens Distribution ................................. SUCCESS [9.499s]
> [INFO] Lens ML Lib ....................................... SUCCESS [1:02.656s]
> [INFO] Lens ML Ext Distribution .......................... SUCCESS [2.552s]
> [INFO] Lens Regression ................................... SUCCESS [12.136s]
> [INFO] Lens UI ........................................... SUCCESS [32.999s]
> [INFO] Lens Contrib ...................................... SUCCESS [0.358s]
> [INFO] Lens Contributed Clients .......................... SUCCESS [0.360s]
> [INFO] Lens Python Client ................................ SUCCESS [0.366s]
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Rajitha R
>
>
Re: Review Request 58796: CRUD and Query rewrite changes for Virtual
Fact and Unit test cases [Lens-1412]
Posted by Rajitha R <ra...@gmail.com>.
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
> > Lines 95 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720857#file1720857line105>
> >
> > Why would we need this mapping? Can you add a comment on why this mapping is required?
done
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
> > Lines 2550 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720857#file1720857line2574>
> >
> > It is dropping tables not just from cache, but persisted tables as well, might need rename.
done
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
> > Line 2459 (original), 2599 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720857#file1720857line2623>
> >
> > Can we make sure all callers for the methods where getCubeFact() is getting casted to (CubeFactTable), virtual fact table will never be passed?
Done.
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java
> > Lines 416 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720862#file1720862line416>
> >
> > can we put back the error?
done
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidate.java
> > Line 607 (original), 597 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720867#file1720867line611>
> >
> > Seems getFact() can be virtual fact as well. Casting would result in error
Fixed
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java
> > Lines 1264 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720871#file1720871line1264>
> >
> > Can we make virtual fact table to belong to a different cube?
done
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/metastore/CubeMetastoreService.java
> > Lines 469 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720876#file1720876line469>
> >
> > remove commented code
done
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java
> > Lines 490 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720877#file1720877line490>
> >
> > remove commented code
done
> On May 17, 2017, 7:18 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java
> > Lines 186 (patched)
> > <https://reviews.apache.org/r/58796/diff/6/?file=1720878#file1720878line186>
> >
> > returning all facts? Instead should we say not implemented
done
- Rajitha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58796/#review175209
-----------------------------------------------------------
On May 17, 2017, 11:54 a.m., Rajitha R wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58796/
> -----------------------------------------------------------
>
> (Updated May 17, 2017, 11:54 a.m.)
>
>
> Review request for lens, Rajat Khandelwal and Puneet Gupta.
>
>
> Repository: lens
>
>
> Description
> -------
>
> CRUD changes and Unit test cases for Virtual Fact
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd 8158e6d
> lens-api/src/main/resources/lens-errors.conf 43de1e9
> lens-cube/src/main/java/org/apache/lens/cube/error/LensCubeErrorCode.java 32b9db3
> lens-cube/src/main/java/org/apache/lens/cube/metadata/AbstractCubeTable.java 67aaff8
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java 88bc1fc
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java b5c4c89
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeVirtualFactTable.java PRE-CREATION
> lens-cube/src/main/java/org/apache/lens/cube/metadata/FactTable.java PRE-CREATION
> lens-cube/src/main/java/org/apache/lens/cube/metadata/JAXBUtils.java e1e3d16
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreConstants.java 88500fd
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 40f766b
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Segmentation.java 1e3cef3
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateTableResolver.java a8d6fbd
> lens-cube/src/main/java/org/apache/lens/cube/parse/CandidateUtil.java 6ba46d6
> lens-cube/src/main/java/org/apache/lens/cube/parse/DimHQLContext.java 43510dd
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidate.java 1e54f13
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageCandidateHQLContext.java 730b802
> lens-cube/src/main/java/org/apache/lens/cube/parse/StorageTableResolver.java 291712b
> lens-cube/src/test/java/org/apache/lens/cube/metadata/CubeFactTableTest.java 42e32cb
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java a76ef13
> lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 2f300d3
> lens-cube/src/test/java/org/apache/lens/cube/parse/TestCubeRewriter.java 7d1f80d
> lens-cube/src/test/resources/schema/cubes/base/virtualcube.xml PRE-CREATION
> lens-cube/src/test/resources/schema/facts/virtualfact.xml PRE-CREATION
> lens-server-api/src/main/java/org/apache/lens/server/api/metastore/CubeMetastoreService.java 28b9d22
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 194b380
> lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java 7451aa9
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 6d32d23
>
>
> Diff: https://reviews.apache.org/r/58796/diff/7/
>
>
> Testing
> -------
>
> [INFO] ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Lens Checkstyle Rules ............................. SUCCESS [1.467s]
> [INFO] Lens .............................................. SUCCESS [3.449s]
> [INFO] Lens API .......................................... SUCCESS [23.572s]
> [INFO] Lens API for server and extensions ................ SUCCESS [20.051s]
> [INFO] Lens Cube ......................................... SUCCESS [5:41.995s]
> [INFO] Lens DB storage ................................... SUCCESS [17.013s]
> [INFO] Lens Query Library ................................ SUCCESS [13.632s]
> [INFO] Lens Hive Driver .................................. SUCCESS [1:16.131s]
> [INFO] Lens Driver for JDBC .............................. SUCCESS [48.127s]
> [INFO] Lens Elastic Search Driver ........................ SUCCESS [21.542s]
> [INFO] Lens Server ....................................... SUCCESS [16:38.220s]
> [INFO] Lens client ....................................... SUCCESS [1:25.658s]
> [INFO] Lens CLI .......................................... SUCCESS [1:16.332s]
> [INFO] Lens Examples ..................................... SUCCESS [8.432s]
> [INFO] Lens Ship Jars to Distributed Cache ............... SUCCESS [0.766s]
> [INFO] Lens Distribution ................................. SUCCESS [9.499s]
> [INFO] Lens ML Lib ....................................... SUCCESS [1:02.656s]
> [INFO] Lens ML Ext Distribution .......................... SUCCESS [2.552s]
> [INFO] Lens Regression ................................... SUCCESS [12.136s]
> [INFO] Lens UI ........................................... SUCCESS [32.999s]
> [INFO] Lens Contrib ...................................... SUCCESS [0.358s]
> [INFO] Lens Contributed Clients .......................... SUCCESS [0.360s]
> [INFO] Lens Python Client ................................ SUCCESS [0.366s]
> [INFO] ------------------------------------------------------------------------
>
>
> Thanks,
>
> Rajitha R
>
>