You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Anne Yu <an...@cloudera.com> on 2015/05/28 06:37:58 UTC
Review Request 34756: Add new test case to validate complex view
privilege: view by union tables, view from another view,
view with subquery and view by joining tables
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34756/
-----------------------------------------------------------
Review request for sentry and Sravya Tirukkovalur.
Bugs: SENTRY-748
https://issues.apache.org/jira/browse/SENTRY-748
Repository: sentry
Description
-------
Add new test case to validate complex view privilege: view by union tables, view from another view, view with subquery and view by joining tables
Diffs
-----
sentry-tests/sentry-tests-hive/pom.xml 7ee5378fdd2af383c4dc9b7f692d60b9c1173038
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java PRE-CREATION
Diff: https://reviews.apache.org/r/34756/diff/
Testing
-------
https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests all passed, with one irrelevant error: one test takes longer time to finish so gets killed.
Thanks,
Anne Yu
Re: Review Request 34756: Add new test case to validate complex view
privilege: view by union tables, view from another view,
view with subquery and view by joining tables
Posted by Anne Yu <an...@cloudera.com>.
> On Oct. 9, 2015, 10:44 p.m., Sravya Tirukkovalur wrote:
> > Also, some of the items here are fixed with HIVE-10875, be sure to comment that we might have to unignore the tests when we upgrade the hive dependency.
>
> Anne Yu wrote:
> Will do these comments. Thanks for your review.
Will submit another patch into the upstream jira to run through unit tests. Once it passed will ping you in the jira.
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34756/#review102125
-----------------------------------------------------------
On Oct. 9, 2015, 4:06 a.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34756/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2015, 4:06 a.m.)
>
>
> Review request for sentry, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-748
> https://issues.apache.org/jira/browse/SENTRY-748
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add new test case to validate complex view privilege: view by union tables, view from another view, view with subquery and view by joining tables;
>
> Resent the cr.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/pom.xml 7ee5378fdd2af383c4dc9b7f692d60b9c1173038
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34756/diff/
>
>
> Testing
> -------
>
> https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests all passed now.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 34756: Add new test case to validate complex view
privilege: view by union tables, view from another view,
view with subquery and view by joining tables
Posted by Anne Yu <an...@cloudera.com>.
> On Oct. 9, 2015, 10:44 p.m., Sravya Tirukkovalur wrote:
> > Also, some of the items here are fixed with HIVE-10875, be sure to comment that we might have to unignore the tests when we upgrade the hive dependency.
Will do these comments. Thanks for your review.
- Anne
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34756/#review102125
-----------------------------------------------------------
On Oct. 9, 2015, 4:06 a.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34756/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2015, 4:06 a.m.)
>
>
> Review request for sentry, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-748
> https://issues.apache.org/jira/browse/SENTRY-748
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add new test case to validate complex view privilege: view by union tables, view from another view, view with subquery and view by joining tables;
>
> Resent the cr.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/pom.xml 7ee5378fdd2af383c4dc9b7f692d60b9c1173038
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34756/diff/
>
>
> Testing
> -------
>
> https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests all passed now.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 34756: Add new test case to validate complex view
privilege: view by union tables, view from another view,
view with subquery and view by joining tables
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34756/#review102125
-----------------------------------------------------------
Also, some of the items here are fixed with HIVE-10875, be sure to comment that we might have to unignore the tests when we upgrade the hive dependency.
- Sravya Tirukkovalur
On Oct. 9, 2015, 4:06 a.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34756/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2015, 4:06 a.m.)
>
>
> Review request for sentry, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-748
> https://issues.apache.org/jira/browse/SENTRY-748
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add new test case to validate complex view privilege: view by union tables, view from another view, view with subquery and view by joining tables;
>
> Resent the cr.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/pom.xml 7ee5378fdd2af383c4dc9b7f692d60b9c1173038
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34756/diff/
>
>
> Testing
> -------
>
> https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests all passed now.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 34756: Add new test case to validate complex view
privilege: view by union tables, view from another view,
view with subquery and view by joining tables
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34756/#review102115
-----------------------------------------------------------
Ship it!
Thanks for adding tests Anne! Overall looks good to me. Left minor comments.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 68)
<https://reviews.apache.org/r/34756/#comment159683>
Nit: I see that we are using execBatch just for ensuring setup is good. But we should err on the side of throwing exceptions than logging them as errors when ever possible.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 89)
<https://reviews.apache.org/r/34756/#comment159670>
Do we want to validate the rows in the resultset?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 164)
<https://reviews.apache.org/r/34756/#comment159642>
Nit: I would be a little cautious when we want to skip tests, as we never investigate skipped tests in a green run :-)
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 183)
<https://reviews.apache.org/r/34756/#comment159657>
user?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 185)
<https://reviews.apache.org/r/34756/#comment159658>
user?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 241)
<https://reviews.apache.org/r/34756/#comment159674>
Why only on unmanagedcluster?
- Sravya Tirukkovalur
On Oct. 9, 2015, 4:06 a.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34756/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2015, 4:06 a.m.)
>
>
> Review request for sentry, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-748
> https://issues.apache.org/jira/browse/SENTRY-748
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add new test case to validate complex view privilege: view by union tables, view from another view, view with subquery and view by joining tables;
>
> Resent the cr.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/pom.xml 7ee5378fdd2af383c4dc9b7f692d60b9c1173038
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/34756/diff/
>
>
> Testing
> -------
>
> https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests all passed now.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 34756: Add new test case to validate complex view
privilege: view by union tables, view from another view,
view with subquery and view by joining tables
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34756/#review103665
-----------------------------------------------------------
Ship it!
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 235)
<https://reviews.apache.org/r/34756/#comment161730>
Can you please comment on the jira that we have a test case here?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java (line 290)
<https://reviews.apache.org/r/34756/#comment161731>
Fix the jira number. 717 seems like is unrelated?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java (line 622)
<https://reviews.apache.org/r/34756/#comment161727>
Nit: Rename it to exec instead? execQuery usually corresponds to a call which returns resultset.
- Sravya Tirukkovalur
On Oct. 22, 2015, 7:43 p.m., Anne Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34756/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2015, 7:43 p.m.)
>
>
> Review request for sentry, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-748
> https://issues.apache.org/jira/browse/SENTRY-748
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Add new test case to validate complex view privilege: view by union tables, view from another view, view with subquery and view by joining tables;
>
> Resent the cr.
>
>
> Diffs
> -----
>
> sentry-tests/sentry-tests-hive/pom.xml 7744da1
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java cc5daef
>
> Diff: https://reviews.apache.org/r/34756/diff/
>
>
> Testing
> -------
>
> https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests all passed now.
>
>
> Thanks,
>
> Anne Yu
>
>
Re: Review Request 34756: Add new test case to validate complex view
privilege: view by union tables, view from another view,
view with subquery and view by joining tables
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34756/
-----------------------------------------------------------
(Updated Oct. 22, 2015, 7:43 p.m.)
Review request for sentry, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
Changes
-------
Uploaded with the most recent patch for review.
Bugs: SENTRY-748
https://issues.apache.org/jira/browse/SENTRY-748
Repository: sentry
Description
-------
Add new test case to validate complex view privilege: view by union tables, view from another view, view with subquery and view by joining tables;
Resent the cr.
Diffs (updated)
-----
sentry-tests/sentry-tests-hive/pom.xml 7744da1
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java cc5daef
Diff: https://reviews.apache.org/r/34756/diff/
Testing
-------
https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests all passed now.
Thanks,
Anne Yu
Re: Review Request 34756: Add new test case to validate complex view
privilege: view by union tables, view from another view,
view with subquery and view by joining tables
Posted by Anne Yu <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34756/
-----------------------------------------------------------
(Updated Oct. 9, 2015, 4:06 a.m.)
Review request for sentry, Li Li, Lenni Kuff, and Sravya Tirukkovalur.
Changes
-------
Sravya, just resent the cr to you.
Bugs: SENTRY-748
https://issues.apache.org/jira/browse/SENTRY-748
Repository: sentry
Description (updated)
-------
Add new test case to validate complex view privilege: view by union tables, view from another view, view with subquery and view by joining tables;
Resent the cr.
Diffs
-----
sentry-tests/sentry-tests-hive/pom.xml 7ee5378fdd2af383c4dc9b7f692d60b9c1173038
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbComplexView.java PRE-CREATION
Diff: https://reviews.apache.org/r/34756/diff/
Testing (updated)
-------
https://builds.apache.org/job/PreCommit-SENTRY-Build/608/, seems like tests all passed now.
Thanks,
Anne Yu