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 10: (4 comments) Thank Aman! http://gerrit.cloudera.org:8080/#/c/16976/9/be/src/util/backend-gflag-util.cc File be/src/util/backend-gflag-util.cc: http://gerrit.cloudera.org:8080/#/c/16976/9/be/src/util/backend-gflag-util.cc@158 PS9, Line 158: DEFINE_bool(enable_row_filtering, true, > Since enabling this flag depends on enabling column masking, it would be us Done http://gerrit.cloudera.org:8080/#/c/16976/9/fe/src/main/java/org/apache/impala/authorization/TableMask.java File fe/src/main/java/org/apache/impala/authorization/TableMask.java: http://gerrit.cloudera.org:8080/#/c/16976/9/fe/src/main/java/org/apache/impala/authorization/TableMask.java@109 PS9, Line 109: String stmtSql = String.format("SELECT * FROM %s.%s WHERE %s", > My understanding is the SELECT * will need to be changed to only project co I think you mean IMPALA-9223. It's commented in InlineViewRef.createTableMaskView() where we actually create the table mask view. This query is just for parsing the row filter string into AST. Note that only the WHERE clause is returned. So "SELECT *" is used for simplicity. http://gerrit.cloudera.org:8080/#/c/16976/9/fe/src/main/java/org/apache/impala/authorization/TableMask.java@111 PS9, Line 111: SelectStmt selectStmt = (SelectStmt) Parser.parse(stmtSql); > Since this will throw an AnalysisException (or ParseException) for a malfor Yes, good question. For example if table functional.alltypestiny has an illegal row filter like "10 id = int_col", query on it will fail as [localhost:21050] default> select * from functional.alltypestiny; Query: select * from functional.alltypestiny Query submitted at: 2021-03-17 14:46:27 (Coordinator: http://quanlong-OptiPlex-BJ:25000) ERROR: ParseException: Syntax error in line 1: SELECT * FROM functional.alltypestiny WHERE 10 id = int_col ^ Encountered: IDENTIFIER Expected: AND, BETWEEN, DIV, EXCEPT, GROUP, HAVING, ILIKE, IN, INTERSECT, IREGEXP, IS, LIKE, LIMIT, ||, MINUS, NOT, OFFSET, OR, ORDER, REGEXP, RLIKE, UNION CAUSED BY: Exception: Syntax error I tried it in Hive as well, the failure is similar: 0: jdbc:hive2://localhost:11050> select * from functional.alltypestiny; Error: Error while compiling statement: FAILED: SemanticException org.apache.hadoop.hive.ql.parse.ParseException: line 1:484 missing ) at 'id' near 'id' line 1:487 missing EOF at '=' near 'id' (state=42000,code=40000) We will expose more details than Hive since we have a more friendly error message. But it can help user to correct the error. I'm not sure whether we should swallow the details. I think it's undefined on the error behavior for illegal row filter. Do you think we should file a JIRA for both Hive and Impala? http://gerrit.cloudera.org:8080/#/c/16976/9/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/9/testdata/workloads/functional-query/queries/QueryTest/ranger_row_filtering.test@1 PS9, Line 1: ==== > Thanks for adding these tests. I had a couple of suggestions to the test c Good questions! 1. The test at line 67 is a related test. Currently we don't have a formatted error message. The raw cause is exposed. 2. Done for WITH clause. Borrowing some tests in ranger_column_masking.test. For the VIEWs, the tests on alltypes_view and masked_view are related. Should I add more? 3. Currently both Hive and Impala will show the real plan containing the masking expressions. So Impala runtime profile will has them as well. I think it's ok to have them, which helps the end user to understand the query results. -- 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: 10 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 17 Mar 2021 08:08:25 +0000 Gerrit-HasComments: Yes
