Spec: v4 constraints - #17822
Spec: v4 constraints#17822huaxingao wants to merge 16 commits into
Conversation
github-actions
Bot
added
the
Specification
|
|
||
| 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. |
There was a problem hiding this comment.
I think this statement should go first. It's a little odd to interject this between the description and definition.
There was a problem hiding this comment.
Moved Constraints are added in v4 and are not supported in v3 or earlier. to the beginning.
| * `unique` -- the non-null values of a set of fields must be distinct across all rows; more than one row may have a null value | ||
| * `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. |
There was a problem hiding this comment.
| 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. | |
| Constraints are stored separately from schemas because they span multiple fields and 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. |
|
|
||
| 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. |
There was a problem hiding this comment.
I'm not sure I understand the reason for including this statement. Required fields are covered elsewhere. This also doesn't prevent a NOT NULL constraint being defined as an expression. This feels like unnecessary commentary.
There was a problem hiding this comment.
deleted this sentence.
|
|
||
| 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. |
There was a problem hiding this comment.
| 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. | |
| 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. When a constraint is not enforced, writers are not required to verify the rows they add. |
There was a problem hiding this comment.
A writer that cannot verify an enforced constraint must reject writes to the table.
Does this include commits that add no new rows, like deletes or compaction? With the shorter wording, I read it as blocking those too for a writer that can't evaluate a check expression. Was that the intent?
There was a problem hiding this comment.
Good catch, that was not the intent. A writer that cannot evaluate a check should still be able to run a delete or a compaction, since those add no rows and cannot create a violation.
I will change to
A writer that cannot verify an enforced constraint must reject writes that add rows to the table.
mbutrovich
left a comment
mbutrovich
left a comment
There was a problem hiding this comment.
Thanks @huaxingao. Coming at this from an engine implementer's standpoint, my questions are mostly about places where I think two implementations could read the text differently.
| Three constraint types are defined: | ||
|
|
||
| * `check` -- every row must satisfy a predicate | ||
| * `unique` -- the non-null values of a set of fields must be distinct across all rows; more than one row may have a null value |
There was a problem hiding this comment.
I know we discussed nulls a bit in the calls about this, but going to revisit the topic :)
For a multi-column key, is a row exempt when any key field is null, or only when all of them are? For example, with a key on (a, b), can two rows both have a = 1 and b = null? SQL UNIQUE allows it, but I could also read "the non-null values of a set of fields" as comparing a alone, which would make those rows duplicates.
Which equality should keys use for floats? As far as I can tell, the spec handles signed zero differently in different places. Partition equality keeps -0.0 and 0.0 distinct, while the hash definition maps -0.0 to 0.0.
There was a problem hiding this comment.
Regarding equality I'm in favor of equality by value not by bit-representation.
Is in line with the other DBs:
- Postgres
- MySQL
- Sql Server
- Postgres
There was a problem hiding this comment.
Agreed the wording was ambiguous. Changing it so a row is exempt when any key field is null, matching SQL UNIQUE, so two rows with a = 1 and b = null are both allowed.
float and double won't be allowed as key fields, same as identifier fields, so signed zero shouldn't come up. Making the type restrictions explicit in your next comment.
|
|
||
| The fields that define what a constraint requires are embedded directly in the constraint based on its `type`. Each type carries only the metadata that it requires: a `check` constraint has an `expression` and must not declare `field-ids`, and a `unique` or `primary-key` constraint has `field-ids` and 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). |
There was a problem hiding this comment.
Is "the same restrictions" meant to cover everything identifier fields exclude? Identifier fields also rule out float, double, and optional fields, which this sentence doesn't mention. Would it be clearer to list the allowed types and nullability for each constraint type?
Are geometry and geography allowed as keys? They are primitive types, so this line allows them, but as far as I can tell the spec doesn't define equality for them. The identity transform excludes them, and the same shape can have more than one WKB encoding.
For primary-key, should the key fields also have to be required in the schema? That would match identifier fields, and the schema would then rule out null keys even when the constraint isn't enforced. For unique, a field in an optional struct reads as null when the struct is null, and the null rule already exempts those rows. Is the required-struct restriction needed there?
There was a problem hiding this comment.
+1 on listing allowed constraint types.s
+1 on primary-key fields must have required
There was a problem hiding this comment.
All these three make sense. I will change accordingly.
There was a problem hiding this comment.
I just took a look at the changes and was wondering if the list should be inverted from exclude to include?
If a new type is added this would automatically exclude it and not impact the correctness of this spec.
The suggestion would be to add
A primitive type added in a later spec version must not be used as a key field unless that version states that it may be.
A bit later
I scanned the spec and this would be a new pattern. Because it's implicitly assumed, I guess.
The question would be if we should start using it or if it just confuses spec readers. I'm leaning towards later.
|
|
||
| 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. When a constraint is not enforced, writers are not required to verify the rows they add. |
There was a problem hiding this comment.
How should commit retries work with enforced constraints? If I'm reading Commit Conflict Resolution and Retry correctly, "append operations have no requirements and can always be applied", so two appends that each pass a unique check against the same parent could both commit.
Should that section get rules for constraints? For example, a retry could recheck its added rows, recheck when a concurrent commit adds a constraint or sets enforced, and recompute constraint-statuses from the new parent, similar to how first-row-id is reassigned.
There was a problem hiding this comment.
My understanding was that this is one of the cases where the writer could fallback to valid instead of validated.
There was a problem hiding this comment.
Good point. I will add a paragraph on retries and update Commit Conflict Resolution and Retry section.
|
|
||
| A table may have at most one `primary-key` constraint. A key that spans several fields is expressed as a single `primary-key` constraint over multiple `field-ids`. | ||
|
|
||
| A `primary-key` constraint replaces [identifier field IDs](#identifier-field-ids), which express the same concept: a set of fields that identifies a row, without a uniqueness guarantee. When a table is upgraded to v4, its `identifier-field-ids` are rewritten as a `primary-key` constraint that is not enforced. Identifier field IDs are not used in v4. |
There was a problem hiding this comment.
The Flink sink uses identifier fields as its default equality fields (the columns it matches on when it writes equality deletes for upserts), so I think this changes behavior for upgraded tables. Should the upgrade spell out a few details so implementations agree?
- Which schema's
identifier-field-idsare used, since they can differ across schemas? - What
constraint-id,name,timestamp-ms, andlast-constraint-iddoes the upgrade write? - Should v4 writers remove
identifier-field-ids, and what should v4 readers do if they find them? - Line 720 would block
inttolongtype promotion on former identifier fields. Is that intended?
Should the Identifier Field IDs section and Appendix C also mention that v4 doesn't use them? And would this part be easier to review as its own PR?
There was a problem hiding this comment.
Good questions. Luckily we don't need to consider Flink equality delete any more because we forbid writing new equality delete in v4. #17783 Reads aren't affected either, since existing v2/v3 equality deletes use the equality_ids in the delete file metadata rather than the table's identifier-field-ids.
Will spell out the rest so implementations agree:
-
Will use the current schema's identifier-field-ids.
-
constraint-idwill be assigned fromlast-constraint-idthe normal way,
name will bepk, andtimestamp-mswill be the time of the upgrade. In
practice the id is 1, since a v2 or v3 table can't have constraints. The name
is a proposal, happy to pick something else, but it should be fixed so two
implementations don't produce different metadata for the same table. -
Writers must not set
identifier-field-idsin a schema added to a v4 table,
and readers must ignore them. No need to require rewriting existing schemas,
since that would mean editing schemas that existing snapshots point at. -
The type promotion block wasn't intended. Will split that rule: promotions
stay forbidden for check, because the expression contains literals bound to
the field's type that would need rebinding, but they'll be allowed for unique
and primary-key, since a widening preserves values and therefore preserves
distinctness. That keeps int to long working on former identifier fields. -
Will add notes to the Identifier Field IDs section and Appendix C.
On splitting this out: I kept it here since #17783 removes the behavior change that made it risky, and what's left is mechanical, but we can still split if it's clearer that way.
|
|
||
| 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 is evaluated for each row over the values of that row. An expression may reference more than one field of the row, such as `start_date <= end_date`. 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. |
There was a problem hiding this comment.
Does "evaluated for each row over the values of that row" rule out functions whose result depends on something other than the row, like current_date() or rand()? The list of excluded expressions covers other rows and other tables but not these. With current_date(), a row that passes today could fail tomorrow, so a validated status could become false without any write.
Should a new version of a UDF that a check expression calls count as a change to the constraint? The apply function section uses a check constraint that calls a UDF as its example, and a UDF's definition can get a new version without any change to the constraint. Line 700 requires a new constraint-id when the expression changes, "so that statuses recorded for the old definition are not read as applying to the new one". A function reference has no way to name a UDF version today.
There was a problem hiding this comment.
Both good catches.
On non-deterministic functions: I will add a rule. A check expression must return the same result every time for the same row. So current_date() and rand() will not be allowed.
Note I removed the word deterministic earlier in this PR. ebyhr asked how a writer can verify it, because nothing labeled functions that way. But udf-spec does have a deterministic field per version. So the new text will point to that.
On UDF versions: I don't think we can fix it here. To pin a version, the function reference in an expression would need to record which UDF version it means. Today it only records the function name, so it always resolves to the current version. That is a change in the expressions spec. For now I will add a paragraph: changing a function definition changes what the constraint requires, statuses recorded before the change do not describe the new definition, and the writer should validate again. I can raise version pinning on the expressions spec.
|
|
||
| A check expression is evaluated for each row over the values of that row. An expression may reference more than one field of the row, such as `start_date <= end_date`. 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. | ||
|
|
||
| Iceberg predicates use two-valued logic: a predicate always produces true or false and never produces null, so a comparison with a null operand produces false. This differs from SQL `CHECK`, where a row satisfies a constraint unless the predicate produces false and a null value therefore satisfies the constraint. |
There was a problem hiding this comment.
More null chat :)
Is this consistent with the expressions spec? Comparisons there are null-safe, so 34 != null and null <= null are both true. What do you think about pointing to the Boolean logic section, which already covers the SQL CHECK translation?
| Iceberg predicates use two-valued logic: a predicate always produces true or false and never produces null, so a comparison with a null operand produces false. This differs from SQL `CHECK`, where a row satisfies a constraint unless the predicate produces false and a null value therefore satisfies the constraint. | |
| Iceberg predicates use two-valued logic and null-safe comparisons, as defined in the [expressions spec](expressions-spec.md#boolean-logic). This differs from SQL `CHECK`, where a row satisfies a constraint unless the predicate produces false. |
There was a problem hiding this comment.
You are right, the old text was wrong. null-safe comparisons mean null = null is true and 34 != null is true, so a comparison with a null operand produces false is not correct. I will take your suggestion.
I checked the next paragraph and it still holds: price >= 0 with a null price is (null = 0) OR (null > 0), both false, so an optional field still needs OR price IS NULL. That matches the example in the Boolean logic section.
| Writers must record `constraint-statuses` in every snapshot of a table that has constraints, and must place every constraint that exists when the snapshot is created into exactly one status list, following these rules: | ||
|
|
||
| * A constraint must not be listed as `validated` unless it was checked for every row in the snapshot | ||
| * A constraint must not be listed as `valid` unless it was enforced for the commit and the parent snapshot's status for the constraint is `validated` or `valid` |
There was a problem hiding this comment.
Should compaction and deletes keep their parent's status? Neither can introduce a violation, but as I read these rules, they downgrade the status unless the writer rechecks every row. A validated status would become valid for an enforced constraint and unvalidated otherwise. Could a commit that adds no new rows carry its parent's status forward?
There was a problem hiding this comment.
Following constraint: COUNT(*) > 100 could becomes false in case of delete. But for compaction I agree. Engines should be allowed to 'carry-over' the statuses if the operation performed does not modify data.
Assuming deterministic constraints.
There was a problem hiding this comment.
Agreed. I think deletes are safe too, not only compaction.
Removing rows cannot break any of the three types. A check is evaluated per row, so if all rows passed, a smaller set still passes. unique and primary-key need distinct values, and a smaller set of distinct values is still distinct. Only adding rows can create a violation.
@mkroll-db on COUNT(*) > 100: that one is already not allowed. The spec says an expression must not depend on more than one row, so aggregates and window functions are out. So a delete cannot break a check constraint here.
I will add a rule: a commit that adds no rows may keep the parent's validated or valid status, with no need to check rows again. I left invalid out of this. Carrying it forward could be wrong: if the delete removed the bad rows, there is no violating row any more, and the spec says a constraint must not be listed as invalid unless a row in the snapshot violates it.
There was a problem hiding this comment.
@huaxingao thanks for pointing it out. I was too eager to find a counter example 😄 .
|
|
||
| * Renaming or reordering a referenced field is allowed; the constraint continues to apply to the same fields. | ||
| * If a dropped field is referenced only by single-column constraints, the drop is allowed and those constraints are removed automatically. If a dropped field is referenced by a multi-column constraint, the writer must reject the drop unless that constraint is removed in the same change. | ||
| * The type of a referenced field must not be changed, even for type promotions that are otherwise allowed. This restriction may be relaxed in a later version. |
There was a problem hiding this comment.
Does "type" here include whether a field is required? The spec treats required and optional as a property of the field, separate from its type, and Appendix C serializes required next to type. As I read it, a key field can still be made optional, and so can a struct that contains one, since the struct isn't a referenced field. Java supports both through UpdateSchema.makeColumnOptional. What should happen to a primary-key or unique constraint in those cases?
There was a problem hiding this comment.
I'd say no, but I would agree we should specify the required/optional behavior for primary keys. As I mentioned above the invariant here is "You can't do a schema change that would make a constraint un-evaluable or impossible", i'm open for other wordings there.
There was a problem hiding this comment.
Good catch. The rule only talks about type, and required is a separate property of the field, not part of the type. So making a key field optional was not a type change and no rule blocked it.
I will add a bullet:
A referenced field must not be made optional, and a struct that contains a referenced
field must not be made optional. This restriction may be relaxed in a later version.
For unique and check we could allow it. For unique a null just means the row is not compared. For simplicity let's not allow it for now. It is easy to relax later, but hard to take back once allowed.
There was a problem hiding this comment.
@RussellSpitzer thanks, I had not seen your comment when I replied. I will narrow it to primary-key only:
A field referenced by a primary-key constraint must not be made optional, and a struct that contains such a field must not be made optional, because a primary-key requires its fields to be non-null.
|
|
||
| A writer does not have to check every row in a single scan. After checking every row in an ancestor snapshot, a writer may check only the rows added between that ancestor and the current snapshot and record `validated` for the current snapshot. This allows a validation to finish on a table that is written concurrently, without blocking writes or restarting the scan. | ||
|
|
||
| A snapshot's `constraint-statuses` must not be modified after the snapshot is created. Recording a different status for a constraint requires a new snapshot. A snapshot that changes only constraint statuses may reuse its parent's manifest list. |
There was a problem hiding this comment.
Which operation and added-rows should a status-only snapshot use? operation is required, and none of the four operations seem to fit. Reusing the parent's manifest list also seems to conflict with Manifest Lists, which says a new manifest list is written for each commit attempt.
There was a problem hiding this comment.
Some background first: in the sync we discussed whether a status change, for example from unvalidated to validated, can just edit the old snapshot, or whether it needs a new snapshot. We decided it needs a new snapshot, because a snapshot must not be edited. That is why this paragraph exists.
But you are right that the rest of the paragraph raises questions we never decided. So I will trim it to only the immutability rule:
A snapshot's constraint-statuses must not be modified after the snapshot is created. Recording a different status for a constraint requires a new snapshot.
|
|
||
| A snapshot's `constraint-statuses` must not be modified after the snapshot is created. Recording a different status for a constraint requires a new snapshot. A snapshot that changes only constraint statuses may reuse its parent's manifest list. | ||
|
|
||
| A constraint that a snapshot reports as `valid` may later be found not to hold for that snapshot. A `valid` status depends on every commit in the snapshot's history having correctly enforced the constraint, and Iceberg records the status a writer reports without re-checking the data. So if any of those writers was buggy or non-compliant, the `valid` status can be wrong even though nothing detected it at commit time. A `validated` status, which reflects an actual scan of all rows, does not depend on that chain. Writers should then commit a snapshot that records `invalid` and should expire the snapshots that state that the constraint holds, because queries against those snapshots would otherwise continue to rely on a constraint that does not hold. |
There was a problem hiding this comment.
Does this conflict with line 775? Expiring the snapshots that "state that the constraint holds" would include the validated ones, but line 775 keeps validated separate so the latest one can be found. Expiring also removes time travel, and the retention rules keep tagged snapshots anyway. Should this stop at committing a snapshot that records invalid?
There was a problem hiding this comment.
Wouldn't that put a lot of burden on the readers?
For a given old snapshot they would need to read all the new snapshots to verify that there was no invalid marker if they want to make sure that the data is correctly constraint.
In addition we expect readers to implement optimizations on top of constraints. Not expiring invalid snapshots could if it hasn't result in wrong data being returned by the readers.
There was a problem hiding this comment.
Looking again, these paragraphs are explanation, not rules, so I am removing them to make the spec focused.
|
|
||
| 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. When a constraint is not enforced, writers are not required to verify the rows they add. |
There was a problem hiding this comment.
My understanding was that this is one of the cases where the writer could fallback to valid instead of validated.
| === "v4" | ||
| | v4 | Field | Description | | ||
| |------------|------------------------------|-------------| | ||
| | _required_ | **`snapshot-id`** | A unique long ID | |
There was a problem hiding this comment.
NIT: The table is not properly rendered.
|
|
||
| The fields that define what a constraint requires are embedded directly in the constraint based on its `type`. Each type carries only the metadata that it requires: a `check` constraint has an `expression` and must not declare `field-ids`, and a `unique` or `primary-key` constraint has `field-ids` and 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). |
There was a problem hiding this comment.
+1 on listing allowed constraint types.s
+1 on primary-key fields must have required
| Three constraint types are defined: | ||
|
|
||
| * `check` -- every row must satisfy a predicate | ||
| * `unique` -- the non-null values of a set of fields must be distinct across all rows; more than one row may have a null value |
There was a problem hiding this comment.
Regarding equality I'm in favor of equality by value not by bit-representation.
Is in line with the other DBs:
- Postgres
- MySQL
- Sql Server
- Postgres
|
|
||
| A `primary-key` constraint replaces [identifier field IDs](#identifier-field-ids), which express the same concept: a set of fields that identifies a row, without a uniqueness guarantee. When a table is upgraded to v4, its `identifier-field-ids` are rewritten as a `primary-key` constraint that is not enforced. Identifier field IDs are not used in v4. | ||
|
|
||
| Only a constraint's `name` and `enforced` fields may be changed in place. Changing the `expression` of a `check` constraint or the `field-ids` of a `unique` or `primary-key` constraint changes what the constraint requires, so it must be done by removing the constraint and adding a new one with a new `constraint-id`, so that statuses recorded for the old definition are not read as applying to the new one. |
There was a problem hiding this comment.
I think timestamp-ms should be mentioned here as well, since the constraint is modified.
There was a problem hiding this comment.
Good point, I will add that a writer that changes name or enforced must update timestamp-ms.
| | Status | Description | | ||
| |---------------|-------------| | ||
| | `validated` | The constraint was checked and holds for all rows in the snapshot | | ||
| | `valid` | The constraint holds for all rows in the snapshot because it was enforced for the commit that produced the snapshot and the parent snapshot's status is `validated` or `valid` | |
There was a problem hiding this comment.
What does valid mean for an empty table?
I think in this case we should force either validated0or unvalidated.
There was a problem hiding this comment.
Empty tables have to be validated :) I'm not sure of a constraint that wouldn't be true for an empty table. At least none that we currently would allow.
There was a problem hiding this comment.
| | `valid` | The constraint holds for all rows in the snapshot because it was enforced for the commit that produced the snapshot and the parent snapshot's status is `validated` or `valid` | | |
| | `valid` | This write has maintained the constraint for all added rows and the previous snapshot was either 'valid' or 'validated. By induction, the constraint holds for all rows in the table. | |
There was a problem hiding this comment.
@RussellSpitzer taking your wording, thanks!
@mkroll-db on the empty table: I think the current rules already give the right answer. An empty snapshot trivially satisfies "checked for every row", so validated is allowed.
| Writers must record `constraint-statuses` in every snapshot of a table that has constraints, and must place every constraint that exists when the snapshot is created into exactly one status list, following these rules: | ||
|
|
||
| * A constraint must not be listed as `validated` unless it was checked for every row in the snapshot | ||
| * A constraint must not be listed as `valid` unless it was enforced for the commit and the parent snapshot's status for the constraint is `validated` or `valid` |
There was a problem hiding this comment.
Following constraint: COUNT(*) > 100 could becomes false in case of delete. But for compaction I agree. Engines should be allowed to 'carry-over' the statuses if the operation performed does not modify data.
Assuming deterministic constraints.
|
|
||
| When a constraint becomes enforced, either by being added with `enforced` set to true or by `enforced` changing from false to true, writers should validate the table and record `validated`. A writer that does not validate records `unvalidated`, and the constraint remains `unvalidated` until a later validation records `validated`. | ||
|
|
||
| A writer does not have to check every row in a single scan. After checking every row in an ancestor snapshot, a writer may check only the rows added between that ancestor and the current snapshot and record `validated` for the current snapshot. This allows a validation to finish on a table that is written concurrently, without blocking writes or restarting the scan. |
There was a problem hiding this comment.
We had a long discussion about uuid and validated vs valid.
If I remember correctly we landed on uuids being validated if they the parent is validated and the current commit valid.
We should mention this explicitly here.
There was a problem hiding this comment.
I went back to the sync. This came up when we asked why "parent validated plus this commit enforced" is not validated again. The answer was that it stays valid, and UUID was the example used to explain why: if you generate random IDs, you never check anything, so you cannot claim you checked. If you do go back and confirm that none of the generated IDs conflict, that is validated. So generating a UUID counts as enforcing, and the status is valid. I think this is just the normal rule.
There was a problem hiding this comment.
This paragraph is a bit confusing to me. It's more of a description of how to check every row in a table which feels like an implementation detail.
There was a problem hiding this comment.
Agreed, it is an implementation detail. Removed it to keep the spec focused.
|
|
||
| A snapshot's `constraint-statuses` must not be modified after the snapshot is created. Recording a different status for a constraint requires a new snapshot. A snapshot that changes only constraint statuses may reuse its parent's manifest list. | ||
|
|
||
| A constraint that a snapshot reports as `valid` may later be found not to hold for that snapshot. A `valid` status depends on every commit in the snapshot's history having correctly enforced the constraint, and Iceberg records the status a writer reports without re-checking the data. So if any of those writers was buggy or non-compliant, the `valid` status can be wrong even though nothing detected it at commit time. A `validated` status, which reflects an actual scan of all rows, does not depend on that chain. Writers should then commit a snapshot that records `invalid` and should expire the snapshots that state that the constraint holds, because queries against those snapshots would otherwise continue to rely on a constraint that does not hold. |
There was a problem hiding this comment.
Wouldn't that put a lot of burden on the readers?
For a given old snapshot they would need to read all the new snapshots to verify that there was no invalid marker if they want to make sure that the data is correctly constraint.
In addition we expect readers to implement optimizations on top of constraints. Not expiring invalid snapshots could if it hasn't result in wrong data being returned by the readers.
| A constraint references fields by ID, so schema changes interact with constraints as follows. The referenced fields of a `check` constraint are the field IDs in its `expression`; the referenced fields of a `unique` or `primary-key` constraint are its `field-ids`. | ||
|
|
||
| * Renaming or reordering a referenced field is allowed; the constraint continues to apply to the same fields. | ||
| * If a dropped field is referenced only by single-column constraints, the drop is allowed and those constraints are removed automatically. If a dropped field is referenced by a multi-column constraint, the writer must reject the drop unless that constraint is removed in the same change. |
There was a problem hiding this comment.
I understand why we are doing it but it feels a asymmetric.
Dropping a single column will drop single-column constraints only of that column 'silently'.
Dropping a single column with a multi-column constraint will fail 'loud'.
I think we should apply the same rule to single-column constraints as well, that the column can only be dropped if the constraints removal is in the same change. This also feels more deliberate.
There was a problem hiding this comment.
I would also rephrase this to be proscriptive rather than reactive and describe valid states rather than the path to getting to them. For example it's an engine decision on whether or not to automatically remove a constraint when a field is removed or fail. The Spec invariant here I believe is
"All fields referred to by constraints MUST exist in the table's current schema. Field removals that would make it impossible to evaluate a constraint must be rejected unless the constraint is modified or removed."
Then if we want to elaborate on engine behaviors
"Writers may decided wether to automatically remove constraints when their corresponding fields are removed or may throw an error and require manual removal of the referencing constraints first."
Although I probably wouldn't recommend an engines automatically removing constraints without manual intervention.
There was a problem hiding this comment.
@RussellSpitzer taking your invariant, thanks. It reads better as a rule about valid states:
Every field referenced by a constraint must exist in the table's current schema.
A schema change that removes a referenced field must be rejected unless the
constraint is removed in the same change.
@mkroll-db I think this also answers your point. The spec no longer mentions auto-dropping at all, so single-column and multi-column constraints are treated the same way and the asymmetry is gone.
@ebyhr this replaces the wording you asked for earlier, but it still says a drop must be rejected while the constraint exists.
There was a problem hiding this comment.
Thanks @RussellSpitzer , @huaxingao this answers my question.
|
|
||
| Constraints are added in v4 and are not supported in v3 or earlier. | ||
|
|
||
| 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). |
There was a problem hiding this comment.
nit: /s/property/condition/ or invariant? Just feel like property is a little ambiguous here.
I think I like condition best
There was a problem hiding this comment.
For the second status I think "holds" is also a bit ambigous. Perhaps we should say
| 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). | |
| A **constraint** declares a condition that a table’s rows are expected to satisfy. Its definition is stored in table metadata. Each snapshot contains the writer-reported status of each constraint that existed when the snapshot was created; see [Constraint Validation](#constraint-validation). |
There was a problem hiding this comment.
Thanks, I will take your suggestion.
|
|
||
| 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. |
There was a problem hiding this comment.
I'd drop the "Iceberg does not evaluate constraints" since i'm not sure what this means. I'd probably drop this whole paragraph.
| * `unique` -- the non-null values of a set of fields must be distinct across all rows; more than one row may have a null value | ||
| * `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 they span multiple fields and 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. |
There was a problem hiding this comment.
I think we can strip this down as well
The "because" line doesn't quite make sense to me. I'd probably just say
"A Constraint is stored separately from table schema and must refer to fields by fieldID. Constraints may evolve independently from schema, being added, modified or removed. "
We then probably need some information on evolution. A constraint must only refer to fields which exist in the current table schema? Or something like that
There was a problem hiding this comment.
Taking your wording, thanks
| | _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 | |
There was a problem hiding this comment.
I do not think it's whether they must verify. I think it's more that
"Writers must only add writers which satisfy the constraint."
There was a problem hiding this comment.
Taking your suggestion, thanks. I will also update the paragraph below, since it used the same "must verify" wording.
|
|
||
| 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. When a constraint is not enforced, writers are not required to verify the rows they add. |
There was a problem hiding this comment.
I again think "verify" is the wrong verb here.
"When a constraint is enforced, writers must only add rows they can prove follow the constraint. An un-enforced constraint can be written to by any writer regardless of their ability to prove that the constraint is followed by new rows."
There was a problem hiding this comment.
Taking your suggestion. Thanks
|
|
||
| Whether to trust a constraint that is not enforced is left to engines and is not tracked in table metadata. | ||
|
|
||
| A table may have at most one `primary-key` constraint. A key that spans several fields is expressed as a single `primary-key` constraint over multiple `field-ids`. |
There was a problem hiding this comment.
I'd probably just drop this. I think we can add "A table may have at most one primary key constraint" to the next paragraph if we need it.
|
|
||
| A `primary-key` constraint replaces [identifier field IDs](#identifier-field-ids), which express the same concept: a set of fields that identifies a row, without a uniqueness guarantee. When a table is upgraded to v4, its `identifier-field-ids` are rewritten as a `primary-key` constraint that is not enforced. Identifier field IDs are not used in v4. | ||
|
|
||
| Only a constraint's `name` and `enforced` fields may be changed in place. Changing the `expression` of a `check` constraint or the `field-ids` of a `unique` or `primary-key` constraint changes what the constraint requires, so it must be done by removing the constraint and adding a new one with a new `constraint-id`, so that statuses recorded for the old definition are not read as applying to the new one. |
There was a problem hiding this comment.
This probably can go in a little section on evolution?
There was a problem hiding this comment.
Makes sense. Will move
| |---------------|-------------| | ||
| | `validated` | The constraint was checked and holds for all rows in the snapshot | | ||
| | `valid` | The constraint holds for all rows in the snapshot because it was enforced for the commit that produced the snapshot and the parent snapshot's status is `validated` or `valid` | | ||
| | `invalid` | The constraint was checked and at least one row in the snapshot violates it | |
There was a problem hiding this comment.
Either the constraint has been checked and at least one row in the snapshot violates the snapshot, or this snapshot was built upon a previous invalid state.
[Invalid] --- any partial write or not ---> [Invalid]
[Valid] --- a failed validation check ---> [Invalid]
[Invalid] -- a total enforced rewrite or a successful validation ---> [Valid | Validated]
There was a problem hiding this comment.
Makes sense. Will change to
Either the constraint was checked and at least one row in the snapshot violates it, or the snapshot was built on a parent snapshot whose status is `invalid`
| | _optional_ | **`invalid`** | `list<int>` | IDs of constraints that are `invalid` for the snapshot | | ||
| | _optional_ | **`unvalidated`** | `list<int>` | IDs of constraints that are `unvalidated` for the snapshot | | ||
|
|
||
| Each list contains the `constraint-id` of every constraint that has that status for the snapshot. Every constraint that exists when the snapshot is created must be listed in exactly one of the four lists, and a `constraint-id` must not appear in more than one list. A list with no constraints may be omitted. A constraint whose ID is not present in any list did not exist when the snapshot was created, so the snapshot makes no claim about it. |
There was a problem hiding this comment.
For the last line, "A constraint whose ID is not present did not exist when the snapshot was created so is 'unvalidated'."
There was a problem hiding this comment.
I will use your words and also also cut the reader rules down to two cases, since with this change "not listed" and "unvalidated" are the same thing.
|
|
||
| Each list contains the `constraint-id` of every constraint that has that status for the snapshot. Every constraint that exists when the snapshot is created must be listed in exactly one of the four lists, and a `constraint-id` must not appear in more than one list. A list with no constraints may be omitted. A constraint whose ID is not present in any list did not exist when the snapshot was created, so the snapshot makes no claim about it. | ||
|
|
||
| This is an explicit representation: each constraint's status is recorded independently, so the size of `constraint-statuses` grows with the number of constraints in a table. This keeps the encoding simple; more compact representations may be added in a later version if it becomes a problem. |
There was a problem hiding this comment.
I don't think we need this paragraph it's mostly just discussion and future plans.
There was a problem hiding this comment.
Agreed, it is explanation and not a rule. I will remove the paragraph.
|
|
||
| This is an explicit representation: each constraint's status is recorded independently, so the size of `constraint-statuses` grows with the number of constraints in a table. This keeps the encoding simple; more compact representations may be added in a later version if it becomes a problem. | ||
|
|
||
| Readers must determine the status of a constraint for a snapshot as follows: |
There was a problem hiding this comment.
I think it's really just two things here.
Either the constraint's status is explicitly listed or it can be considered "unvalidated"
There was a problem hiding this comment.
Agreed. I will cut it down to two cases.
…-update Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # format/spec.md
| | _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 | | ||
| | _optional_ | **`expression`** | `expression` | The predicate that every row must satisfy, see [Check Constraint Expressions](#check-constraint-expressions). Required for a `check` constraint and must not be set for other types | |
|
|
||
| A check expression is evaluated for each row over the values of that row. An expression may reference more than one field of the row, such as `start_date <= end_date`. 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. | ||
|
|
||
| A check expression must produce the same result every time it is evaluated for the same row. A function that depends on anything other than its arguments, such as the current time or a random value, must not be called, because the status recorded for a snapshot describes the table's data and an expression whose result can change on its own would make a recorded status wrong without any write. A [user-defined function](udf-spec.md) records whether it is deterministic. |
There was a problem hiding this comment.
Small change to the last part.
A user-defined function must not be called unless it declares deterministic as true.
There was a problem hiding this comment.
Applied the change. Thanks!
| Writers must record `constraint-statuses` in every snapshot of a table that has constraints, and must place every constraint that exists when the snapshot is created into exactly one status list, following these rules: | ||
|
|
||
| * A constraint must not be listed as `validated` unless it was checked for every row in the snapshot | ||
| * A constraint must not be listed as `valid` unless it was enforced for the commit and the parent snapshot's status for the constraint is `validated` or `valid` |
There was a problem hiding this comment.
@huaxingao thanks for pointing it out. I was too eager to find a counter example 😄 .
|
|
||
| When a constraint becomes enforced, either by being added with `enforced` set to true or by `enforced` changing from false to true, writers should validate the table and record `validated`. A writer that does not validate records `unvalidated`, and the constraint remains `unvalidated` until a later validation records `validated`. | ||
|
|
||
| A writer does not have to check every row in a single scan. After checking every row in an ancestor snapshot, a writer may check only the rows added between that ancestor and the current snapshot and record `validated` for the current snapshot. This allows a validation to finish on a table that is written concurrently, without blocking writes or restarting the scan. |
| A constraint references fields by ID, so schema changes interact with constraints as follows. The referenced fields of a `check` constraint are the field IDs in its `expression`; the referenced fields of a `unique` or `primary-key` constraint are its `field-ids`. | ||
|
|
||
| * Renaming or reordering a referenced field is allowed; the constraint continues to apply to the same fields. | ||
| * If a dropped field is referenced only by single-column constraints, the drop is allowed and those constraints are removed automatically. If a dropped field is referenced by a multi-column constraint, the writer must reject the drop unless that constraint is removed in the same change. |
There was a problem hiding this comment.
Thanks @RussellSpitzer , @huaxingao this answers my question.
|
|
||
| The fields that define what a constraint requires are embedded directly in the constraint based on its `type`. Each type carries only the metadata that it requires: a `check` constraint has an `expression` and must not declare `field-ids`, and a `unique` or `primary-key` constraint has `field-ids` and 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). |
There was a problem hiding this comment.
I just took a look at the changes and was wondering if the list should be inverted from exclude to include?
If a new type is added this would automatically exclude it and not impact the correctness of this spec.
The suggestion would be to add
A primitive type added in a later spec version must not be used as a key field unless that version states that it may be.
A bit later
I scanned the spec and this would be a new pattern. Because it's implicitly assumed, I guess.
The question would be if we should start using it or if it just confuses spec readers. I'm leaning towards later.
|
@danielcweeks @mbutrovich @RussellSpitzer @mkroll-db I have addressed all your comments. Could you please take one more look when you have a moment? Thanks! |
mkroll-db
left a comment
mkroll-db
left a comment
There was a problem hiding this comment.
LGTM. Thanks @huaxingao!!!!
|
|
||
| #### Constraint Validation | ||
|
|
||
| Enforcement and validation are separate properties. Whether writers must verify the rows that they add is a property of a constraint, tracked by `enforced`. Whether a table is known to satisfy a constraint is a property of a table's data, tracked per snapshot by `constraint-statuses`. |
There was a problem hiding this comment.
NIT:
The text later uses status a lot which is actually the short form of constraint status.
It's clear from reading, but following change makes it a tad more clear:
| Enforcement and validation are separate properties. Whether writers must verify the rows that they add is a property of a constraint, tracked by `enforced`. Whether a table is known to satisfy a constraint is a property of a table's data, tracked per snapshot by `constraint-statuses`. | |
| Enforcement and constraint status are tracked separately.. Whether writers must verify the rows that they add is a property of a constraint, tracked by `enforced`. Whether a table is known to satisfy a constraint is a property of a table's data, tracked per snapshot by `constraint-statuses`. |
But I'm fine as is.
There was a problem hiding this comment.
Thanks for your suggestion! Applied.
This branch has not been deployed
6 participants
Adds table constraints to the v4 spec
Constraintssection definingcheck,unique, andprimary-keyconstraintsconstraintsandlast-constraint-idadded to v4 table metadataconstraint-statusesadded to v4 snapshots, recording whether a constraint isvalidated,valid,invalid, orunvalidated