kevinjqliu opened a new issue, #112: URL: https://github.com/apache/terraform-provider-iceberg/issues/112
## Summary This epic tracks **7 correctness issues** and **9 feature gaps**. ## Shared reproduction setup ```bash docker compose -f dev/docker-compose.yml up -d export ICEBERG_CATALOG_URI=http://localhost:8181 ``` The audited fixture is Iceberg REST `1.10.1` backed by MinIO. Provider-facing cases can be reproduced with a focused Go test under `internal/provider`, a Terraform apply/read against the fixture, or a raw REST-created table followed by provider import/read. After testing: ```bash docker compose -f dev/docker-compose.yml down ``` ## Correctness issues ### Schema and partition identity - [ ] **Reject reserved schema field IDs** **Reproduce:** Create a table, then update its schema with a new field whose ID is `2147483448`. **Observed:** The provider and REST fixture accept and persist the reserved ID. **Expected:** Validation rejects IDs reserved by the Iceberg specification before commit. Add a regression test covering the reserved range. - [ ] **Prevent reuse of dropped schema field IDs** **Reproduce:** Create fields with IDs `1` and `2`; drop field `2`; then add a different field using ID `2`. **Observed:** The new field is committed with the historical ID, changing that ID's meaning across schemas. **Expected:** Field IDs remain unique for the table's lifetime. Validate against all historical schemas and `last-column-id`. - [ ] **Prevent moving a field ID between parent structs** **Reproduce:** Create structs `left` and `right` with child field ID `3` under `left`; update the schema so ID `3` appears under `right`. **Observed:** The provider commits the reparented field. **Expected:** An existing field ID cannot change its parent path. Reject the update and cover nested struct, list, and map identities. - [ ] **Prevent reuse of historical partition field IDs** **Reproduce:** Commit partition field ID `1000` for one source/transform, replace the spec, then use ID `1000` for a different source or transform. **Observed:** The provider commits the reused ID. **Expected:** Partition field IDs are not reassigned to a different partition field. Validate all historical specs and `last-partition-id`. ### Commit concurrency - [ ] **Add complete optimistic-concurrency requirements** **Reproduce:** Capture an update request with a mock/proxy catalog, or pause after Terraform reads metadata, commit a schema/spec change with a second client, and then resume the Terraform update. **Observed:** The request asserts only the table UUID. Schema and partition replacements omit requirements for the current schema, default spec, and relevant last-assigned IDs. **Expected:** Stale updates fail rather than overwrite concurrent changes or allocate colliding IDs. Use the same requirements as the corresponding iceberg-go update builders and add conflict tests. ### Metadata preservation - [ ] **Preserve identifier field IDs during unrelated schema updates** **Reproduce:** Create a table through REST with `identifier-field-ids:[1]`; import it; then make an unrelated schema change such as adding an optional field. **Observed:** The provider's replacement schema has no identifier IDs, so the committed current schema silently clears them. **Expected:** Identifier IDs are represented in state/configuration and remain unchanged unless explicitly updated. ## Feature gaps ### Terraform schema ID support - [ ] **Support explicit schema and field IDs without inconsistent results** **Reproduce:** Apply a table with schema ID `7` and field IDs `10` and `20`: ```hcl schema = { id = 7 fields = [ { id = 10, name = "first", type = "string", required = false }, { id = 20, name = "second", type = "string", required = false } ] } ``` **Observed:** REST assigns schema ID `0` and field IDs `1,2`; apply fails with `Provider produced inconsistent result after apply`. The remote table may already exist. **Expected:** Either preserve supported explicit IDs or make them computed/plan-normalized so apply succeeds and state matches the remote table. ### Nested schema support - [ ] **Support nested collection child types** **Reproduce:** Create a valid table through REST with a field such as `list<struct<id: long>>`, then read it through the table data source or import it as a resource. **Observed:** Conversion attempts to decode the element type object as a string and fails. **Expected:** Struct, list, and map child types use a recursive representation and round trip without loss. - [ ] **Remove the hard-coded four-level struct nesting limit** **Reproduce:** Create a valid schema with five nested structs and read/import it through the provider. **Observed:** Terraform object conversion fails beyond the statically expanded nesting depth. **Expected:** Any nesting depth accepted by Iceberg is representable, subject only to practical framework limits. Add a test deeper than four levels. ## Format-v3 and forward compatibility ### Correctness issues - [ ] **Enforce required-field default rules during schema evolution** **Reproduce:** Use Terraform to add a required field to a format-v3 table without an initial default. **Observed:** The invalid required field is accepted and persisted. **Expected:** Reject required-field additions that do not provide the defaults required by the Iceberg specification. ### Feature gaps - [ ] **Support multi-source partition fields** **Reproduce:** In a focused conversion test, decode a v3 partition field containing `"source-ids":[1,2]`, convert it with `FromIceberg`, and inspect the Terraform model. **Observed:** `source_ids` becomes `[1]` because conversion uses the singular source accessor. **Expected:** Round trips preserve `[1,2]` exactly. Add raw-metadata and Terraform-state tests. - [ ] **Support multi-source sort fields** **Reproduce:** Convert a v3 sort field containing multiple source IDs into the Terraform model. **Observed:** The singular `source_id` model decodes the field as `0`. **Expected:** Model sort sources as a list and preserve all IDs through read, import, plan, and apply. - [ ] **Represent format-v3 field defaults** **Reproduce:** Create a format-v3 table through REST with `initial-default` and `write-default`, then read it through the provider. **Observed:** Defaults cannot be expressed or observed in the Terraform schema. Existing server-side defaults are not erased merely by reading the table. **Expected:** Defaults round trip through resource and data-source state and can be configured for supported schema changes. - [ ] **Allow the valid `date` to `timestamp` promotion in v3** **Reproduce:** Import a format-v3 table containing a `date` field and update that field's type to `timestamp`. **Observed:** Provider schema-evolution validation rejects the promotion. **Expected:** Accept promotions allowed by the table's format version while continuing to reject unsafe changes. - [ ] **Read tables containing valid unknown transforms** **Reproduce:** Create a table through REST whose partition spec contains a syntactically valid transform unknown to this provider, then read/import it. **Observed:** Metadata conversion fails, making the table unmanageable through the provider. **Expected:** Readers preserve unknown transforms and expose the table without pretending they can write with those transforms. Unsupported writes should produce a targeted diagnostic. - [ ] **Represent v3 `geometry` and `geography` field types** **Reproduce:** Create a format-v3 table through REST with `geometry` and `geography` fields, then read/import it. **Observed:** The provider cannot convert the valid schema into Terraform state. **Expected:** Both types round trip through resource and data-source state, with v3-aware validation. ## Completion criteria - Every checklist item has a focused regression test. - Resource create, read, import, update, and data-source reads are lossless for supported metadata. - Invalid identity/evolution updates fail before a catalog commit. - Concurrent schema and partition updates fail with a requirement conflict rather than overwrite newer metadata. - `go test ./...` and all Terraform acceptance tests pass against the supported REST fixture. ## Out of scope and rejected candidates - Decimal-scale promotion is tracked in [apache/iceberg-go#1480](https://github.com/apache/iceberg-go/pull/1480) and is not part of this epic. - REST `1.10.1` correctly rejected the tested transform/type mismatch, missing sort source, unary-transform arity violation, and zero bucket size; these are not listed as persisted defects. - Existing acceptance coverage confirmed partition and sort-order removal; those earlier candidates were false positives. ## Appendix: Validation tests The regression cases in [correctness_repro_test.go](https://github.com/kevinjqliu/iceberg-terraform/blob/iceberg-correctness-repros/internal/provider/correctness_repro_test.go) assert the expected fixed behavior and intentionally fail against the current provider. They are excluded from normal builds by the `repro` build tag. Every issue has a corresponding validation test: | Issue | Validation test | Reproduced current behavior | |---|---|---| | Reject reserved schema field IDs | `TestReproRejectReservedSchemaFieldIDs` | A schema update using ID `2147483448` produces commit updates without a diagnostic. | | Prevent reuse of dropped schema field IDs | `TestReproRejectDroppedSchemaFieldIDReuse` | ID `2` is accepted for a new field after the historical field using ID `2` is dropped. | | Prevent moving a field ID between parent structs | `TestReproRejectReparentedFieldIDs` | Struct child, list element, and map key/value IDs can all move between parents. | | Prevent reuse of historical partition field IDs | `TestReproRejectHistoricalPartitionFieldIDReuse` | Reassigning historical partition ID `1000` produces commit updates without a diagnostic. | | Add complete optimistic-concurrency requirements | `TestReproCompleteOptimisticConcurrencyRequirements` | Stale requirements accept both concurrent schema and partition metadata changes. | | Preserve identifier field IDs during unrelated schema updates | `TestReproPreserveIdentifierFieldIDs` | Schema conversion changes identifier field IDs from `[1]` to `[]`. | | Support explicit schema and field IDs without inconsistent results | `TestAccReproPreserveExplicitSchemaAndFieldIDs` | REST rewrites schema ID `7` to `0` and field IDs `10,20` to `1,2`, causing an inconsistent result. | | Support nested collection child types | `TestReproReadNestedCollectionChildTypes` | Both list-of-struct and map-with-struct-value decoding fail because child types are strings. | | Remove the hard-coded four-level struct nesting limit | `TestReproReadFiveNestedStructLevels` | Terraform object conversion fails on the fifth nested struct level. | | Enforce required-field default rules during schema evolution | `TestReproRejectRequiredFieldWithoutDefaults` | A format-v3 required field without defaults produces commit updates without a diagnostic. | | Support multi-source partition fields | `TestReproPreserveMultiSourcePartitionField` | Partition source IDs `[1,2]` become `[1]`. | | Support multi-source sort fields | `TestReproPreserveMultiSourceSortField` | Sort source IDs `[1,2]` become `[0]`. | | Represent format-v3 field defaults | `TestReproPreserveFormatV3FieldDefaults` | `initial-default` and `write-default` both become unset. | | Allow the valid `date` to `timestamp` promotion in v3 | `TestReproAllowDateToTimestampPromotionInV3` | A format-v3 table update rejects the valid promotion. | | Read tables containing valid unknown transforms | `TestReproReadUnknownPartitionTransform` | Metadata parsing rejects an unrecognized transform. | | Represent v3 `geometry` and `geography` field types | `TestReproReadFormatV3GeospatialTypes` | Metadata parsing rejects both valid v3 types. | Run the 15 in-process validation test functions with: ```bash go test -tags repro ./internal/provider -run '^TestRepro' -count=1 ``` The explicit schema/field ID validation test requires the shared REST fixture: ```bash docker compose -f dev/docker-compose.yml up -d TF_ACC=1 ICEBERG_CATALOG_URI=http://localhost:8181 \ go test -tags repro ./internal/provider \ -run '^TestAccReproPreserveExplicitSchemaAndFieldIDs$' -count=1 -v docker compose -f dev/docker-compose.yml down ``` -- 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]
