ebyhr commented on code in PR #17822:
URL: https://github.com/apache/iceberg/pull/17822#discussion_r3870057370


##########
format/spec.md:
##########
@@ -654,6 +655,129 @@ Sorting floating-point numbers should produce the 
following behavior: `-NaN` < `
 
 A data or delete file is associated with a sort order by the sort order's id 
within [a manifest](#manifests). Therefore, the table must declare all the sort 
orders for lookup. A table could also be configured with a default sort order 
id, indicating how the new data should be sorted by default. Writers should use 
this default sort order to sort the data on write, but are not required to if 
the default order is prohibitively expensive, as it would be for streaming 
writes.
 
+### Constraints
+
+A **constraint** declares a property that a table's rows are expected to 
satisfy. A constraint's definition is stored in table metadata. Whether a 
constraint holds is recorded for each snapshot, see [Constraint 
Validation](#constraint-validation).
+
+Iceberg does not evaluate constraints. Enforcement and validation are 
performed by engines that write to a table. Iceberg stores constraint 
definitions and records the status that a writer reports for a commit without 
verifying it.
+
+Constraints are added in v4 and are not supported in v3 or earlier.
+
+Three constraint types are defined:
+
+* `check` -- every row must satisfy a predicate
+* `unique` -- the values of a set of fields must be distinct across all rows; 
a null value is not equal to any other null value, so more than one row may be 
null
+* `primary-key` -- the values of a set of fields must be distinct across all 
rows and must not be null
+
+Constraints are stored separately from schemas because the two evolve 
independently. Every constraint references the fields that it applies to by 
field ID, so a constraint continues to apply to the same columns after a column 
is renamed or reordered.
+
+A required field in a schema expresses `NOT NULL`. It is not represented as a 
constraint.
+
+#### Constraint Fields
+
+A constraint consists of the following fields:
+
+| Requirement | Field name                | Type      | Description |
+|-------------|---------------------------|-----------|-------------|
+| _required_ | **`constraint-id`**       | `int`     | ID of the constraint; 
unique within the table |
+| _required_ | **`type`**                | `string`  | The constraint type: 
`check`, `unique`, or `primary-key` |
+| _required_ | **`name`**                | `string`  | A name for the 
constraint that is unique within the table. Names are for human consumption and 
must not be used to identify a constraint in metadata |
+| _required_ | **`enforced`**            | `boolean` | Whether writers must 
verify that the rows they add satisfy the constraint |
+| _required_ | **`timestamp-ms`**        | `long`    | Timestamp in 
milliseconds from the unix epoch when the constraint was created or last 
modified. The timestamp is informational and must not be used to determine 
whether a constraint applies to a snapshot or whether it holds |
+| _required_ | **`constraint-metadata`** | `struct`  | Defines what the 
constraint requires; its fields depend on the constraint `type` (see below) |
+
+`constraint-metadata` for a `check` constraint consists of:
+
+| Requirement | Field name       | Type         | Description |
+|-------------|------------------|--------------|-------------|
+| _required_ | **`expression`** | `expression` | A predicate that every row 
must satisfy, see [Check Constraint Expressions](#check-constraint-expressions) 
|
+
+`constraint-metadata` for a `unique` or `primary-key` constraint consists of:
+
+| Requirement | Field name      | Type        | Description |
+|-------------|-----------------|-------------|-------------|
+| _required_ | **`field-ids`** | `list<int>` | A list of field IDs that the 
constraint applies to |
+
+Each type carries only the metadata that it requires: a `check` constraint 
must not declare `field-ids` and a `unique` or `primary-key` constraint must 
not declare an `expression`. This keeps a single source of truth for the fields 
that a constraint references.
+
+The `field-ids` of a `unique` or `primary-key` constraint must reference 
primitive fields that are either top-level fields or nested in required 
structs, and must not reference fields within a `list` or a `map`. These are 
the same restrictions that apply to [identifier fields](#identifier-field-ids).
+
+When a constraint is `enforced`, writers must verify that the rows they add 
satisfy the constraint and must fail the write if they do not. A writer that 
cannot verify an enforced constraint must reject writes to the table rather 
than add rows that have not been verified. When a constraint is not enforced, 
writers are not required to verify the rows they add.
+
+Whether to trust a constraint that is not enforced is left to engines and is 
not tracked in table metadata.
+
+Constraint IDs are assigned from the table's `last-constraint-id`, which is 
treated as 0 when it is not present. Writers must assign a new constraint an ID 
that is higher than the table's current `last-constraint-id` and must update 
`last-constraint-id` to the highest assigned ID. Constraint IDs must not be 
reused after the constraint that used an ID is removed, because retained 
snapshots may still reference the removed ID. Readers must not assume that 
every `constraint-id` referenced by a snapshot is present in `constraints`.
+
+#### Check Constraint Expressions
+
+The `expression` of a `check` constraint is serialized as described in the 
[Iceberg expressions spec](expressions-spec.md) and must use ID references so 
that it remains bound to the same fields when columns are renamed or reordered.

Review Comment:
   Should the spec explicitly state that dropping a field referenced by a 
constraint must fail? How about adding: 
   ```
   A field referenced by a constraint must not be removed; writers must reject 
a schema change that drops such a field until the constraint is first removed.
   ```



##########
format/spec.md:
##########
@@ -654,6 +655,129 @@ Sorting floating-point numbers should produce the 
following behavior: `-NaN` < `
 
 A data or delete file is associated with a sort order by the sort order's id 
