nryanov opened a new issue, #13263:
URL: https://github.com/apache/iceberg/issues/13263
### Proposed Change
Not sure that i've chose a category correctly but this issue with spec
doesn't seem like a bug.
### Motivation
Current generated server-side code seems to be hard to use because existing
components for TableUpdate & ViewUpdate do not use it's discriminator as they
should.
### Current behavior
Currently in openapi spec there is three components for updates:
- table update
- view update
- base update
```
TableUpdate:
anyOf:
- $ref: '#/components/schemas/AssignUUIDUpdate'
- ...
ViewUpdate:
anyOf:
- $ref: '#/components/schemas/AssignUUIDUpdate'
- ...
BaseUpdate:
discriminator:
propertyName: action
mapping:
assign-uuid: '#/components/schemas/AssignUUIDUpdate'
upgrade-format-version:
'#/components/schemas/UpgradeFormatVersionUpdate'
add-schema: '#/components/schemas/AddSchemaUpdate'
set-current-schema: '#/components/schemas/SetCurrentSchemaUpdate'
add-spec: '#/components/schemas/AddPartitionSpecUpdate'
set-default-spec: '#/components/schemas/SetDefaultSpecUpdate'
add-sort-order: '#/components/schemas/AddSortOrderUpdate'
set-default-sort-order:
'#/components/schemas/SetDefaultSortOrderUpdate'
add-snapshot: '#/components/schemas/AddSnapshotUpdate'
set-snapshot-ref: '#/components/schemas/SetSnapshotRefUpdate'
remove-snapshots: '#/components/schemas/RemoveSnapshotsUpdate'
remove-snapshot-ref: '#/components/schemas/RemoveSnapshotRefUpdate'
set-location: '#/components/schemas/SetLocationUpdate'
set-properties: '#/components/schemas/SetPropertiesUpdate'
remove-properties: '#/components/schemas/RemovePropertiesUpdate'
add-view-version: '#/components/schemas/AddViewVersionUpdate'
set-current-view-version:
'#/components/schemas/SetCurrentViewVersionUpdate'
set-statistics: '#/components/schemas/SetStatisticsUpdate'
remove-statistics: '#/components/schemas/RemoveStatisticsUpdate'
set-partition-statistics:
'#/components/schemas/SetPartitionStatisticsUpdate'
remove-partition-statistics:
'#/components/schemas/RemovePartitionStatisticsUpdate'
remove-partition-specs:
'#/components/schemas/RemovePartitionSpecsUpdate'
enable-row-lineage: '#/components/schemas/EnableRowLineageUpdate'
type: object
required:
- action
properties:
action:
type: string
```
When server side code is generated using this spec then, for example,
TableUpdate class will look like this:
```
@org.eclipse.microprofile.openapi.annotations.media.Schema(description="")
@JsonTypeName("TableUpdate")
@jakarta.annotation.Generated(value =
"org.openapitools.codegen.languages.JavaJAXRSSpecServerCodegen", comments =
"Generator version: 7.12.0")
public class IcebergTableUpdateDto {
private String action;
private Integer formatVersion;
private IcebergSchemaDto schema;
private Integer lastColumnId;
private Integer schemaId;
private IcebergPartitionSpecDto spec;
private Integer specId;
private IcebergSortOrderDto sortOrder;
private Integer sortOrderId;
private IcebergSnapshotDto snapshot;
private String refName;
private TypeEnum type;
private Long snapshotId;
private Long maxRefAgeMs;
private Long maxSnapshotAgeMs;
private Integer minSnapshotsToKeep;
private List<Long> snapshotIds = new ArrayList<>();
private String location;
private Map<String, String> updates = new HashMap<>();
private List<String> removals = new ArrayList<>();
private IcebergStatisticsFileDto statistics;
private List<Integer> specIds = new ArrayList<>();
/*
other methods
*/
}
```
The issue here is that it seems not possible to use this class to correctly
handle TableUpdates in `CommitTableRequest` because this class doesn't use
discriminator (`action` field as it should).
In the meantime, there is `TableRequirement` (and `ViewRequirement`) which
looks like:
```
TableRequirement:
type: object
discriminator:
propertyName: type
mapping:
assert-create: '#/components/schemas/AssertCreate'
assert-table-uuid: '#/components/schemas/AssertTableUUID'
assert-ref-snapshot-id: '#/components/schemas/AssertRefSnapshotId'
assert-last-assigned-field-id:
'#/components/schemas/AssertLastAssignedFieldId'
assert-current-schema-id:
'#/components/schemas/AssertCurrentSchemaId'
assert-last-assigned-partition-id:
'#/components/schemas/AssertLastAssignedPartitionId'
assert-default-spec-id: '#/components/schemas/AssertDefaultSpecId'
assert-default-sort-order-id:
'#/components/schemas/AssertDefaultSortOrderId'
properties:
type:
type: string
required:
- type
```
and generated class will be:
```
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include =
JsonTypeInfo.As.PROPERTY, property = "type", visible = true)
@JsonSubTypes({
@JsonSubTypes.Type(value = IcebergAssertCreateDto.class, name =
"assert-create"),
@JsonSubTypes.Type(value = IcebergAssertCurrentSchemaIdDto.class, name =
"assert-current-schema-id"),
@JsonSubTypes.Type(value = IcebergAssertDefaultSortOrderIdDto.class, name
= "assert-default-sort-order-id"),
@JsonSubTypes.Type(value = IcebergAssertDefaultSpecIdDto.class, name =
"assert-default-spec-id"),
@JsonSubTypes.Type(value = IcebergAssertLastAssignedFieldIdDto.class, name
= "assert-last-assigned-field-id"),
@JsonSubTypes.Type(value = IcebergAssertLastAssignedPartitionIdDto.class,
name = "assert-last-assigned-partition-id"),
@JsonSubTypes.Type(value = IcebergAssertRefSnapshotIdDto.class, name =
"assert-ref-snapshot-id"),
@JsonSubTypes.Type(value = IcebergAssertTableUUIDDto.class, name =
"assert-table-uuid"),
})
@org.eclipse.microprofile.openapi.annotations.media.Schema(description="")
@JsonTypeName("TableRequirement")
@jakarta.annotation.Generated(value =
"org.openapitools.codegen.languages.JavaJAXRSSpecServerCodegen", comments =
"Generator version: 7.12.0")
public class IcebergTableRequirementDto {
private String type;
/*
other stuff
*/
}
```
And there will be some subclasses of `IcebergTableRequirementDto` which can
be determined using `type` field.
### Desired behavior
Desired behavior is to get almost the same result as if it was
`TableRequirement`. If this part of spec will be changed then it will give
these benefits:
- Generated classes will be possible to use in a generic way to determine
which action should be applied
- ViewUpdate and TableUpdate will not share same parts like it is now in
BaseUpdate (there are components which applied either to view or table only)
Final result for the `TableUpdate` may look like this:
```
TableUpdate:
discriminator:
propertyName: action
mapping:
assign-uuid: '#/components/schemas/AssignTableUUIDUpdate'
... // other update types
type: object
required:
- action
properties:
action:
type: string
```
and the result class:
```
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include =
JsonTypeInfo.As.PROPERTY, property = "action", visible = true)
@JsonSubTypes({
@JsonSubTypes.Type(value = IcebergAddTableSchemaUpdateDto.class, name =
"add-schema"),
@JsonSubTypes.Type(value = IcebergAddTableSnapshotUpdateDto.class, name =
"add-snapshot"),
@JsonSubTypes.Type(value = IcebergAddTableSortOrderUpdateDto.class, name =
"add-sort-order"),
...
})
@org.eclipse.microprofile.openapi.annotations.media.Schema(description="")
@JsonTypeName("TableUpdate")
@jakarta.annotation.Generated(value =
"org.openapitools.codegen.languages.JavaJAXRSSpecServerCodegen", comments =
"Generator version: 7.12.0")
public class IcebergTableUpdateDto {
private String action;
/*
other stuff
*/
}
```
### Possible implementation
Implementation is simple enough in terms of changes:
- Remove BaseUpdate
- Refactor ViewUpdate & TableUpdate using separate components for each action
In the end, final endpoints which use these components, will not be affected
in terms of structure of these endpoints, classes which they use and naming of
these classes. Only internal structure of TableUpdate/ViewUpdate will be
affected.
### Breaking changes/incompatibilities
Possibly it may affect already existing implementations of rest catalog
during update spec in the next releases / iterations
### How to reproduce
Currently i were able to reproduce it using:
- "org.openapi.generator" version "7.12.0"
- iceberg spec: 1.7.1 / 1.8.1 / 1.9.1
- Generator settings:
```
openApiGenerate {
var properties = new HashMap<String, String>()
properties.put("supportAsync", "false")
properties.put("useMutiny", "false")
properties.put("useMicroProfileOpenAPIAnnotations", "true")
properties.put("useBeanValidation", "false")
properties.put("hideGenerationTimestamp", "true")
properties.put("library", "quarkus")
properties.put("dateLibrary", "java8")
properties.put("interfaceOnly", "true")
properties.put("returnResponse", "false")
properties.put("useOneOfInterfaces", "false") // true will not help in
this case
properties.put("generateBuilders", "true")
properties.put("generatePom", "false")
properties.put("apiPackage", "iceberg.rest.api")
properties.put("modelPackage", "iceberg.rest.api.dto")
properties.put("modelNamePrefix", "Iceberg")
properties.put("modelNameSuffix", "Dto")
properties.put("generateApiTests", "false")
properties.put("useJakartaEe", "true")
properties.put("generateConstructorWithAllArgs", "true")
properties.put("generateJsonCreator", "false")
properties.put("cleanupOutput", "true")
generatorName.set("jaxrs-spec")
additionalProperties.putAll(properties)
inputSpec.set("$projectDir/src/main/resources/openapi/openapi.yaml")
outputDir.set("$buildDir/generated")
}
```
### Proposal document
_No response_
### Specifications
- [ ] Table
- [ ] View
- [x] REST
- [ ] Puffin
- [ ] Encryption
- [ ] Other
--
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-unsubscr...@iceberg.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
