singhpk234 commented on code in PR #13879:
URL: https://github.com/apache/iceberg/pull/13879#discussion_r2294486557
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3260,25 +3260,25 @@ components:
additionalProperties:
type: string
- FineGrainedDataProtectionRules:
+ ReadRestrictions:
type: object
description: >
- Fine-grained data protection rules for a table as result of fine
grained policy evaluation at the catalog end based on the clients access rights.
-
- The client SHOULD use these rules to enforce fine-grained data
protection like column and row level access when reading data from the table.
+ Read Restrictions for a table including projection and row filter
expressions.
+ The client MUST enforce these rules to read data from the table.
Review Comment:
> This sentence seems to disallow doing that because it says clients MUST
treat it as a table without restrictions
I see, the intention was to be explicit in saying there is no additional
work that a client should do, as the confusion could be that when i have a
projection list empty does it mean am i not allowed to access any columns ?
How about we add `required-projection` the list of projection that must be
`additionally` applied, this way we convey that this is an additional Project
on table of the ScanNode, thoughts, i do agree this seems a bit redundant
since this is obvious / implicit, it just to address the concern if there is on
the no project list is it `select *` just to be overly explicit, as otherwise
a catalog could have thrown 403 instead if none of the columns were supposed to
be seen.
> make it clear in this description that these restrictions are specific to
the authenticated principal/user/account that is using the client
Agree, I had this line initially
> as result of fine grained policy evaluation at the catalog end based on
the clients access right
to which the feedback
([here](https://github.com/apache/iceberg/pull/13879#discussion_r2289483359))
was it was a bit over specified and implicit, my personal opinion is, I also do
see value in the above in adding this line but i removed it when i checked the
credentials description as its also according to calling clients authenticated
principal, but since we don't se anything there I removed it, thinking we are
being overly implicit.
https://github.com/apache/iceberg/blob/b82dac4858fb2d15929a797660252c50f8e00ed8/open-api/rest-catalog-open-api.yaml#L3248-L3271
@amogh-jahagirdar thoughts on including the line again, about it being
according the clients authenticated prinicpal / roles ?
--
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]
