[KYUUBI #7576][AUTHZ] Fix data masking failure on a UNION-ALL view over a masked view#7577
Open
j1wonpark wants to merge 3 commits into
Open
[KYUUBI #7576][AUTHZ] Fix data masking failure on a UNION-ALL view over a masked view#7577j1wonpark wants to merge 3 commits into
j1wonpark wants to merge 3 commits into
Conversation
…iew nested over a masked view RED reproduction only (no fix yet). Reading a view that UNION-ALLs a masked view fails analysis with MISSING_ATTRIBUTES, while a single-level masked view (KYUUBI apache#3581) works. [MISSING_ATTRIBUTES.RESOLVED_ATTRIBUTE_APPEAR_IN_OPERATION] Resolved attribute(s) "value2" missing from "value2" in operator !Project [cast(value2#591 as string) AS value2#581]. Attribute(s) with the same name appear in the operation: "value2". Two references to the masked view force exprId deduplication, and the outer view's schema compensation adds a top cast Project. RuleApplyDataMaskingStage1 rewrites by original scan exprId (transformExpressionsUp), so across the union + schema-binding boundary the mapping goes stale and the top cast dangles. Added a type-preserving view-level MASK policy on value2 (perm_view_masked) to the test fixture -- value1's MASK_HASH changes INT to STRING and would trip CANNOT_UP_CAST first, hiding this bug. Mirrors a production Impala/Hive view (get_json_object-derived STRING column, masked, wrapped by a 5-branch UNION ALL view, both WITH SCHEMA BINDING). Signed-off-by: Jiwon Park <jpark92@outlook.kr>
… — masking is the cause Adds a control test proving the UNION-ALL + WITH SCHEMA BINDING structure resolves fine in Spark, and only the Kyuubi data-masking rewrite breaks it. perm_view_user (has value2 mask) : MISSING_ATTRIBUTES (fails) admin (no mask policy) : resolves, sees unmasked value (passes) Same view shapes, same nesting, same schema binding. The only difference is whether a masking policy applies -- so this is a Kyuubi data-masking bug, not a Spark bug. Signed-off-by: Jiwon Park <jpark92@outlook.kr>
…iew over a masked view Reading a permanent view that UNION-ALLs a masked permanent view fails analysis with MISSING_ATTRIBUTES. Both branches expand the masked view with identical output exprIds; after RuleApplyDataMaskingStage0 marks both branches, DeduplicateRelations re-instances one branch's PermanentViewMarker, but the marker's scan snapshot is not a child and keeps the pre-dedup ids, so that branch's exprToMaskers() becomes keyed on ids now owned by the other branch. RuleApplyDataMaskingStage1 then merges all marker maps into one Map (duplicate keys silently last-wins) and substitutes unconditionally, rewiring the outer view's schema compensation Project -- above the Union -- to attributes the Union never outputs. The stale map is still the correct substitution inside its own branch, whose ancestors do reference the stale ids, so the map itself must not be changed; what is wrong is its application scope. Scope the Stage1 substitution instead: rewrite the children first, keep only the maskers the rewritten children actually expose (matched by exprId, since SubqueryAlias may requalify child outputs), and merge the per-marker maps after that filtering so an out-of-scope entry cannot shadow the in-scope one on a colliding key. When a child is not resolved yet (a type-changing masker such as MASK_HASH leaves Union branches incompatible until type coercion runs, and Union.output fails on them), keep the previous unfiltered substitution for that operator. All five data masking suites pass (84 succeeded, 6 pre-existing view-unsupported skips) and the full kyuubi-spark-authz module is green (662 succeeded, 42 pre-existing skips). Signed-off-by: Jiwon Park <jpark92@outlook.kr>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why are the changes needed?
Close #7576.
Reading a permanent view that UNION-ALLs a masked permanent view fails with
MISSING_ATTRIBUTES, while reading the masked view directly works (#3581).Both branches expand the masked view with identical output exprIds. After
RuleApplyDataMaskingStage0marks both branches,DeduplicateRelationsre-instances one branch, so that branch'sexprToMaskers()ends up keyed on ids now owned by the other branch.RuleApplyDataMaskingStage1merges all marker maps into oneMap(duplicate keys silently last-wins) and substitutes unconditionally, rewiring the outer view's schema compensation Project — above the Union — to attributes the Union never outputs. See #7576 for the full analysis.This change scopes the Stage1 substitution: rewrite the children first, keep only the maskers the rewritten children actually expose (matched by exprId, since
SubqueryAliasmay requalify outputs), and merge the per-marker maps after that filtering. When a child is not resolved yet (a type-changing masker leaves Union branches incompatible until type coercion runs, andUnion.outputfails on them), the previous unfiltered substitution is kept. Branch-local substitution keeps working; the cross-branch leak is blocked at the Union boundary.How was this patch tested?
Two tests added to
DataMaskingTestBase(run by all data masking suites): a reproduction that failed withMISSING_ATTRIBUTESbefore this fix, and a control proving the same view shape resolves fine without a masking policy. A view-level type-preserving MASK policy is added to the fixture, since a type-changing mask dies earlier withCANNOT_UP_CASTand would hide the bug.All five data masking suites pass, and the full kyuubi-spark-authz module is green (662 succeeded / 0 failed / 42 pre-existing skips).
Was this patch authored or co-authored using generative AI tooling?
Assisted-by: Claude:claude-fable-5