You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Fokko (via GitHub)" <gi...@apache.org> on 2023/05/26 09:19:40 UTC
[GitHub] [iceberg] Fokko opened a new pull request, #7710: Open-API: Refactor TableRequirements
Fokko opened a new pull request, #7710:
URL: https://github.com/apache/iceberg/pull/7710
Noticed this when working on https://github.com/apache/iceberg/pull/6323
I believe the requirements are not defined in the right way. After generating code I got:
```python
class TableRequirement(BaseModel):
type: Literal[
'assert-create',
'assert-table-uuid',
'assert-ref-snapshot-id',
'assert-last-assigned-field-id',
'assert-current-schema-id',
'assert-last-assigned-partition-id',
'assert-default-spec-id',
'assert-default-sort-order-id',
]
ref: Optional[str] = None
uuid: Optional[str] = None
snapshot_id: Optional[int] = Field(None, alias='snapshot-id')
last_assigned_field_id: Optional[int] = Field(None, alias='last-assigned-field-id')
current_schema_id: Optional[int] = Field(None, alias='current-schema-id')
last_assigned_partition_id: Optional[int] = Field(
None, alias='last-assigned-partition-id'
)
default_spec_id: Optional[int] = Field(None, alias='default-spec-id')
default_sort_order_id: Optional[int] = Field(None, alias='default-sort-order-id')
```
Which encapsulates all the requirements. After the refactor in this PR, we'll end up with:
```python
class AssertCreate(BaseModel):
type: Literal['assert-create']
class AssertTableUUID(BaseModel):
type: Literal['assert-table-uuid']
uuid: str
class AssertRefSnapshotId(BaseModel):
type: Literal['assert-ref-snapshot-id']
ref: str
snapshot_id: int = Field(..., alias='snapshot-id')
class AssertLastAssignedFieldId(BaseModel):
type: Literal['assert-last-assigned-field-id']
last_assigned_partition_id: int = Field(..., alias='last-assigned-partition-id')
class AssertCurrentSchemaId(BaseModel):
type: Literal['assert-current-schema-id']
current_schema_id: int = Field(..., alias='current-schema-id')
class AssertLastAssignedPartitionId(BaseModel):
type: Literal['assert-last-assigned-partition-id']
last_assigned_partition_id: int = Field(..., alias='last-assigned-partition-id')
class AssertDefaultSpecId(BaseModel):
type: Literal['assert-default-spec-id']
default_spec_id: int = Field(..., alias='default-spec-id')
class AssertDefaultSortOrderId(BaseModel):
type: Literal['assert-default-sort-order-id']
default_sort_order_id: int = Field(..., alias='default-sort-order-id')
class TableRequirement(BaseModel):
__root__: Union[
AssertCreate,
AssertTableUUID,
AssertRefSnapshotId,
AssertLastAssignedFieldId,
AssertCurrentSchemaId,
AssertLastAssignedPartitionId,
AssertDefaultSpecId,
AssertDefaultSortOrderId,
] = Field(..., discriminator='type')
```
Which makes sense to me.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Open-API: Refactor TableRequirements [iceberg]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#issuecomment-1785528935
Thanks for the review @rdblue and @nastra for the review. I'll might revisitor other parts as well 👍
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Open-API: Refactor TableRequirements [iceberg]
Posted by "romusz (via GitHub)" <gi...@apache.org>.
romusz commented on PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#issuecomment-1809358653
Hi,
Have you tested this refactoring with code generated for Go?
What we get now is:
```go
type TableRequirement struct {
Type string `json:"type"`
}
```
and (for example)
```go
type AssertTableUUID struct {
TableRequirement
Type string `json:"type"`
Uuid string `json:"uuid"`
}
```
There are couple of problems with that:
1. double storage of `Type`
2. provided `[]TableRequirement{}` slice and the fact that `TableRequirement` is a struct not an interface, it cannot be "type switched" into it's "subtypes" (e.g. `AssertTableUUID`).
3. All previously-available, useful methods on `TableRequirement` have been nuked (e.g. `GetUuidOk` that allowed to extract TableUUID from `assert-table-uuid` requirement).
In summary, Go generated code does not produce equivalent to what it's shown for Python.
Please let me know if there is other way to traverse the requirements - so far it looks like a regression.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Open-API: Refactor TableRequirements [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#issuecomment-1783900918
Sorry for the late review. This looks great. Thanks @Fokko!
I didn't merge this in case it conflicts with the other linked PR. Feel free to merge in the order that makes the most sense.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Open-API: Refactor TableRequirements [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#discussion_r1375308652
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1824,58 +1824,146 @@ components:
- $ref: '#/components/schemas/SetPropertiesUpdate'
- $ref: '#/components/schemas/RemovePropertiesUpdate'
- TableRequirement:
Review Comment:
Nit: Should TableRequirement be defined first?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on a diff in pull request #7710: Open-API: Refactor TableRequirements
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#discussion_r1216646858
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1649,58 +1649,134 @@ components:
- $ref: '#/components/schemas/SetPropertiesUpdate'
- $ref: '#/components/schemas/RemovePropertiesUpdate'
- TableRequirement:
- description:
- Assertions from the client that must be valid for the commit to succeed. Assertions are identified by `type` -
-
- - `assert-create` - the table must not already exist; used for create transactions
-
- - `assert-table-uuid` - the table UUID must match the requirement's `uuid`
-
- - `assert-ref-snapshot-id` - the table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`; if `snapshot-id` is `null` or missing, the ref must not already exist
-
- - `assert-last-assigned-field-id` - the table's last assigned column id must match the requirement's `last-assigned-field-id`
-
- - `assert-current-schema-id` - the table's current schema id must match the requirement's `current-schema-id`
-
- - `assert-last-assigned-partition-id` - the table's last assigned partition id must match the requirement's `last-assigned-partition-id`
-
- - `assert-default-spec-id` - the table's default spec id must match the requirement's `default-spec-id`
-
- - `assert-default-sort-order-id` - the table's default sort order id must match the requirement's `default-sort-order-id`
+ AssertCreate:
type: object
+ description: The table must not already exist; used for create transactions
required:
- type
properties:
type:
type: string
- enum:
- - assert-create
- - assert-table-uuid
- - assert-ref-snapshot-id
- - assert-last-assigned-field-id
- - assert-current-schema-id
- - assert-last-assigned-partition-id
- - assert-default-spec-id
- - assert-default-sort-order-id
- ref:
+ enum: ["assert-create"]
+
+ AssertTableUUID:
+ description: The table UUID must match the requirement's `uuid`
+ required:
+ - type
+ - uuid
+ properties:
+ type:
type: string
+ enum: ["assert-table-uuid"]
uuid:
type: string
+
+ AssertRefSnapshotId:
+ description:
+ The table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`;
+ if `snapshot-id` is `null` or missing, the ref must not already exist
+ required:
+ - type
+ - ref
+ - snapshot-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-ref-snapshot-id" ]
+ ref:
+ type: string
snapshot-id:
type: integer
format: int64
- last-assigned-field-id:
+
+ AssertLastAssignedFieldId:
+ description:
+ The table's last assigned column id must match the requirement's `last-assigned-field-id`
+ required:
+ - type
+ - last-assigned-partition-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-last-assigned-field-id" ]
+ last-assigned-partition-id:
type: integer
+
+ AssertCurrentSchemaId:
+ description:
+ The table's current schema id must match the requirement's `current-schema-id`
+ required:
+ - type
+ - current-schema-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-current-schema-id" ]
current-schema-id:
type: integer
+
+ AssertLastAssignedPartitionId:
+ description:
+ The table's last assigned partition id must match the requirement's `last-assigned-partition-id`
+ required:
+ - type
+ - last-assigned-partition-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-last-assigned-partition-id" ]
last-assigned-partition-id:
type: integer
+
+ AssertDefaultSpecId:
+ description:
+ The table's default spec id must match the requirement's `default-spec-id`
+ required:
+ - type
+ - default-spec-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-default-spec-id" ]
default-spec-id:
type: integer
+
+ AssertDefaultSortOrderId:
+ description:
+ The table's default sort order id must match the requirement's `default-sort-order-id`
+ required:
+ - type
+ - default-sort-order-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-default-sort-order-id" ]
default-sort-order-id:
type: integer
+ TableRequirement:
+ oneOf:
Review Comment:
I don't think the `ReportMetricsRequest` metric is correct.
```yaml
ReportMetricsRequest:
anyOf:
- $ref: '#/components/schemas/ScanReport'
- $ref: '#/components/schemas/CommitReport'
required:
- report-type
properties:
report-type:
type: string
```
Let's take the `report-type`. Which value corresponds to the `ScanReport` or `CommitReport`? This is `scan-report` and `commit-report`, but this is not described in the spec. If you would generate code from it, and you have a class that accepts a `ReportMetricRequests`:
```yaml
post:
tags:
- Catalog API
summary: Send a metrics report to this endpoint to be processed by the backend
operationId: reportMetrics
requestBody:
description: The request containing the metrics report to be sent
content:
application/json:
schema:
$ref: '#/components/schemas/ReportMetricsRequest'
```
It cannot determine if is a `ScanReport` or `CommitReport`, this is where you need the discriminator.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7710: Open-API: Refactor TableRequirements
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#discussion_r1212781004
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1649,58 +1649,134 @@ components:
- $ref: '#/components/schemas/SetPropertiesUpdate'
- $ref: '#/components/schemas/RemovePropertiesUpdate'
- TableRequirement:
- description:
- Assertions from the client that must be valid for the commit to succeed. Assertions are identified by `type` -
-
- - `assert-create` - the table must not already exist; used for create transactions
-
- - `assert-table-uuid` - the table UUID must match the requirement's `uuid`
-
- - `assert-ref-snapshot-id` - the table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`; if `snapshot-id` is `null` or missing, the ref must not already exist
-
- - `assert-last-assigned-field-id` - the table's last assigned column id must match the requirement's `last-assigned-field-id`
-
- - `assert-current-schema-id` - the table's current schema id must match the requirement's `current-schema-id`
-
- - `assert-last-assigned-partition-id` - the table's last assigned partition id must match the requirement's `last-assigned-partition-id`
-
- - `assert-default-spec-id` - the table's default spec id must match the requirement's `default-spec-id`
-
- - `assert-default-sort-order-id` - the table's default sort order id must match the requirement's `default-sort-order-id`
+ AssertCreate:
type: object
+ description: The table must not already exist; used for create transactions
required:
- type
properties:
type:
type: string
- enum:
- - assert-create
- - assert-table-uuid
- - assert-ref-snapshot-id
- - assert-last-assigned-field-id
- - assert-current-schema-id
- - assert-last-assigned-partition-id
- - assert-default-spec-id
- - assert-default-sort-order-id
- ref:
+ enum: ["assert-create"]
+
+ AssertTableUUID:
+ description: The table UUID must match the requirement's `uuid`
+ required:
+ - type
+ - uuid
+ properties:
+ type:
type: string
+ enum: ["assert-table-uuid"]
uuid:
type: string
+
+ AssertRefSnapshotId:
+ description:
+ The table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`;
+ if `snapshot-id` is `null` or missing, the ref must not already exist
+ required:
+ - type
+ - ref
+ - snapshot-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-ref-snapshot-id" ]
+ ref:
+ type: string
snapshot-id:
type: integer
format: int64
- last-assigned-field-id:
+
+ AssertLastAssignedFieldId:
+ description:
+ The table's last assigned column id must match the requirement's `last-assigned-field-id`
+ required:
+ - type
+ - last-assigned-partition-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-last-assigned-field-id" ]
+ last-assigned-partition-id:
type: integer
+
+ AssertCurrentSchemaId:
+ description:
+ The table's current schema id must match the requirement's `current-schema-id`
+ required:
+ - type
+ - current-schema-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-current-schema-id" ]
current-schema-id:
type: integer
+
+ AssertLastAssignedPartitionId:
+ description:
+ The table's last assigned partition id must match the requirement's `last-assigned-partition-id`
+ required:
+ - type
+ - last-assigned-partition-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-last-assigned-partition-id" ]
last-assigned-partition-id:
type: integer
+
+ AssertDefaultSpecId:
+ description:
+ The table's default spec id must match the requirement's `default-spec-id`
+ required:
+ - type
+ - default-spec-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-default-spec-id" ]
default-spec-id:
type: integer
+
+ AssertDefaultSortOrderId:
+ description:
+ The table's default sort order id must match the requirement's `default-sort-order-id`
+ required:
+ - type
+ - default-sort-order-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-default-sort-order-id" ]
default-sort-order-id:
type: integer
+ TableRequirement:
+ oneOf:
Review Comment:
would it make sense to define the `type` enum here rather than having it in every subtype? (similar to how we do it for ` ReportMetricsRequest`)
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1649,58 +1649,134 @@ components:
- $ref: '#/components/schemas/SetPropertiesUpdate'
- $ref: '#/components/schemas/RemovePropertiesUpdate'
- TableRequirement:
- description:
- Assertions from the client that must be valid for the commit to succeed. Assertions are identified by `type` -
-
- - `assert-create` - the table must not already exist; used for create transactions
-
- - `assert-table-uuid` - the table UUID must match the requirement's `uuid`
-
- - `assert-ref-snapshot-id` - the table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`; if `snapshot-id` is `null` or missing, the ref must not already exist
-
- - `assert-last-assigned-field-id` - the table's last assigned column id must match the requirement's `last-assigned-field-id`
-
- - `assert-current-schema-id` - the table's current schema id must match the requirement's `current-schema-id`
-
- - `assert-last-assigned-partition-id` - the table's last assigned partition id must match the requirement's `last-assigned-partition-id`
-
- - `assert-default-spec-id` - the table's default spec id must match the requirement's `default-spec-id`
-
- - `assert-default-sort-order-id` - the table's default sort order id must match the requirement's `default-sort-order-id`
+ AssertCreate:
type: object
+ description: The table must not already exist; used for create transactions
required:
- type
properties:
type:
type: string
- enum:
- - assert-create
- - assert-table-uuid
- - assert-ref-snapshot-id
- - assert-last-assigned-field-id
- - assert-current-schema-id
- - assert-last-assigned-partition-id
- - assert-default-spec-id
- - assert-default-sort-order-id
- ref:
+ enum: ["assert-create"]
+
+ AssertTableUUID:
+ description: The table UUID must match the requirement's `uuid`
+ required:
+ - type
+ - uuid
+ properties:
+ type:
type: string
+ enum: ["assert-table-uuid"]
uuid:
type: string
+
+ AssertRefSnapshotId:
+ description:
+ The table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`;
+ if `snapshot-id` is `null` or missing, the ref must not already exist
+ required:
+ - type
+ - ref
+ - snapshot-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-ref-snapshot-id" ]
+ ref:
+ type: string
snapshot-id:
type: integer
format: int64
- last-assigned-field-id:
+
+ AssertLastAssignedFieldId:
+ description:
+ The table's last assigned column id must match the requirement's `last-assigned-field-id`
+ required:
+ - type
+ - last-assigned-partition-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-last-assigned-field-id" ]
+ last-assigned-partition-id:
type: integer
+
+ AssertCurrentSchemaId:
+ description:
+ The table's current schema id must match the requirement's `current-schema-id`
+ required:
+ - type
+ - current-schema-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-current-schema-id" ]
current-schema-id:
type: integer
+
+ AssertLastAssignedPartitionId:
+ description:
+ The table's last assigned partition id must match the requirement's `last-assigned-partition-id`
+ required:
+ - type
+ - last-assigned-partition-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-last-assigned-partition-id" ]
last-assigned-partition-id:
type: integer
+
+ AssertDefaultSpecId:
+ description:
+ The table's default spec id must match the requirement's `default-spec-id`
+ required:
+ - type
+ - default-spec-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-default-spec-id" ]
default-spec-id:
type: integer
+
+ AssertDefaultSortOrderId:
+ description:
+ The table's default sort order id must match the requirement's `default-sort-order-id`
+ required:
+ - type
+ - default-sort-order-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-default-sort-order-id" ]
default-sort-order-id:
type: integer
+ TableRequirement:
+ oneOf:
Review Comment:
would it make sense to define the `type` enum here rather than having it in every subtype? (similar to how we do it for `ReportMetricsRequest`)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Open-API: Refactor TableRequirements [iceberg]
Posted by "romusz (via GitHub)" <gi...@apache.org>.
romusz commented on PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#issuecomment-1809368041
@Fokko not sure if you'll see the above comment since the pr has been merged, thus an explicit call-out.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a diff in pull request #7710: Open-API: Refactor TableRequirements
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#discussion_r1212771288
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1649,58 +1649,134 @@ components:
- $ref: '#/components/schemas/SetPropertiesUpdate'
- $ref: '#/components/schemas/RemovePropertiesUpdate'
- TableRequirement:
- description:
- Assertions from the client that must be valid for the commit to succeed. Assertions are identified by `type` -
-
- - `assert-create` - the table must not already exist; used for create transactions
-
- - `assert-table-uuid` - the table UUID must match the requirement's `uuid`
-
- - `assert-ref-snapshot-id` - the table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`; if `snapshot-id` is `null` or missing, the ref must not already exist
-
- - `assert-last-assigned-field-id` - the table's last assigned column id must match the requirement's `last-assigned-field-id`
-
- - `assert-current-schema-id` - the table's current schema id must match the requirement's `current-schema-id`
-
- - `assert-last-assigned-partition-id` - the table's last assigned partition id must match the requirement's `last-assigned-partition-id`
-
- - `assert-default-spec-id` - the table's default spec id must match the requirement's `default-spec-id`
-
- - `assert-default-sort-order-id` - the table's default sort order id must match the requirement's `default-sort-order-id`
+ AssertCreate:
type: object
+ description: The table must not already exist; used for create transactions
required:
- type
properties:
type:
type: string
- enum:
- - assert-create
- - assert-table-uuid
- - assert-ref-snapshot-id
- - assert-last-assigned-field-id
- - assert-current-schema-id
- - assert-last-assigned-partition-id
- - assert-default-spec-id
- - assert-default-sort-order-id
- ref:
+ enum: ["assert-create"]
+
+ AssertTableUUID:
+ description: The table UUID must match the requirement's `uuid`
+ required:
+ - type
+ - uuid
+ properties:
+ type:
type: string
+ enum: ["assert-table-uuid"]
uuid:
type: string
+
+ AssertRefSnapshotId:
+ description:
+ The table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`;
+ if `snapshot-id` is `null` or missing, the ref must not already exist
+ required:
+ - type
+ - ref
+ - snapshot-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-ref-snapshot-id" ]
+ ref:
+ type: string
snapshot-id:
type: integer
format: int64
- last-assigned-field-id:
+
+ AssertLastAssignedFieldId:
+ description:
+ The table's last assigned column id must match the requirement's `last-assigned-field-id`
+ required:
+ - type
+ - last-assigned-partition-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-last-assigned-field-id" ]
+ last-assigned-partition-id:
Review Comment:
```suggestion
last-assigned-field-id:
```
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1649,58 +1649,134 @@ components:
- $ref: '#/components/schemas/SetPropertiesUpdate'
- $ref: '#/components/schemas/RemovePropertiesUpdate'
- TableRequirement:
- description:
- Assertions from the client that must be valid for the commit to succeed. Assertions are identified by `type` -
-
- - `assert-create` - the table must not already exist; used for create transactions
-
- - `assert-table-uuid` - the table UUID must match the requirement's `uuid`
-
- - `assert-ref-snapshot-id` - the table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`; if `snapshot-id` is `null` or missing, the ref must not already exist
-
- - `assert-last-assigned-field-id` - the table's last assigned column id must match the requirement's `last-assigned-field-id`
-
- - `assert-current-schema-id` - the table's current schema id must match the requirement's `current-schema-id`
-
- - `assert-last-assigned-partition-id` - the table's last assigned partition id must match the requirement's `last-assigned-partition-id`
-
- - `assert-default-spec-id` - the table's default spec id must match the requirement's `default-spec-id`
-
- - `assert-default-sort-order-id` - the table's default sort order id must match the requirement's `default-sort-order-id`
+ AssertCreate:
type: object
+ description: The table must not already exist; used for create transactions
required:
- type
properties:
type:
type: string
- enum:
- - assert-create
- - assert-table-uuid
- - assert-ref-snapshot-id
- - assert-last-assigned-field-id
- - assert-current-schema-id
- - assert-last-assigned-partition-id
- - assert-default-spec-id
- - assert-default-sort-order-id
- ref:
+ enum: ["assert-create"]
+
+ AssertTableUUID:
+ description: The table UUID must match the requirement's `uuid`
+ required:
+ - type
+ - uuid
+ properties:
+ type:
type: string
+ enum: ["assert-table-uuid"]
uuid:
type: string
+
+ AssertRefSnapshotId:
+ description:
+ The table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`;
+ if `snapshot-id` is `null` or missing, the ref must not already exist
+ required:
+ - type
+ - ref
+ - snapshot-id
+ properties:
+ type:
+ type: string
+ enum: [ "assert-ref-snapshot-id" ]
+ ref:
+ type: string
snapshot-id:
type: integer
format: int64
- last-assigned-field-id:
+
+ AssertLastAssignedFieldId:
+ description:
+ The table's last assigned column id must match the requirement's `last-assigned-field-id`
+ required:
+ - type
+ - last-assigned-partition-id
Review Comment:
```suggestion
- last-assigned-field-id
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Open-API: Refactor TableRequirements [iceberg]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#discussion_r1376457242
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1824,58 +1824,146 @@ components:
- $ref: '#/components/schemas/SetPropertiesUpdate'
- $ref: '#/components/schemas/RemovePropertiesUpdate'
- TableRequirement:
Review Comment:
Yes, I don't recall this isn't the case.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] Fokko commented on pull request #7710: Open-API: Refactor TableRequirements
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #7710:
URL: https://github.com/apache/iceberg/pull/7710#issuecomment-1575557040
Alright, now I have it right:
```python
class TableRequirement(BaseModel):
name: Optional[str] = None
class AssertCreate(TableRequirement):
"""
The table must not already exist; used for create transactions
"""
type: Literal['assert-create']
class AssertTableUUID(TableRequirement):
"""
The table UUID must match the requirement's `uuid`
"""
type: Literal['assert-table-uuid']
uuid: str
class AssertRefSnapshotId(TableRequirement):
"""
The table branch or tag identified by the requirement's `ref` must reference the requirement's `snapshot-id`; if `snapshot-id` is `null` or missing, the ref must not already exist
"""
type: Literal['assert-ref-snapshot-id']
ref: str
snapshot_id: int = Field(..., alias='snapshot-id')
class AssertLastAssignedFieldId(TableRequirement):
"""
The table's last assigned column id must match the requirement's `last-assigned-field-id`
"""
type: Literal['assert-last-assigned-field-id']
last_assigned_field_id: int = Field(..., alias='last-assigned-field-id')
class AssertCurrentSchemaId(TableRequirement):
"""
The table's current schema id must match the requirement's `current-schema-id`
"""
type: Literal['assert-current-schema-id']
current_schema_id: int = Field(..., alias='current-schema-id')
class AssertLastAssignedPartitionId(TableRequirement):
"""
The table's last assigned partition id must match the requirement's `last-assigned-partition-id`
"""
type: Literal['assert-last-assigned-partition-id']
last_assigned_partition_id: int = Field(..., alias='last-assigned-partition-id')
class AssertDefaultSpecId(TableRequirement):
"""
The table's default spec id must match the requirement's `default-spec-id`
"""
type: Literal['assert-default-spec-id']
default_spec_id: int = Field(..., alias='default-spec-id')
class AssertDefaultSortOrderId(TableRequirement):
"""
The table's default sort order id must match the requirement's `default-sort-order-id`
"""
type: Literal['assert-default-sort-order-id']
default_sort_order_id: int = Field(..., alias='default-sort-order-id')
```
The inheritance tree now checks out.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Open-API: Refactor TableRequirements [iceberg]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #7710:
URL: https://github.com/apache/iceberg/pull/7710
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org