rambleraptor commented on code in PR #29:
URL: https://github.com/apache/iceberg-terraform/pull/29#discussion_r3018893326


##########
internal/provider/table_schema.go:
##########
@@ -392,16 +427,17 @@ func (s *icebergTableSchemaFieldStructProperties) 
UnmarshalJSON(b []byte) error
 
 func marshalFieldJSON(id types.Int64, name, typeStr string, required bool, doc 
*string, listProps, mapProps, structProps interface{}) ([]byte, error) {
        type Field struct {
-               ID       int64       `json:"id"`
+               ID       *int64      `json:"id,omitempty"`

Review Comment:
   We're not being consistent about how we're treating IDs. 
   
   Some IDs should be managed by Terraform (not user-settable, Terraform will 
store these values + increment them on updates). Some should be managed by 
users.
   
   Let's just standardize this. Here's what I'm thinking. 
   
   - `schema-id` - Terraform managed.
   - `field-id` - User managed. This isn't ideal, but the ID is the primary key 
of a field. If you try to rename a field (without specifying the ID), Terraform 
will become very confused.
   - `partition-specs.spec-id` - Terraform managed.
   - `sort_orders.order-id` - Terraform managed.
   - `partition-specs.source-id`. User managed. These are references to a field.
   - `partition-specs.field-id`. User managed. These are references to a field.



-- 
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]

Reply via email to