[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Reviewed-on: http://gerrit.cloudera.org:8080/11721
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 260 insertions(+), 13 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 11: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 20 Oct 2018 00:19:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 10:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1114/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:53:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 11: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:26:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 11:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3336/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:26:48 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 10: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:21:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1113/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:21:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#10). ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 260 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/11721/10
--
To view, visit http://gerrit.cloudera.org:8080/11721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11721/8/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/11721/8/be/src/catalog/catalog-util.cc@227
PS8, Line 227:
> lets use StringParser::StringToInt here (util/string-parser.h)
Ah I didn't know about that function. Done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:20:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11721/8/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/11721/8/be/src/catalog/catalog-util.cc@227
PS8, Line 227: stoi(principal_id))
lets use StringParser::StringToInt here (util/string-parser.h)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:02:48 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#8). ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 257 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/11721/8
--
To view, visit http://gerrit.cloudera.org:8080/11721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 8:

Fixed clang-tidy.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:00:13 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 7:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/1110/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 19:48:05 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#7). ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 257 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/11721/7
--
To view, visit http://gerrit.cloudera.org:8080/11721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 7:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util-test.cc
File be/src/catalog/catalog-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util-test.cc@122
PS6, Line 122:   TPrivilege privilege;
> add more edge cases for these negative tests. for example, empty string, un
Done. More tests added. The string comes from: 
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java#L56-L127


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@210
PS6, Line 210: principal
> principal
Done


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@210
PS6, Line 210: principal
> principal
Done


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@226
PS6, Line 226:
> what does this do for a malformed id?
atoi usally returns 0 (not guaranteed) for a malformed ID, which is pretty bad: 
https://stackoverflow.com/questions/8871711/atoi-how-to-identify-the-difference-between-zero-and-error.
 I changed it to use std::stoi instead.


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@227
PS6, Line 227: 
catalog_object->privilege.__set_principal_id(stoi(principal_id));
> guaranteed to be uppercase?
Yeah we control this: 
https://gerrit.cloudera.org/c/11721/6/fe/src/main/java/org/apache/impala/catalog/Catalog.java#570


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@248
PS6, Line 248: e;
> dcheck not null for this. its a publicly exposed method (for test only?)
It's for exposed for test only. DCHECK added.


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@250
PS6, Line 250:   }
> add a comment about the expected format and an example.
Done


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@253
PS6, Line 253:
 : Status TPrivilegeFro
