[WIP][core] Fix column masking correctness in query-auth reads#8570
Open
plusplusjiajia wants to merge 1 commit into
Open
[WIP][core] Fix column masking correctness in query-auth reads#8570plusplusjiajia wants to merge 1 commit into
plusplusjiajia wants to merge 1 commit into
Conversation
6bb3c6c to
117e3e5
Compare
62a53ad to
5a49a17
Compare
f2f677f to
62849c7
Compare
A cross-column mask (e.g. display := concat_ws('-', first, last)) threw
at read time when the query projected the masked target but not the
mask's input columns: "Column masking refers to field 'first' which is
not present in output row type".
Row-filter operands are already added to the read projection and
projected back out (apache#8447); this does the same for column-mask inputs.
TableQueryAuthResult.requiredAuthFields collects filter operands plus,
transitively, the inputs of every mask whose target is readable; the
widening appends the missing table columns as-is (preserving nested
pruning), and the scan pushes the widened read type before planning so
file-level column pruning (e.g. data evolution) keeps the files those
columns live in.
Stale rules (columns absent from the latest schema, e.g. after a rename
or drop) fail closed once at plan time in AbstractDataTableScan.authQuery,
the common path of every scan; re-validated only when the fetched rules
change. Projected system fields (e.g. _ROW_ID) stay valid, unprojected
ones are rejected; rules touching a nested-pruned or unprojected
blob-view column are rejected too.
The read schema is fixed once the first split reader exists (split reads
cache their format readers and ignore later read-type changes); every
split is projected back to the query's read type, and masks apply only
to columns readable from the query, so a column merely retained in a
previously widened schema does not activate a mask after a rules change.
Previously the first createReader call mutated this.readType, so later
splits of the same TableRead leaked the auth-added columns -- also on
the pre-existing row-filter path.
Related re-configuration fixes: MergeFileSplitRead.withReadType and
DataEvolutionFileStoreScan.withReadType reset stale projections, and
IncrementalDiffSplitRead projects from the merge read's actual output
type instead of the full table row type.
Raw-value pushdown safety on masked columns (filter/TopN/limit stats
pruning) is deliberately left to a follow-up PR.
Behavior changes are scoped to query-auth.enabled tables. Tables without
query auth and the write path are unaffected. System tables and
vector/full-text search reads do not consult auth rules (pre-existing;
restrict at the permission layer).
62849c7 to
17823a0
Compare
Contributor
|
Member
Author
|
@JingsongLi Agreed. I'll fold the masked-column pushdown safety into this PR so masking is correct end to end in one place, and close #8582. Updating this PR shortly. |
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
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.
Purpose
A cross-column mask (e.g. display := concat_ws('-', first, last)) throws at read time when the query projects the masked target but not the mask's input columns:
Column masking refers to field 'first' which is not present in output row type
Row-filter operands are already added to the read projection (#8447); this PR does the same for column-mask inputs, and hardens the paths that surfaced while testing it:
(preserving nested pruning), the widened columns are projected back out after masking, and the scan pushes the widened read type before planning so file-level column pruning (e.g. data evolution) keeps the
files those columns live in.
re-validated only when the fetched rules change. Projected system fields (e.g. _ROW_ID) stay valid; unprojected ones, nested-pruned columns and blob-view columns are rejected.
only to columns readable from the query, so a column merely retained in a previously widened schema cannot activate a mask after a rules change. Previously the first createReader call mutated this.readType, so
later splits of the same TableRead leaked the auth-added columns — also on the pre-existing row-filter path.
output type.
Behavior changes are scoped to query-auth.enabled tables; tables without query auth and the write path are unaffected.
Out of scope: raw-value pushdown on masked columns (filter/TopN/limit statistics pruning, masked partition predicates) is unsafe today on master for the already-merged masking feature. This PR does not change
any pushdown behavior; the fix is #8582 (stacked on this PR), where a predicate on a masked column evaluates on the masked value.
Tests
changing between reads of one TableRead, repeated reads.