singhpk234 commented on code in PR #13879:
URL: https://github.com/apache/iceberg/pull/13879#discussion_r3654614778
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3480,6 +3480,309 @@ components:
additionalProperties:
type: string
+ ReadRestrictions:
+ type: object
+ description: >
+ Read restrictions for a table, including column projections and row
filter expressions.
+
+ A client MUST enforce the restrictions defined in this object when
reading data
+ from the table.
+
+ These restrictions apply only to the authenticated principal, user,
or account
+ associated with the request. They MUST NOT be interpreted as global
policy and
+ MUST NOT be applied beyond the entity identified by the
Authentication header
+ (or other applicable authentication mechanism).
+
+ If both properties are absent or empty, the ReadRestrictions object
imposes no
+ restrictions and is equivalent to the field being absent from the
response.
+ A server MUST NOT return an action for a column whose type is not
listed in
+ that action's "Applicable to" set.
+ For all actions, if the input column value is NULL, the output MUST
be NULL.
+
+ If a column projection targets a struct-typed field, other column
projections
+ in the same ReadRestrictions MUST NOT target any of that struct's
subfields
+ (at any depth). This avoids ambiguity about which action governs a
given
+ leaf value.
Review Comment:
I spent a fair amount of time researching how other systems handle this
before answering, and the short version is that I couldn't find an established
semantics to borrow. That's why I'd rather not define a resolution here yet.
Every system I looked at either makes the container un-targetable or rejects
the overlap outright. BigQuery: "Policy tags can't be applied to columns that
use the STRUCT data type, but they can be associated with the leaf fields of
such columns" [1]. Redshift: "You can only apply masking policies to scalar
values on the SUPER path. You can't apply masking policies to complex
structures or arrays" [2]. I also checked Snowflake, Databricks, Oracle, Db2,
Vertica, Trino, Hive, Impala and Ranger, and none of them define a
parent-vs-nested-field resolution for masks.
Redshift is the only engine I found that lets both be expressed. It formally
defines the pair as a conflict: "the SUPER paths a.b and a.b.c conflict because
they are on the same path, with a.b being the parent of a.b.c" [2]. It then
resolves that with an admin-supplied integer, where "only the highest priority
attachment takes effect" [3]. Critically, it "can't attach two different
policies to the same column with equal priority", and the default priority is 0
[4]. We have no priority concept, and its part of ATTACHEMENT of Redshift's own
behaviour in our situation is to reject.
The question is also unresolved everywhere it has been raised. RANGER-3525
"Clarify handling of column masks on nested types" has been open since 2021
[5], HIVE-22823 likewise [6], and Trino #16964 is open with a reviewer
explicitly confirming that ANSI SQL defines nothing for nested column access
control [7]. I checked the SQL:2016 Foundation grammar directly: <privilege
column list> resolves to a flat <column name list>, and MASK, MASKED, POLICY
and ROW LEVEL don't appear anywhere in it [8].
Given that, forbidding is the reversible choice and defining precedence is
not. If we say MUST NOT now and later find a real use case, relaxing it is
backward-compatible, because responses that were invalid become valid. If we
define "outer-most wins" now and later decide the inner action should
contribute, we have silently changed the values a compliant reader returns for
the same response.
On the future-actions concern specifically, I don't think the prohibition
limits any individual action. A future action can still target a container, or
target a nested field. It just can't do both in the same response. Only the
combination is ruled out, and that combination has no defined meaning today
regardless.
What I do think is wrong is the current justification. It claims this
"avoids ambiguity about which action governs a given leaf value", and that
isn't accurate. With today's action set there is no ambiguity, because
replace-with-null and mask-to-fixed-value are the only actions applicable to a
nested type and both fully overwrite everything beneath. mask-to-fixed-value on
a struct sets "each field ... to its type-specific default (applied
recursively)", on a list yields [], and on a map yields {} [9]. So I'd like the
wording to say what we actually mean:
> If a projection targets a nested-typed field (struct, list, or map), other
projections in the same ReadRestrictions must not target any nested field-id
(struct subfields, list elements, or map keys/values) at any depth. This
specification does not define how such actions combine. A reader that receives
such a response must fail the query.
The last sentence closes a gap I noticed while re-reading. The
duplicate-field-id rule (item 3) tells the reader to fail, but this rule only
constrains the server. Silently ignoring an action is the failure mode I'd most
want ruled out. Impala shipped exactly that for container masks and changed it
to fail-closed in 5.0.0 precisely because it
"returned the column unmasked, leaking data te" [10].
References
[1] BigQuery, Introduction to column data mask
https://docs.cloud.google.com/bigquery/docs/column-data-masking-intro
[2] Amazon Redshift, Using dynamic data masking with SUPER data type paths.
https://docs.aws.amazon.com/redshift/latest/dg/t_ddm-super.html
[3] Amazon Redshift, Dynamic data masking
polhttps://docs.aws.amazon.com/redshift/latest/dg/t_ddm-hierarchy.html
[4] Amazon Redshift, ATTACH MASKING POLICY.
https://docs.aws.amazon.com/redshift/latest/dhtml
[5] RANGER-3525, Clarify handling of column masks on nested types (open
since 2021-11-24). https://issues.apache.org/jira/browse/RANGER-3525
[6] HIVE-22823, Support column masking
policyhttps://issues.apache.org/jira/browse/HIVE-22823
[7] trinodb/trino#16964, Add support of access control for nested columns
(open).
https://github.com/trinodb/trino/issues/16964
[8] SQL:2016 Foundation grammar (extracted BNF), section 12 <grant
statement>.
https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html
[9] This PR, MaskToFixedValue and ReplaceWithitions,
open-api/rest-catalog-open-api.yamllines 3805-3854.
[10] IMPALA-15130, Raise error when top-levelasking (fixed, Impala
5.0.0).https://issues.apache.org/jira/browse/IMPALA-15130
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
