[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..

IMPALA-6802 (part 3): Clean up authorization tests

The third part of this patch is to rewrite the following authorization
tests:
- with
- union
- reset metadata
- show

Testing:
- Added new authorization tests
- Ran all front-end tests

Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Cherry-picks: not for 2.x
Reviewed-on: http://gerrit.cloudera.org:8080/10358
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins 
---
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
1 file changed, 396 insertions(+), 114 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5: Verified+1


--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 16 May 2018 02:50:29 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2485/


--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 15 May 2018 23:27:53 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5: Code-Review+2


--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 23:23:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5: -Code-Review

My understanding is you and Adam are still working on this. Let me know when 
this is ready to be merged.


--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 22:08:48 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#5). ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..

IMPALA-6802 (part 3): Clean up authorization tests

The third part of this patch is to rewrite the following authorization
tests:
- with
- union
- reset metadata
- show

Testing:
- Added new authorization tests
- Ran all front-end tests

Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Cherry-picks: not for 2.x
---
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
1 file changed, 396 insertions(+), 114 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10358/5
--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 5:

Alex, can you please carry +2?


--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 22:06:37 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Adam Holley (Code Review) 
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10358/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@522
PS3, Line 522: // With clause with insert.s
> typo: trailing s
Done



--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 21:33:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Adam Holley (Code Review) 
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@859
PS2, Line 859: for (AuthzTest test : new AuthzTest[]{
> The objects are just passed as a string and the code appears to be identica
Done



--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 21:29:31 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2:

(2 comments)

Thanks for the cleanup, much more condensed!

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@859
PS2, Line 859: for (AuthzTest test : new AuthzTest[]{
> This redundancy cannot be removed since the objects are different.
The objects are just passed as a string and the code appears to be identical 
otherwise, so can't we just loop over ["alltypes", "alltypes_view"] generating 
a new AuthzTest for each?


http://gerrit.cloudera.org:8080/#/c/10358/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@522
PS3, Line 522: onColumn("functional", "alltypestiny", new 
String[]{"id", "bool_col",
typo: trailing s



--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 20:54:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#3). ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..

IMPALA-6802 (part 3): Clean up authorization tests

The third part of this patch is to rewrite the following authorization
tests:
- with
- union
- reset metadata
- show

Testing:
- Added new authorization tests
- Ran all front-end tests

Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Cherry-picks: not for 2.x
---
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
1 file changed, 409 insertions(+), 98 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10358/3
--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-15 Thread Adam Holley (Code Review) 
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2:

(9 comments)

Taking over for Fredy while he's out.

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@96
PS2, Line 96: Set expectedAuthorizables = Sets.newHashSet(
> Let's factor this out somewhere, it's repeated many times
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@718
PS2, Line 718: authorize("with t as (select id from functional.alltypes) 
select * from t")
> Tests are fine, but overall there's still a lot of redundancy. Basically an
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@736
PS2, Line 736: authorize("with t as (select id, int_col, 2019 from 
functional.alltypesagg) " +
> Same here, this is basically the same as INSERT (without WITH)
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@772
PS2, Line 772: authorize("select id from functional.alltypes union all " +
> Same as SELECT with a join... also redundancy here
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@842
PS2, Line 842: for (AuthzTest test : new AuthzTest[]{
> This is an interesting pattern for handling the redundancy I mentioned abov
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@859
PS2, Line 859: for (AuthzTest test : new AuthzTest[]{
> redundancy with L842
This redundancy cannot be removed since the objects are different.


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@880
PS2, Line 880: .error(refreshError("functional"), 
onDatabase("functional", allExcept(
> also error with onServer()
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@917
PS2, Line 917: // Show partitions.
> Many of these require db or table level show metadata privs. Also add negat
Done


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1053
PS2, Line 1053: return new TPrivilegeLevel[]{TPrivilegeLevel.INSERT, 
TPrivilegeLevel.INSERT,
> INSERT listed twice, is SELECT missing?
Done



--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Tue, 15 May 2018 18:06:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-14 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@96
PS2, Line 96: Set expectedAuthorizables = Sets.newHashSet(
Let's factor this out somewhere, it's repeated many times


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@718
PS2, Line 718: authorize("with t as (select id from functional.alltypes) 
select * from t")
Tests are fine, but overall there's still a lot of redundancy. Basically any 
"read" query that produces the same privilege requests will have these same 
checks. We should think about how we can coalesce them.

We don't need to do that in this patch, but we should not forget.


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@736
PS2, Line 736: authorize("with t as (select id, int_col, 2019 from 
functional.alltypesagg) " +
Same here, this is basically the same as INSERT (without WITH)


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@772
PS2, Line 772: authorize("select id from functional.alltypes union all " +
Same as SELECT with a join... also redundancy here


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@842
PS2, Line 842: for (AuthzTest test : new AuthzTest[]{
This is an interesting pattern for handling the redundancy I mentioned above


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@859
PS2, Line 859: for (AuthzTest test : new AuthzTest[]{
redundancy with L842


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@880
PS2, Line 880: .error(refreshError("functional"), 
onDatabase("functional", allExcept(
also error with onServer()


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@917
PS2, Line 917: // Show partitions.
Many of these require db or table level show metadata privs. Also add negative 
tests that only SELECT on a column is not sufficient.


http://gerrit.cloudera.org:8080/#/c/10358/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1053
PS2, Line 1053: return new TPrivilegeLevel[]{TPrivilegeLevel.INSERT, 
TPrivilegeLevel.INSERT,
INSERT listed twice, is SELECT missing?



--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Mon, 14 May 2018 22:43:14 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-11 Thread Adam Holley (Code Review) 
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Sat, 12 May 2018 03:44:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-11 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10358/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@750
PS1, Line 750: .error(selectError("functional.alltypesagg"))
 : .error(selectError("functional.alltypesagg"), 
onServer(allExcept(
> I'm not sure this test makes sense because you can only grant SELECT at the
Done. I'll just remove all other onColumn error checks that don't make sense.



--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Fri, 11 May 2018 22:53:49 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-11 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..

IMPALA-6802 (part 3): Clean up authorization tests

The third part of this patch is to rewrite the following authorization
tests:
- with
- union
- reset metadata
- show

Testing:
- Added new authorization tests
- Ran all front-end tests

Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Cherry-picks: not for 2.x
---
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
1 file changed, 443 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10358/2
--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-11 Thread Adam Holley (Code Review) 
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10358 )

Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10358/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java:

http://gerrit.cloudera.org:8080/#/c/10358/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@750
PS1, Line 750: .error(selectError("functional.alltypes"), onColumn("functional",
 : "alltypes", "id", allExcept(TPrivilegeLevel.ALL, 
TPrivilegeLevel.SELECT)));
I'm not sure this test makes sense because you can only grant SELECT at the 
column level.  Maybe change the column name and remove "allExcept".  I know 
this pattern is already used a lot of other places, but looking at the select 
tests, I don't think it's covered that the user is granted select to other 
(possibly all) columns except one, and that it should still fail.  Maybe the 
existing tests can remain if case additional column privileges are added, but 
we still need to cover the other case.



--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Comment-Date: Fri, 11 May 2018 15:02:28 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6802 (part 3): Clean up authorization tests

2018-05-09 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/10358


Change subject: IMPALA-6802 (part 3): Clean up authorization tests
..

IMPALA-6802 (part 3): Clean up authorization tests

The third part of this patch is to rewrite the following authorization
tests:
- with
- union
- reset metadata
- show

Testing:
- Added new authorization tests
- Ran all front-end tests

Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Cherry-picks: not for 2.x
---
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java
1 file changed, 436 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10358/1
--
To view, visit http://gerrit.cloudera.org:8080/10358
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9681cc3c7094db33ab7c5caa69b99dd803b908b7
Gerrit-Change-Number: 10358
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya