Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16976 )
Change subject: IMPALA-9234: Support Ranger row filtering policies ...................................................................... Patch Set 4: (11 comments) Addressed Fang-Yu's comment. PS4 and PS5 both add more tests. http://gerrit.cloudera.org:8080/#/c/16976/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java: http://gerrit.cloudera.org:8080/#/c/16976/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@56 PS1, Line 56: * Stash and deduplicate the audit events produced by table masking (Column-masking / > Thanks Quanlong! Maybe we could also add here a statement like "Refer to IM Done http://gerrit.cloudera.org:8080/#/c/16976/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java: http://gerrit.cloudera.org:8080/#/c/16976/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@56 PS3, Line 56: * Stash and deduplicate the audit events produced by table masking (Column-masking / : * Row-filtering) which are performed during the analyze phase. Called at the end of : * analyzing. These stashed events will be added back after the query pass the : * authorization phase. Note that normal events (select, insert, drop, etc.) are : * produced in the authorization phase. Stashing table masking events avoids exposing : * them when the query fails authorization. > Thanks Quanlong! Done http://gerrit.cloudera.org:8080/#/c/16976/3/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_and_row_filtering.test File testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_and_row_filtering.test: http://gerrit.cloudera.org:8080/#/c/16976/3/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_and_row_filtering.test@16 PS3, Line 16: # Column-masking policies of functional.alltypes mask "id" to "-id" and redact column : # "date_string_col". Row-filtering policy of functional.alltypes_view keeps rows with : # "id >= -8 and date_string_col = 'nn/nn/nn'" > Thanks Quanlong for adding this test case! Done http://gerrit.cloudera.org:8080/#/c/16976/3/testdata/workloads/functional-query/queries/QueryTest/ranger_row_filtering.test File testdata/workloads/functional-query/queries/QueryTest/ranger_row_filtering.test: http://gerrit.cloudera.org:8080/#/c/16976/3/testdata/workloads/functional-query/queries/QueryTest/ranger_row_filtering.test@76 PS3, Line 76: > Maybe it would be clearer to emphasize that the subquery is on the same tab Done http://gerrit.cloudera.org:8080/#/c/16976/3/testdata/workloads/functional-query/queries/QueryTest/ranger_row_filtering.test@129 PS3, Line 129: ---- RESULTS > Maybe it would be good to briefly mention here that what the query is to fe Done http://gerrit.cloudera.org:8080/#/c/16976/3/testdata/workloads/functional-query/queries/QueryTest/ranger_row_filtering.test@136 PS3, Line 136: BIGINT,INT,INT > Thanks for adding this test case! Done http://gerrit.cloudera.org:8080/#/c/16976/3/testdata/workloads/functional-query/queries/QueryTest/ranger_row_filtering.test@146 PS3, Line 146: 8,-1 > I was wondering if it would be good to add a test case involving 2 tables e Sure, done. I reused the existing filters and added the tests at line 23 in PS5. http://gerrit.cloudera.org:8080/#/c/16976/3/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/16976/3/tests/authorization/test_ranger.py@1095 PS3, Line 1095: policy_names.append(policy_name) > It may be good to add a comment here to point out that we do not need to > additionally grant the SELECT privileges to 'getuser()' because 'getuser()' > is the owner of the databases used in the test queries. Done > A user cannot access some rows in a table if there is some row-filtering > policy applied to the table even though this user is the owner of the table. > Is my understanding correct? Yeah, this is the behavior by designed. http://gerrit.cloudera.org:8080/#/c/16976/3/tests/authorization/test_ranger.py@1104 PS3, Line 1104: e_policy(policy > I am not very familiar with the creation of a row-filtering policy via REST No, in PS1 I use string = '0' and it's ok. This is just for making sure function calls can be used in the filter. I'll add a comment for it. http://gerrit.cloudera.org:8080/#/c/16976/4/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/16976/4/tests/authorization/test_ranger.py@1124 PS4, Line 1124: # > flake8: E266 too many leading '#' for block comment Done http://gerrit.cloudera.org:8080/#/c/16976/4/tests/authorization/test_ranger.py@1169 PS4, Line 1169: # > flake8: E266 too many leading '#' for block comment Done -- To view, visit http://gerrit.cloudera.org:8080/16976 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I580517be241225ca15e45686381b78890178d7cc Gerrit-Change-Number: 16976 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 01 Mar 2021 05:52:28 +0000 Gerrit-HasComments: Yes
