[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking resolvePathWithMasking() is a wrapper on resolvePath() to further resolve nested columns inside the table masking view. When it was added, complex types in the select list hadn't been supported yet. So the table masking view can't expose complex type columns directly in the select list. Any paths in nested types will be further resolved inside the table masking view in resolvePathWithMasking(). Take the following query as an example: select id, nested_struct.* from complextypestbl; If Ranger column-masking/row-filter policies applied on the table, the query is rewritten as select id, nested_struct.* from ( select mask(id) from complextypestbl where row-filtering-condition ) t; Table masking view "t" can't expose the nested column "nested_struct". So we further resolve "nested_struct" inside the inlineView to use the masked table "complextypestbl". The underlying TableRef is expected to be a BaseTableRef. Paths that don't reference nested columns should be resolved and returned directly (just like the original resolvePath() does). E.g. select v.* from masked_view v is rewritten to select v.* from ( select mask(c1), mask(c2), ..., mask(cn) from masked_view where row-filtering-condition ) v; The STAR path "v.*" should be resolved directly. However, it's treated as a nested column unexpectedly. The code then tries to resolve it inside the table "masked_view" and found "masked_view" is not a table so throws the IllegalStateException. These are the current conditions for identifying nested STAR paths: - The destType is STRUCT - And the resolved path is rooted at a valid tuple descriptor They don't really recognize the nested struct columns because STAR paths on table/view also match these conditions. When the STAR path is an expansion on a catalog table/view, the root tuple descriptor is exactly the output tuple of the table/view. The destType is the type of the tuple descriptor which is always a StructType. Note that STAR paths on other nested types, i.e. array/map, are invalid. So the first condition matches for all valid cases. The second condition also matches all valid cases since both the table/view and struct STAR expansion have the path rooted at a valid tuple descriptor. This patch fixes the check for nested struct STAR path by checking the matched types instead. Note that if "v.*" is a table/view expansion, the matched type list is empty. If "v.*" is a struct column expansion, the matched type list contains the STRUCT column type. Tests: - Add missing coverage on STAR paths (v.*) on masked views. Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Reviewed-on: http://gerrit.cloudera.org:8080/19429 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking.test M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test 4 files changed, 166 insertions(+), 5 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 10 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 9 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 01 Feb 2023 04:23:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9011/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 9 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 23:13:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 9 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 23:13:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 8: > Patch Set 8: Verified-1 > > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9007/ The failure is due to IMPALA-11572: https://jenkins.impala.io/job/ubuntu-16.04-dockerised-tests/6947 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 23:12:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 8: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9007/ -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 15:19:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9007/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 10:15:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 10:15:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 10:09:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12274/ : 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/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 10:02:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/19429/6/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test File testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test: http://gerrit.cloudera.org:8080/#/c/19429/6/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test@63 PS6, Line 63: # Test resolving nested columns in expanding star expression. > Can you add a test with EXPAND_COMPLEX_TYPES=1? It should return an analysi Done. That would be a good coverage! -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 09:43:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Hello Fang-Yu Rao, Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19429 to look at the new patch set (#7). Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking resolvePathWithMasking() is a wrapper on resolvePath() to further resolve nested columns inside the table masking view. When it was added, complex types in the select list hadn't been supported yet. So the table masking view can't expose complex type columns directly in the select list. Any paths in nested types will be further resolved inside the table masking view in resolvePathWithMasking(). Take the following query as an example: select id, nested_struct.* from complextypestbl; If Ranger column-masking/row-filter policies applied on the table, the query is rewritten as select id, nested_struct.* from ( select mask(id) from complextypestbl where row-filtering-condition ) t; Table masking view "t" can't expose the nested column "nested_struct". So we further resolve "nested_struct" inside the inlineView to use the masked table "complextypestbl". The underlying TableRef is expected to be a BaseTableRef. Paths that don't reference nested columns should be resolved and returned directly (just like the original resolvePath() does). E.g. select v.* from masked_view v is rewritten to select v.* from ( select mask(c1), mask(c2), ..., mask(cn) from masked_view where row-filtering-condition ) v; The STAR path "v.*" should be resolved directly. However, it's treated as a nested column unexpectedly. The code then tries to resolve it inside the table "masked_view" and found "masked_view" is not a table so throws the IllegalStateException. These are the current conditions for identifying nested STAR paths: - The destType is STRUCT - And the resolved path is rooted at a valid tuple descriptor They don't really recognize the nested struct columns because STAR paths on table/view also match these conditions. When the STAR path is an expansion on a catalog table/view, the root tuple descriptor is exactly the output tuple of the table/view. The destType is the type of the tuple descriptor which is always a StructType. Note that STAR paths on other nested types, i.e. array/map, are invalid. So the first condition matches for all valid cases. The second condition also matches all valid cases since both the table/view and struct STAR expansion have the path rooted at a valid tuple descriptor. This patch fixes the check for nested struct STAR path by checking the matched types instead. Note that if "v.*" is a table/view expansion, the matched type list is empty. If "v.*" is a struct column expansion, the matched type list contains the STRUCT column type. Tests: - Add missing coverage on STAR paths (v.*) on masked views. Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking.test M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test 4 files changed, 166 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19429/7 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 6: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/19429/6/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test File testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test: http://gerrit.cloudera.org:8080/#/c/19429/6/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test@63 PS6, Line 63: # Test resolving nested columns in expanding star expression. Can you add a test with EXPAND_COMPLEX_TYPES=1? It should return an analysis for now, but it is something that should work with column mask on ID once https://gerrit.cloudera.org/#/c/19322/ is merged -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 31 Jan 2023 09:27:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 30 Jan 2023 14:25:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 6: Code-Review+1 Thanks for adding those 4 test queries in ranger_column_masking_complex_types.test to characterize Impala's current behavior! I do not have any additional suggestion. -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 30 Jan 2023 05:42:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 28 Jan 2023 16:33:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8994/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 28 Jan 2023 11:25:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12252/ : 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/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 28 Jan 2023 08:47:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 6: (3 comments) Thanks for the comments! Added two more tests. Also found another issue of column masking on full ACID table: IMPALA-11870. Since it's a rare use case and unrelated to this, we can fix it in follow-on patches. http://gerrit.cloudera.org:8080/#/c/19429/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/5//COMMIT_MSG@39 PS5, Line 39: > nit: "as a" should be put in the next line. Done http://gerrit.cloudera.org:8080/#/c/19429/5//COMMIT_MSG@49 PS5, Line 49: path > This is the one I said should be 'path', the previous one should remain 'be Done http://gerrit.cloudera.org:8080/#/c/19429/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/19429/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1196 PS5, Line 1196: resolvedPath.getMatchedTypes( > We could also add a preconditions check below that the matched types includ Yes, added the code comment and precondition check. -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Sat, 28 Jan 2023 08:30:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Hello Fang-Yu Rao, Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19429 to look at the new patch set (#6). Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking resolvePathWithMasking() is a wrapper on resolvePath() to further resolve nested columns inside the table masking view. When it was added, complex types in the select list hadn't been supported yet. So the table masking view can't expose complex type columns directly in the select list. Any paths in nested types will be further resolved inside the table masking view in resolvePathWithMasking(). Take the following query as an example: select id, nested_struct.* from complextypestbl; If Ranger column-masking/row-filter policies applied on the table, the query is rewritten as select id, nested_struct.* from ( select mask(id) from complextypestbl where row-filtering-condition ) t; Table masking view "t" can't expose the nested column "nested_struct". So we further resolve "nested_struct" inside the inlineView to use the masked table "complextypestbl". The underlying TableRef is expected to be a BaseTableRef. Paths that don't reference nested columns should be resolved and returned directly (just like the original resolvePath() does). E.g. select v.* from masked_view v is rewritten to select v.* from ( select mask(c1), mask(c2), ..., mask(cn) from masked_view where row-filtering-condition ) v; The STAR path "v.*" should be resolved directly. However, it's treated as a nested column unexpectedly. The code then tries to resolve it inside the table "masked_view" and found "masked_view" is not a table so throws the IllegalStateException. These are the current conditions for identifying nested STAR paths: - The destType is STRUCT - And the resolved path is rooted at a valid tuple descriptor They don't really recognize the nested struct columns because STAR paths on table/view also match these conditions. When the STAR path is an expansion on a catalog table/view, the root tuple descriptor is exactly the output tuple of the table/view. The destType is the type of the tuple descriptor which is always a StructType. Note that STAR paths on other nested types, i.e. array/map, are invalid. So the first condition matches for all valid cases. The second condition also matches all valid cases since both the table/view and struct STAR expansion have the path rooted at a valid tuple descriptor. This patch fixes the check for nested struct STAR path by checking the matched types instead. Note that if "v.*" is a table/view expansion, the matched type list is empty. If "v.*" is a struct column expansion, the matched type list contains the STRUCT column type. Tests: - Add missing coverage on STAR paths (v.*) on masked views. Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking.test M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test 4 files changed, 159 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19429/6 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/19429/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/19429/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1196 PS5, Line 1196: resolvedPath.getMatchedTypes( > Is it true that when 'pathType' is PathType.STAR and resolvedPath.getMatche We could also add a preconditions check below that the matched types include a struct if we don't return here. -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 23 Jan 2023 09:55:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 5: (2 comments) Thanks for preparing a fix so quickly Quanlong! I only have 2 very minor comments and the second one regarding adding a code comment is only optional. I raised that question because I do not know enough about how paths are resolved in Impala's FE in detail. http://gerrit.cloudera.org:8080/#/c/19429/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/5//COMMIT_MSG@39 PS5, Line 39: as a nit: "as a" should be put in the next line. http://gerrit.cloudera.org:8080/#/c/19429/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/19429/5/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@1196 PS5, Line 1196: resolvedPath.getMatchedTypes( Is it true that when 'pathType' is PathType.STAR and resolvedPath.getMatchedTypes() is not empty, 'resolvedPath' has to correspond to a struct column, e.g., the column of 'nested_struct' of the table 'complextypestbl' under the database 'functional_parquet'? If so, it may be good to add a more detailed code comment at the definition of 'matchedTypes_' in Path.java as well. -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Mon, 23 Jan 2023 05:05:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/19429/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/5//COMMIT_MSG@49 PS5, Line 49: paths This is the one I said should be 'path', the previous one should remain 'because STAR paths'. -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 20 Jan 2023 11:47:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 5: (3 comments) Good to know that it's clear now. Thanks for the feedbacks! http://gerrit.cloudera.org:8080/#/c/19429/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/4//COMMIT_MSG@49 PS4, Line 49: paths > Nit: path Done http://gerrit.cloudera.org:8080/#/c/19429/4//COMMIT_MSG@50 PS4, Line 50: root t > Shouldn't it be 'root tuple descriptor'? Done http://gerrit.cloudera.org:8080/#/c/19429/4//COMMIT_MSG@56 PS4, Line 56: an > Nit: and Done -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 20 Jan 2023 11:12:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Hello Fang-Yu Rao, Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19429 to look at the new patch set (#5). Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking resolvePathWithMasking() is a wrapper on resolvePath() to further resolve nested columns inside the table masking view. When it was added, complex types in the select list hadn't been supported yet. So the table masking view can't expose complex type columns directly in the select list. Any paths in nested types will be further resolved inside the table masking view in resolvePathWithMasking(). Take the following query as an example: select id, nested_struct.* from complextypestbl; If Ranger column-masking/row-filter policies applied on the table, the query is rewritten as select id, nested_struct.* from ( select mask(id) from complextypestbl where row-filtering-condition ) t; Table masking view "t" can't expose the nested column "nested_struct". So we further resolve "nested_struct" inside the inlineView to use the masked table "complextypestbl". The underlying TableRef is expected to be a BaseTableRef. Paths that don't reference nested columns should be resolved and returned directly (just like the original resolvePath() does). E.g. select v.* from masked_view v is rewritten to select v.* from ( select mask(c1), mask(c2), ..., mask(cn) from masked_view where row-filtering-condition ) v; The STAR path "v.*" should be resolved directly. However, it's treated as a nested column unexpectedly. The code then tries to resolve it inside the table "masked_view" and found "masked_view" is not a table so throws the IllegalStateException. These are the current conditions for identifying nested STAR paths: - The destType is STRUCT - And the resolved path is rooted at a valid tuple descriptor They don't really recognize the nested struct columns because STAR path on table/view also match these conditions. When the STAR paths is an expansion on a catalog table/view, the root tuple descriptor is exactly the output tuple of the table/view. The destType is the type of the tuple descriptor which is always a StructType. Note that STAR paths on other nested types, i.e. array/map, are invalid. So the first condition matches for all valid cases. The second condition also matches all valid cases since both the table/view and struct STAR expansion have the path rooted at a valid tuple descriptor. This patch fixes the check for nested struct STAR path by checking the matched types instead. Note that if "v.*" is a table/view expansion, the matched type list is empty. If "v.*" is a struct column expansion, the matched type list contains the STRUCT column type. Tests: - Add missing coverage on STAR paths (v.*) on masked views. Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking.test M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test 4 files changed, 67 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19429/5 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 4: Code-Review+1 (3 comments) Thanks, the commit message is much clearer now. Just a few nits, otherwise it looks good. http://gerrit.cloudera.org:8080/#/c/19429/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/4//COMMIT_MSG@49 PS4, Line 49: paths Nit: path http://gerrit.cloudera.org:8080/#/c/19429/4//COMMIT_MSG@50 PS4, Line 50: rooted Shouldn't it be 'root tuple descriptor'? http://gerrit.cloudera.org:8080/#/c/19429/4//COMMIT_MSG@56 PS4, Line 56: or Nit: and -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 20 Jan 2023 08:38:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12209/ : 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/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 20 Jan 2023 02:53:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/19429/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/3//COMMIT_MSG@40 PS3, Line 40: tri > Nit: tries Done http://gerrit.cloudera.org:8080/#/c/19429/3//COMMIT_MSG@50 PS3, Line 50: rooted tuple d > Could you explain why the first condition is true for tables/views? Is it b Yeah, needs the second point to better explain this. When it's a table/view STAR expansion, destType() of the STAR path is the type of the rooted tuple descriptor: https://github.com/apache/impala/blob/9baf790606073d88c3a2fd431110812140df0cb7/fe/src/main/java/org/apache/impala/analysis/Path.java#L354 The type of a tuple descriptor is always a StructType: https://github.com/apache/impala/blob/9baf790606073d88c3a2fd431110812140df0cb7/fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java#L88 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 20 Jan 2023 02:32:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Hello Fang-Yu Rao, Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19429 to look at the new patch set (#4). Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking resolvePathWithMasking() is a wrapper on resolvePath() to further resolve nested columns inside the table masking view. When it was added, complex types in the select list hadn't been supported yet. So the table masking view can't expose complex type columns directly in the select list. Any paths in nested types will be further resolved inside the table masking view in resolvePathWithMasking(). Take the following query as an example: select id, nested_struct.* from complextypestbl; If Ranger column-masking/row-filter policies applied on the table, the query is rewritten as select id, nested_struct.* from ( select mask(id) from complextypestbl where row-filtering-condition ) t; Table masking view "t" can't expose the nested column "nested_struct". So we further resolve "nested_struct" inside the inlineView to use the masked table "complextypestbl". The underlying TableRef is expected to be a BaseTableRef. Paths that don't reference nested columns should be resolved and returned directly (just like the original resolvePath() does). E.g. select v.* from masked_view v is rewritten to select v.* from ( select mask(c1), mask(c2), ..., mask(cn) from masked_view where row-filtering-condition ) v; The STAR path "v.*" should be resolved directly. However, it's treated as a nested column unexpectedly. The code then tries to resolve it inside the table "masked_view" and found "masked_view" is not a table so throws the IllegalStateException. These are the current conditions for identifying nested STAR paths: - The destType is STRUCT - And the resolved path is rooted at a valid tuple descriptor They don't really recognize the nested struct columns because STAR paths on table/view also match these conditions. When the STAR paths is an expansion on a catalog table/view, the rooted tuple descriptor is exactly the output tuple of the table/view. The destType is the type of the tuple descriptor which is always a StructType. Note that STAR paths on other nested types, i.e. array/map, are invalid. So the first condition matches for all valid cases. The second condition also matches all valid cases since both the table/view or struct STAR expansion have the path rooted at a valid tuple descriptor. This patch fixes the check for nested struct STAR path by checking the matched types instead. Note that if "v.*" is a table/view expansion, the matched type list is empty. If "v.*" is a struct column expansion, the matched type list contains the STRUCT column type. Tests: - Add missing coverage on STAR paths (v.*) on masked views. Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking.test M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test 4 files changed, 67 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19429/4 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/19429/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/3//COMMIT_MSG@40 PS3, Line 40: try Nit: tries http://gerrit.cloudera.org:8080/#/c/19429/3//COMMIT_MSG@50 PS3, Line 50: always matches Could you explain why the first condition is true for tables/views? Is it because their types are also represented as structs? -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 19 Jan 2023 14:12:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/19429/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/2//COMMIT_MSG@41 PS2, Line 41: it's not a table > Shouldn't this be "it's not a struct"? I think I should reword it to: "masked_view" is not a table Note that the underlying TableRef is expected to be a BaseTableRef. http://gerrit.cloudera.org:8080/#/c/19429/2//COMMIT_MSG@44 PS2, Line 44: These are the conditions for returning the STAR paths directly: > After looking at the code again, aren't the conditions for returning the pa Ah, my bad! I said it in the reverse way.. It's the condition for nested struct path. -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 19 Jan 2023 13:08:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Hello Fang-Yu Rao, Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19429 to look at the new patch set (#3). Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking resolvePathWithMasking() is a wrapper on resolvePath() to further resolve nested columns inside the table masking view. When it was added, complex types in the select list hadn't been supported yet. So the table masking view can't expose complex type columns directly in the select list. Any paths in nested types will be further resolved inside the table masking view in resolvePathWithMasking(). Take the following query as an example: select id, nested_struct.* from complextypestbl; If Ranger column-masking/row-filter policies applied on the table, the query is rewritten as select id, nested_struct.* from ( select mask(id) from complextypestbl where row-filtering-condition ) t; Table masking view "t" can't expose the nested column "nested_struct". So we further resolve "nested_struct" inside the inlineView to use the masked table "complextypestbl". The underlying TableRef is expected to be a BaseTableRef. Paths that don't reference nested columns should be resolved and returned directly (just like the original resolvePath() does). E.g. select v.* from masked_view v is rewritten to select v.* from ( select mask(c1), mask(c2), ..., mask(cn) from masked_view where row-filtering-condition ) v; The STAR path "v.*" should be resolved directly. However, it's treated as a nested column unexpectedly. The code then try to resolve it inside the table "masked_view" and found "masked_view" is not a table so throws the IllegalStateException. These are the conditions for nested STAR paths: - The type is STRUCT - And the resolved path is rooted at a valid tuple descriptor They don't really recognize the nested columns. In fact, all STAR paths match these conditions. Reason: - STAR expansion is only valid for paths to a struct type (or a table/view). So the first condition always matches. - The second condition also matches for STAR paths on table/view, i.e. paths of "v.*" when "v" is a catalog table/view. The rooted tuple descriptor is exactly the output tuple of the table/view. This patch fixes the check for nested struct STAR path by checking the matched types instead. Note that if "v.*" is a table/view expansion, the matched type list is empty. If "v.*" is a struct column expansion, the matched type list contains the STRUCT column type. Tests: - Add missing coverage on STAR paths (v.*) on masked views. Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking.test M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test 4 files changed, 67 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19429/3 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Daniel Becker has posted comments on this change. (
http://gerrit.cloudera.org:8080/19429 )
Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in
resolvePathWithMasking
..
Patch Set 2:
(3 comments)
Thanks
http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@12
PS1, Line 12: the table masking view can't expose complex type columns directly
> Yeah, I filed IMPALA-11847 for the refactor since it's a broader change. It
Great, thanks.
http://gerrit.cloudera.org:8080/#/c/19429/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19429/2//COMMIT_MSG@41
PS2, Line 41: it's not a table
Shouldn't this be "it's not a struct"?
http://gerrit.cloudera.org:8080/#/c/19429/2//COMMIT_MSG@44
PS2, Line 44: These are the conditions for returning the STAR paths directly:
After looking at the code again, aren't the conditions for returning the path
directly that at least one of the mentioned statements is false?
if (!resolvedPath.destType().isStructType() || !resolvedPath.isRootedAtTuple())
{
return resolvedPath;
}
Shouldn't we say
- the type is not struct OR
- the resolved path is not rooted at a valid tuple descriptor
?
--
To view, visit http://gerrit.cloudera.org:8080/19429
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77
Gerrit-Change-Number: 19429
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Daniel Becker
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Thu, 19 Jan 2023 10:16:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12200/ : 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/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 19 Jan 2023 06:20:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 2: (7 comments) Thanks for Daniel's feedback! Adjusted the commit message. While revisiting the Precondition, I found another bug: IMPALA-11851. It happens when the catalog view exposes complex types and need to apply table masking policies. Since it's a bug in branches that support complex types in SelectList. I'll fix it in another patch. http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@7 PS1, Line 7: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking > Do I understand it correctly that the error occurs if we try to query a sta Yeah, the error is due to "v.*" always being treated as a star on a struct. http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@10 PS1, Line 10: When > Nit: When Done http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@11 PS1, Line 11: hadn' > Nit: hadn't Done http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@12 PS1, Line 12: the table masking view can't expose complex type columns directly > I see you've already created IMPALA-11847 for this. Yeah, I filed IMPALA-11847 for the refactor since it's a broader change. It also depends on the full functionality of complex type support in select list which hasn't finished yet, e.g. some remaining items: IMPALA-9551, IMPALA-10851, IMPALA-11052. This patch is a fix for the current solution and it will be backported to older branches. http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@32 PS1, Line 32: is rewritten to > Could you make it clearer that these are the conditions for returning the p Done http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@33 PS1, Line 33: > Nit: rooted? Done http://gerrit.cloudera.org:8080/#/c/19429/1/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test File testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test: http://gerrit.cloudera.org:8080/#/c/19429/1/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test@63 PS1, Line 63: a > Nit: expanding. Done -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 19 Jan 2023 06:06:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Hello Fang-Yu Rao, Daniel Becker, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19429 to look at the new patch set (#2). Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking resolvePathWithMasking() is a wrapper on resolvePath() to further resolve nested columns inside the table masking view. When it was added, complex types in the select list hadn't been supported yet. So the table masking view can't expose complex type columns directly in the select list. Any paths in nested types will be further resolved inside the table masking view in resolvePathWithMasking(). Take the following query as an example: select id, nested_struct.* from complextypestbl; If Ranger column-masking/row-filter policies applied on the table, the query is rewritten as select id, nested_struct.* from ( select mask(id) from complextypestbl where row-filtering-condition ) t; Table masking view "t" can't expose the nested column "nested_struct". So we further resolve "nested_struct" inside the inlineView to use the masked table "complextypestbl". The underlying TableRef is expected to be a BaseTableRef. Paths that don't reference nested columns should be resolved and returned directly (just like the original resolvePath() does). E.g. select v.* from masked_view v is rewritten to select v.* from ( select mask(c1), mask(c2), ..., mask(cn) from masked_view where row-filtering-condition ) v; The STAR path "v.*" should be resolved directly. However, it's treated as a nested column unexpectedly. The code then try to resolve it inside the table "masked_view" and found it's not a table so throws the IllegalStateException. These are the conditions for returning the STAR paths directly: - The type is STRUCT - And the resolved path is rooted at a valid tuple descriptor They don't really recognize the nested columns. STAR expansion is only valid for paths to a struct type (or a table/view). So the first condition always matches. The second condition also matches for STAR paths on table/view, i.e. paths of "v.*" when "v" is a catalog table/view. The rooted tuple descriptor is exactly the output tuple of the table/view. This patch fixes the check for nested struct STAR expansion by checking the matched types instead. Note that if "v.*" is a table/view expansion, the matched type list is empty. If "v.*" is a struct column expansion, the matched type list contains the STRUCT column type. Tests: - Add missing coverage on STAR paths (v.*) on masked views. Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking.test M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test 4 files changed, 67 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19429/2 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@12 PS1, Line 12: the table masking view can't expose complex type columns directly > Is it still true? If so, can't we / should we change table masking views so I see you've already created IMPALA-11847 for this. -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 18 Jan 2023 12:31:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@7 PS1, Line 7: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking Do I understand it correctly that the error occurs if we try to query a star on a table/view, not when we try to query a star on a struct? Such as the example in the Jira: select v.* from masked_view v; ERROR: IllegalStateException: null If so, could you make this clear in the commit message, and include the example here? http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@10 PS1, Line 10: While Nit: When http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@11 PS1, Line 11: haven Nit: hadn't http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@12 PS1, Line 12: the table masking view can't expose complex type columns directly Is it still true? If so, can't we / should we change table masking views so that they can expose complex types too? http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@32 PS1, Line 32: - The type is STRUCT Could you make it clearer that these are the conditions for returning the paths directly? It wasn't obvious for me. http://gerrit.cloudera.org:8080/#/c/19429/1//COMMIT_MSG@33 PS1, Line 33: root Nit: rooted? http://gerrit.cloudera.org:8080/#/c/19429/1/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test File testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test: http://gerrit.cloudera.org:8080/#/c/19429/1/testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test@63 PS1, Line 63: e Nit: expanding. -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 18 Jan 2023 11:07:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19429 ) Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12181/ : 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/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 18 Jan 2023 03:40:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19429 Change subject: IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking .. IMPALA-11845: Fix incorrect check of struct STAR path in resolvePathWithMasking resolvePathWithMasking() is a wrapper on resolvePath() to further resolve nested columns inside the table masking view. While it was added, complex types in the select list haven't been supported yet. So the table masking view can't expose complex type columns directly in the select list. Any paths in nested types will be further resolved inside the table masking view in resolvePathWithMasking(). Take the following query as an example: select id, nested_struct.* from complextypestbl; If Ranger column-masking/row-filter policies applied on the table, the query is rewritten as select id, nested_struct.* from ( select mask(id) from complextypestbl where row-filtering-condition ) t; Table masking view "t" can't expose the nested column "nested_struct". So we further resolve it inside the inlineView if it's a table masking view. Paths that don't reference nested columns are resolved and returned directly (just like the original resolvePath() does). However, the current check on STAR type paths doesn't really recognize the nested columns: - The type is STRUCT - The resolved path is root at a valid tuple descriptor STAR expansion is only valid for paths to a struct type (or a table/view). So the first condition always matches. The second condition also matches for STAR paths on table/view, i.e. paths of "v.*" when "v" is a catalog table/view. The code then treats all paths of "v.*" as nested column expansion so result in IllegalStateException. This patch fixes the check for nested struct STAR expansion by checking the matched types instead. Note that if "v.*" is a table/view expansion, the matched type list is empty. If "v.*" is a struct column expansion, the matched type list contains the STRUCT column type. Tests: - Add missing coverage on STAR paths (v.*) on masked views. Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 --- M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/Path.java M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking.test M testdata/workloads/functional-query/queries/QueryTest/ranger_column_masking_complex_types.test 4 files changed, 66 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/19429/1 -- To view, visit http://gerrit.cloudera.org:8080/19429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8f1e78e325baafbe23101909d47e82bf140a2d77 Gerrit-Change-Number: 19429 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang
