sungwy commented on code in PR #15750:
URL: https://github.com/apache/iceberg/pull/15750#discussion_r3579680083
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3734,6 +3734,50 @@ components:
type: array
items:
$ref: '#/components/schemas/StorageCredential'
+ labels:
+ $ref: '#/components/schemas/Labels'
+
+ Labels:
+ type: object
+ description: |
+ Catalog-provided metadata enrichment (for example ownership,
+ classification, or cost attribution) returned with a table or view.
+ Labels are ephemeral: catalog-owned, generated per request, and never
+ persisted to table metadata or versioned with table history. Optional;
Review Comment:
I gave this some thought, and I'm not sure if we want to go as far as saying
that this is: _catalog-owned_, or that they are _never persisted to table
metadata or versioned with table history_.
The decision to persist the labels together with table metadata or history
seems to me like an implementation choice, and we'd be over-reaching beyond the
REST Spec's responsibility by mandating that. The decision to have the labels
be _catalog-owned_, I think is also an implementation concern.
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3734,6 +3734,50 @@ components:
type: array
items:
$ref: '#/components/schemas/StorageCredential'
+ labels:
+ $ref: '#/components/schemas/Labels'
+
+ Labels:
+ type: object
+ description: |
+ Catalog-provided metadata enrichment (for example ownership,
+ classification, or cost attribution) returned with a table or view.
+ Labels are ephemeral: catalog-owned, generated per request, and never
+ persisted to table metadata or versioned with table history. Optional;
+ clients may ignore them. Labels are catalog-specific; different
catalogs
+ may return different or no labels for the same object. `table` carries
+ entity-level labels; `columns` is an array of per-column entries, each
Review Comment:
> entity-level
What do we mean by entity-level? If we mean table or views, do we want to
consider referring to this as "catalog object" instead?
https://github.com/apache/iceberg/blob/7a882c2d34dda1e0fdb4a3b588453ab1788867c4/open-api/rest-catalog-open-api.yaml#L2413-L2418
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3734,6 +3734,50 @@ components:
type: array
items:
$ref: '#/components/schemas/StorageCredential'
+ labels:
+ $ref: '#/components/schemas/Labels'
+
+ Labels:
+ type: object
+ description: |
+ Catalog-provided metadata enrichment (for example ownership,
+ classification, or cost attribution) returned with a table or view.
+ Labels are ephemeral: catalog-owned, generated per request, and never
+ persisted to table metadata or versioned with table history. Optional;
+ clients may ignore them. Labels are catalog-specific; different
catalogs
+ may return different or no labels for the same object. `table` carries
Review Comment:
> Labels are catalog-specific; different catalogs
may return different or no labels for the same object
I'm trying to understand this better. Are we talking about the case where
the "same" catalog object can be exposed via multiple catalogs?
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3734,6 +3734,50 @@ components:
type: array
items:
$ref: '#/components/schemas/StorageCredential'
+ labels:
+ $ref: '#/components/schemas/Labels'
+
+ Labels:
+ type: object
+ description: |
+ Catalog-provided metadata enrichment (for example ownership,
+ classification, or cost attribution) returned with a table or view.
+ Labels are ephemeral: catalog-owned, generated per request, and never
+ persisted to table metadata or versioned with table history. Optional;
+ clients may ignore them. Labels are catalog-specific; different
catalogs
+ may return different or no labels for the same object. `table` carries
+ entity-level labels; `columns` is an array of per-column entries, each
+ identifying its column by field-id.
+ properties:
+ table:
+ type: object
+ description: Top-level entity labels (attached to the catalog object
as a whole).
+ additionalProperties:
+ type: string
+ columns:
+ type: array
+ description: >-
+ Column-level labels. Each entry identifies its column by field-id.
+ items:
+ $ref: '#/components/schemas/ColumnLabels'
+
+ ColumnLabels:
+ type: object
+ required:
+ - field-id
+ - labels
+ properties:
+ field-id:
+ type: integer
+ format: int32
+ minimum: 1
+ maximum: 2147483447
Review Comment:
nit: I don't see a precedent for putting in language type limitations into
the REST Spec today
```suggestion
```
example:
https://github.com/apache/iceberg/blob/7a882c2d34dda1e0fdb4a3b588453ab1788867c4/open-api/rest-catalog-open-api.yaml#L2689-L2699
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3734,6 +3734,50 @@ components:
type: array
items:
$ref: '#/components/schemas/StorageCredential'
+ labels:
+ $ref: '#/components/schemas/Labels'
+
+ Labels:
+ type: object
+ description: |
+ Catalog-provided metadata enrichment (for example ownership,
+ classification, or cost attribution) returned with a table or view.
+ Labels are ephemeral: catalog-owned, generated per request, and never
+ persisted to table metadata or versioned with table history. Optional;
+ clients may ignore them. Labels are catalog-specific; different
catalogs
+ may return different or no labels for the same object. `table` carries
+ entity-level labels; `columns` is an array of per-column entries, each
+ identifying its column by field-id.
+ properties:
+ table:
+ type: object
+ description: Top-level entity labels (attached to the catalog object
as a whole).
+ additionalProperties:
+ type: string
+ columns:
+ type: array
+ description: >-
+ Column-level labels. Each entry identifies its column by field-id.
+ items:
+ $ref: '#/components/schemas/ColumnLabels'
Review Comment:
do we anticipate all future catalog objects to always have columns? Or would
it make sense to decouple these as:
- `CatalogObjectLabels` (only additionalProperties)
- `FieldBoundLabels` array of (field-id and additionalProperties)
I see there was a previous discussion thread on this with @nastra
(https://github.com/apache/iceberg/pull/15750#discussion_r3232344676)
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3734,6 +3734,50 @@ components:
type: array
items:
$ref: '#/components/schemas/StorageCredential'
+ labels:
+ $ref: '#/components/schemas/Labels'
+
+ Labels:
+ type: object
+ description: |
+ Catalog-provided metadata enrichment (for example ownership,
+ classification, or cost attribution) returned with a table or view.
+ Labels are ephemeral: catalog-owned, generated per request, and never
+ persisted to table metadata or versioned with table history. Optional;
+ clients may ignore them. Labels are catalog-specific; different
catalogs
+ may return different or no labels for the same object. `table` carries
+ entity-level labels; `columns` is an array of per-column entries, each
+ identifying its column by field-id.
+ properties:
+ table:
Review Comment:
nit for consideration:
```suggestion
labels:
```
column labels are referred to as `labels` but we refer to them as `table`
here
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3734,6 +3734,50 @@ components:
type: array
items:
$ref: '#/components/schemas/StorageCredential'
+ labels:
+ $ref: '#/components/schemas/Labels'
+
+ Labels:
Review Comment:
style nit:
Should we follow the repository convention of putting the Base Schema before
the composite schema? (i.e. Labels before LoadTableResult)
--
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]