within [a manifest](#manifests). Therefore, the table must declare all the sort 
orders for lookup. A table could also be configured with a default sort order 
id, indicating how the new data should be sorted by default. Writers should use 
this default sort order to sort the data on write, but are not required to if 
the default order is prohibitively expensive, as it would be for streaming 
writes.
 
+### Constraints
+
+A **constraint** declares a property that a table's rows are expected to 
satisfy. A constraint's definition is stored in table metadata. Whether a 
constraint holds is recorded for each snapshot, see [Constraint 
Validation](#constraint-validation).
+
+Iceberg does not evaluate constraints. Enforcement and validation are 
performed by engines that write to a table. Iceberg stores constraint 
definitions and records the status that a writer reports for a commit without 
verifying it.
+
+Constraints are added in v4 and are not supported in v3 or earlier.
+
+Three constraint types are defined:
+
+* `check` -- every row must satisfy a predicate
+* `unique` -- the values of a set of fields must be distinct across all rows; 
a null value is not equal to any other null value, so more than one row may be 
null
+* `primary-key` -- the values of a set of fields must be distinct across all 
rows and must not be null
+
+Constraints are stored separately from schemas because the two evolve 
independently. Every constraint references the fields that it applies to by 
field ID, so a constraint continues to apply to the same columns after a column 
is renamed or reordered.
+
+A required field in a schema expresses `NOT NULL`. It is not represented as a 
constraint.
+
+#### Constraint Fields
+
+A constraint consists of the following fields:
+
+| Requirement | Field name                | Type      | Description |
+|-------------|---------------------------|-----------|-------------|
+| _required_ | **`constraint-id`**       | `int`     | ID of the constraint; 
unique within the table |
+| _required_ | **`type`**                | `string`  | The constraint type: 
`check`, `unique`, or `primary-key` |
+| _required_ | **`name`**                | `string`  | A name for the 
constraint that is unique within the table. Names are for human consumption and 
must not be used to identify a constraint in metadata |
+| _required_ | **`enforced`**            | `boolean` | Whether writers must 
verify that the rows they add satisfy the constraint |
+| _required_ | **`timestamp-ms`**        | `long`    | Timestamp in 
milliseconds from the unix epoch when the constraint was created or last 
modified. The timestamp is informational and must not be used to determine 
whether a constraint applies to a snapshot or whether it holds |
+| _required_ | **`constraint-metadata`** | `struct`  | Defines what the 
constraint requires; its fields depend on the constraint `type` (see below) |
+
+`constraint-metadata` for a `check` constraint consists of:
+
+| Requirement | Field name       | Type         | Description |
+|-------------|------------------|--------------|-------------|
+| _required_ | **`expression`** | `expression` | A predicate that every row 
must satisfy, see [Check Constraint Expressions](#check-constraint-expressions) 
|
+
+`constraint-metadata` for a `unique` or `primary-key` constraint consists of:
+
+| Requirement | Field name      | Type        | Description |
+|-------------|-----------------|-------------|-------------|
+| _required_ | **`field-ids`** | `list<int>` | A list of field IDs that the 
constraint applies to |
+
+Each type carries only the metadata that it requires: a `check` constraint 
must not declare `field-ids` and a `unique` or `primary-key` constraint must 
not declare an `expression`. This keeps a single source of truth for the fields 
that a constraint references.
+
+The `field-ids` of a `unique` or `primary-key` constraint must reference 
primitive fields that are either top-level fields or nested in required 
structs, and must not reference fields within a `list` or a `map`. These are 
the same restrictions that apply to [identifier fields](#identifier-field-ids).
+
+When a constraint is `enforced`, writers must verify that the rows they add 
satisfy the constraint and must fail the write if they do not. A writer that 
cannot verify an enforced constraint must reject writes to the table rather 
than add rows that have not been verified. When a constraint is not enforced, 
writers are not required to verify the rows they add.
+
+Whether to trust a constraint that is not enforced is left to engines and is 
not tracked in table metadata.
+
+Constraint IDs are assigned from the table's `last-constraint-id`, which is 
treated as 0 when it is not present. Writers must assign a new constraint an ID 
that is higher than the table's current `last-constraint-id` and must update 
`last-constraint-id` to the highest assigned ID. Constraint IDs must not be 
reused after the constraint that used an ID is removed, because retained 
snapshots may still reference the removed ID. Readers must not assume that 
every `constraint-id` referenced by a snapshot is present in `constraints`.
+
+#### Check Constraint Expressions
+
+The `expression` of a `check` constraint is serialized as described in the 
[Iceberg expressions spec](expressions-spec.md) and must use ID references so 
that it remains bound to the same fields when columns are renamed or reordered.
+
+A check expression must be a deterministic predicate over a single row. 
Expressions that depend on more than one row, such as aggregates and window 
functions, and expressions that depend on another table, such as subqueries, 
must not be used.

Review Comment:
   >  check expression must be a deterministic
   
   How is a writer supposed to verify this? The [Iceberg expressions 
spec](https://github.com/apache/iceberg/blob/main/format/expressions-spec.md) 
doesn't appear to label functions as deterministic or non-deterministic, nor 
does it prohibit time/session functions like CURRENT_TIMESTAMP explicitly. 



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