> check the len... valid?
Oops. Forgot to add the check. Done.


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@254
PS6, Line 254: ectName(
> all guaranteed to be lowercase?
Yeah, we control this: 
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java#L56-L127



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 19:13:07 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 6:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util-test.cc
File be/src/catalog/catalog-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util-test.cc@122
PS6, Line 122:   TPrivilege privilege;
add more edge cases for these negative tests. for example, empty string, 
unexpected lengths, mixed case. as it stands, I don't trust that string 
(where's it come from?)


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@210
PS6, Line 210: privilege
principal


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@210
PS6, Line 210: privilege
principal


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@226
PS6, Line 226: atoi(principal_id.c_str())
what does this do for a malformed id?


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@227
PS6, Line 227:   if (principal_type == "ROLE") {
guaranteed to be uppercase?


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@248
PS6, Line 248: TPrivilege* privilege
dcheck not null for this. its a publicly exposed method (for test only?)


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@250
PS6, Line 250:   boost::algorithm::split_regex(split, object_name, 
boost::regex("->"));
add a comment about the expected format and an example.


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@253
PS6, Line 253: key_value, s, [](char c){ return c == '='; });
 : if (key_value[0]
check the len... valid?


http://gerrit.cloudera.org:8080/#/c/11721/6/be/src/catalog/catalog-util.cc@254
PS6, Line 254: "server"
all guaranteed to be lowercase?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 18:38:25 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-19 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 6: Code-Review+1

Carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 06:50:19 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 6:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1100/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 00:30:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 6:

Fixed clang-tidy error.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 19 Oct 2018 00:02:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#6). ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 232 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/11721/6
--
To view, visit http://gerrit.cloudera.org:8080/11721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 5:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/1098/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 18 Oct 2018 23:52:26 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 232 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11721/4/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/11721/4/be/src/catalog/catalog-util.cc@a214
PS4, Line 214:
> what is the severity of the issue? ... priv doesn't get shown, crash, somet
privilege doesn't get shown.


http://gerrit.cloudera.org:8080/#/c/11721/4/be/src/catalog/catalog-util.cc@271
PS4, Line 271:   Status status = 
TPrivilegeLevelFromObjectName(key_value[1], _level);
> is there a comparable parsing method on the java side (frontend)? if so, wo
There is for building the privilege name and not parsing it.


http://gerrit.cloudera.org:8080/#/c/11721/4/be/src/catalog/catalog-util.cc@274
PS4, Line 274: } else if (key_value[0] == "grantoption") {
> const
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 18 Oct 2018 23:22:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Vuk Ercegovac (Code Review)
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11721/4/be/src/catalog/catalog-util.cc
File be/src/catalog/catalog-util.cc:

http://gerrit.cloudera.org:8080/#/c/11721/4/be/src/catalog/catalog-util.cc@a214
PS4, Line 214:
what is the severity of the issue? ... priv doesn't get shown, crash, something 
else?


http://gerrit.cloudera.org:8080/#/c/11721/4/be/src/catalog/catalog-util.cc@271
PS4, Line 271: Status TPrivilegeFromObjectName(const string& object_name, 
TPrivilege* privilege) {
is there a comparable parsing method on the java side (frontend)? if so, would 
be worthwhile for this to avoid duplication.


http://gerrit.cloudera.org:8080/#/c/11721/4/be/src/catalog/catalog-util.cc@274
PS4, Line 274:   for (auto& s: split) {
const



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 18 Oct 2018 18:40:13 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 18 Oct 2018 18:15:45 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 229 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/11721/4
--
To view, visit http://gerrit.cloudera.org:8080/11721
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1090/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 18 Oct 2018 16:09:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled. This patch fixes the issue and makes the
/catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 232 insertions(+), 13 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 3: Code-Review+1

(3 comments)

Carry Csaba's +1.

http://gerrit.cloudera.org:8080/#/c/11721/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11721/2//COMMIT_MSG@12
PS2, Line 12: his pa
> Typo?
Oops. Done.


http://gerrit.cloudera.org:8080/#/c/11721/2/be/src/catalog/catalog-util.h
File be/src/catalog/catalog-util.h:

http://gerrit.cloudera.org:8080/#/c/11721/2/be/src/catalog/catalog-util.h@104
PS2, Line 104: /// Populates a TPrivilegeLevel::type based on the given object 
name string.
 : Status TPrivilegeLevelFromObjectName(const std::string& 
object_name,
 : TPrivilegeLevel::type* privilege_level);
> Does this have to be in the header? It is only used in catalog-util.cc
It's also used in catalog-util-test.cc.


http://gerrit.cloudera.org:8080/#/c/11721/2/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:

http://gerrit.cloudera.org:8080/#/c/11721/2/tests/authorization/test_authorization.py@468
PS2, Line 468: assert "catalog_version" in obj_dump
> nit: this does not look as descriptive as 'assert "CatalogException" in obj
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 18 Oct 2018 15:36:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11721 )

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 2: Code-Review+1

(3 comments)

Just a few minor issues, lgtm otherwise.

http://gerrit.cloudera.org:8080/#/c/11721/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11721/2//COMMIT_MSG@12
PS2, Line 12: types.
Typo?


http://gerrit.cloudera.org:8080/#/c/11721/2/be/src/catalog/catalog-util.h
File be/src/catalog/catalog-util.h:

http://gerrit.cloudera.org:8080/#/c/11721/2/be/src/catalog/catalog-util.h@104
PS2, Line 104: /// Populates a TPrivilegeLevel::type based on the given object 
name string.
 : Status TPrivilegeLevelFromObjectName(const std::string& 
object_name,
 : TPrivilegeLevel::type* privilege_level);
Does this have to be in the header? It is only used in catalog-util.cc


http://gerrit.cloudera.org:8080/#/c/11721/2/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:

http://gerrit.cloudera.org:8080/#/c/11721/2/tests/authorization/test_authorization.py@468
PS2, Line 468: service.extract_catalog_object_version(obj_dump)
nit: this does not look as descriptive as 'assert "CatalogException" in 
obj_dump' - maybe 'assert "CatalogException" not in obj_dump' or 'assert 
"catalog_version" in obj_dump' would clearer.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 18 Oct 2018 12:45:02 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

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

Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..


Patch Set 2:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/1085/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 18 Oct 2018 06:51:56 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7721: Fix broken /catalog object web API when getting a privilege

2018-10-18 Thread Fredy Wijaya (Code Review)
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/11721


Change subject: IMPALA-7721: Fix broken /catalog_object web API when getting a 
privilege
..

IMPALA-7721: Fix broken /catalog_object web API when getting a privilege

Before this patch, /catalog_object web API was broken when getting a
privilege due to an incorrect way of getting a role ID. IMPALA-7616
broke this even more due to a lack of test coverage in /catalog_object
when authorization is enabled.  types. This patch fixes the issue and
makes the /catalog_object web API usable again for getting a privilege.

Testing:
- Added a new BE test
- Added a new E2E test
- Ran all E2E authorization tests

Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
---
M be/src/catalog/catalog-util-test.cc
M be/src/catalog/catalog-util.cc
M be/src/catalog/catalog-util.h
M fe/src/main/java/org/apache/impala/catalog/Catalog.java
M fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
M tests/authorization/test_authorization.py
6 files changed, 232 insertions(+), 13 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I525149d113a1437c1e1493ad3c25a755e370b0c7
Gerrit-Change-Number: 11721
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya