singhpk234 commented on code in PR #15750:
URL: https://github.com/apache/iceberg/pull/15750#discussion_r3647501297
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3675,6 +3675,50 @@ components:
items:
$ref: '#/components/schemas/StorageCredential'
+ CatalogObjectLabels:
+ type: object
+ description: >-
+ Catalog-object-level labels, attached to the object (table, view, ...)
+ as a whole.
+ additionalProperties:
+ type: string
+
+ FieldLabels:
+ type: object
+ description: Labels attached to a single field, identified by field-id.
+ required:
+ - field-id
+ - labels
+ properties:
+ field-id:
+ type: integer
+ description: Field ID from the current schema of the table or view
Review Comment:
why only from current schema ? for example consider a PII column which is
now dropped from current schema ... when one does tt reads are we saying its ok
to not to send that info ?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3675,6 +3675,50 @@ components:
items:
$ref: '#/components/schemas/StorageCredential'
+ CatalogObjectLabels:
+ type: object
+ description: >-
+ Catalog-object-level labels, attached to the object (table, view, ...)
Review Comment:
This is getting tricky and goes in the world of defining how attaching works
when we don't have an agreed upon path of how the attachment can happen ?
why can't labels be attached to namespace/ function for example ?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -4033,6 +4074,8 @@ components:
type: object
additionalProperties:
type: string
+ labels:
+ $ref: '#/components/schemas/Labels'
Review Comment:
can you please elaborate the use case of when does one has labels on view ?
we had similar debate on read restriction on this and we decided to not have
this for views
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3675,6 +3675,50 @@ components:
items:
$ref: '#/components/schemas/StorageCredential'
+ CatalogObjectLabels:
+ type: object
+ description: >-
+ Catalog-object-level labels, attached to the object (table, view, ...)
+ as a whole.
+ additionalProperties:
+ type: string
+
+ FieldLabels:
+ type: object
+ description: Labels attached to a single field, identified by field-id.
+ required:
+ - field-id
+ - labels
+ properties:
+ field-id:
+ type: integer
+ description: Field ID from the current schema of the table or view
+ labels:
+ type: object
+ description: Flat key-value labels for this field
+ additionalProperties:
+ type: string
+
+ Labels:
+ type: object
+ description: |
+ Catalog-provided metadata enrichment (for example ownership,
+ classification, or cost attribution) returned with a table or view.
+ Labels are generated per request and optional; clients may ignore them.
+ The spec does not require how a catalog produces or stores labels, nor
+ whether they are persisted or versioned. `object` carries
+ catalog-object-level labels; `fields` carries per-field labels, each
+ identified by field-id.
+ properties:
+ object:
+ $ref: '#/components/schemas/CatalogObjectLabels'
+ fields:
+ type: array
+ description: >-
+ Field-level labels. Each entry identifies its field by field-id.
Review Comment:
what happens in case of nested fields ? can the outer struct say
{'sensitivity': high} and the inner struct say {'sensistivity': low } ....
which is correct then ?
how would one treat this info, if we are saying this is info what one should
use for governing access to the enitities, we should be explicit on the
interpretation
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3675,6 +3675,50 @@ components:
items:
$ref: '#/components/schemas/StorageCredential'
+ CatalogObjectLabels:
+ type: object
+ description: >-
+ Catalog-object-level labels, attached to the object (table, view, ...)
+ as a whole.
+ additionalProperties:
+ type: string
+
+ FieldLabels:
+ type: object
+ description: Labels attached to a single field, identified by field-id.
+ required:
+ - field-id
+ - labels
+ properties:
+ field-id:
+ type: integer
+ description: Field ID from the current schema of the table or view
+ labels:
+ type: object
+ description: Flat key-value labels for this field
Review Comment:
I am fairly concerned on how feasible / practical it for platforms to rely
on this,
since these are sometimes well defined object which have their own
inheritance mechanism.
I checked some of the catalogs which actually do supports tags
(unfortunately Apache Polaris is not a good example which is used in POC as it
neither supports ABAC nor does it support TAGs (there is proposal just
published)) and it feels pretty standard to have such kind of info carried for
example :
- https://github.com/apache/gravitino/blob/main/docs/open-api/tags.yaml#L376
i believe we should think this through more on how are we gonna converge
this behaviour for example consider a label attached to a namespace and gets
inherited to the table, it flows to another catalog which mounts this as an
external obj... how will the external catalog know this was inherited ? and if
in the external catalog platform if some one attaches direct label to the obj
it would override the namespace one ?
These things make me nervous in having this in the protocol without we
agreeing on the attachment experience.
I understand the proposal with exposing things that catalog tracks and
should not be part of table props but this is getting more involved if we
include governance.
not dictating this behaviour can also be possible but i believe folks who
exchange this info will be forced to stuff more in this kv pair and end up
making custom adapters around it and i feel it defeats the purpose of having
this in the interoperable spec in the first place
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3675,6 +3675,50 @@ components:
items:
$ref: '#/components/schemas/StorageCredential'
+ CatalogObjectLabels:
+ type: object
+ description: >-
+ Catalog-object-level labels, attached to the object (table, view, ...)
+ as a whole.
+ additionalProperties:
+ type: string
+
+ FieldLabels:
+ type: object
+ description: Labels attached to a single field, identified by field-id.
+ required:
+ - field-id
+ - labels
+ properties:
+ field-id:
+ type: integer
+ description: Field ID from the current schema of the table or view
+ labels:
+ type: object
+ description: Flat key-value labels for this field
+ additionalProperties:
+ type: string
+
+ Labels:
+ type: object
+ description: |
+ Catalog-provided metadata enrichment (for example ownership,
+ classification, or cost attribution) returned with a table or view.
+ Labels are generated per request and optional; clients may ignore them.
Review Comment:
can you please elaborate what is meant per request ? are you suggesting
clients should not cache ?
can labels be contextual ? based on caller context ?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3675,6 +3675,50 @@ components:
items:
$ref: '#/components/schemas/StorageCredential'
+ CatalogObjectLabels:
+ type: object
+ description: >-
+ Catalog-object-level labels, attached to the object (table, view, ...)
+ as a whole.
+ additionalProperties:
+ type: string
+
+ FieldLabels:
+ type: object
+ description: Labels attached to a single field, identified by field-id.
+ required:
+ - field-id
+ - labels
+ properties:
+ field-id:
+ type: integer
+ description: Field ID from the current schema of the table or view
+ labels:
+ type: object
+ description: Flat key-value labels for this field
+ additionalProperties:
+ type: string
Review Comment:
if labels names can be renamed then key name is not sufficient for
uniqueness in case of cross platform syncing, as a user might have taken a lot
of action based on these synced label for example attaching a PII policy to the
label.
How do we expect the system to be reconcile in this scenario if we have
things like `uuid` missing from these ?
--
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]
