nvartolomei commented on code in PR #14004: URL: https://github.com/apache/iceberg/pull/14004#discussion_r2336676949
########## format/spec.md: ########## @@ -1861,6 +1861,18 @@ Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers t Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`. +### Schema Evolution/Type Promotion + +Column projection rules are designed so that the table will remain readable even if writers use an outdated schema. At the beginning of a transaction Writers should load the latest schema (the schema pointed to by `current-schema-id` from the latest table metadata) and use it for reading and writing data. Note, that in the common cases of schema evolution (adding nullable columns, adding required columns with an `initial-default`, renaming a column, dropping a column, or doing type promotion) then appending data with outdated schemas presents no issues under either SNAPSHOT or SERIALIZABLE isolation levels. + +While writers are not required to bind to the latest schema there are edge cases to consider: + +1. Assume two transactions that are started concurrently. The first modifies the `write-default` on the column. The second is a data write that makes use of `write-default` from the changed column in the first transaction. If the first transaction gets committed first, the result of the second transaction depends on isolation level. Under SNAPSHOT isolation the second transaction can be committed. However, the second transaction produces the serialization anomaly of using the outdated `write-default` default value. SERIALIZABLE isolation does not allow for such anomolies and the second transaction must fail in this mode. In this scenario, the transaction could be retried after updating to the new schema and rewriting the data using the new `write-default`. To generalize, for SERIALIZABLE isolation, writers must confirm that the schema did not have a change to a `write-default` value, this can be confirmed in two stages: First check if there was any schema change (for the REST catal og this can be done with `assert-ref-snapshot-id`); Second, if the schema changed determine if there was a change to a `write-default` value used in the transaction. No check for schema changes is needed For SNAPSHOT isolation. Review Comment: Wording suggestion Consider two concurrent transactions: **T1** updates a column’s `write-default`; T2 writes data for the same column. If **T1** commits before **T2**, the **T2** must be handled according to the isolation level: * **SNAPSHOT**: the writer **MAY** commit **T2** even though it used the old `write-default` (this is a permitted serialization anomaly). * **SERIALIZABLE**: the writer **MUST** abort **T2** because it read a stale `write-default`. Followed by the implementation suggestion. --- > First check if there was any schema change (for the REST catalog this can be done with `assert-ref-snapshot-id`); Changing schema does not create a new snapshot. I believe you want `assert-current-schema-id` here. ########## format/spec.md: ########## @@ -1861,6 +1861,18 @@ Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers t Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`. +### Schema Evolution/Type Promotion + +Column projection rules are designed so that the table will remain readable even if writers use an outdated schema. At the beginning of a transaction Writers should load the latest schema (the schema pointed to by `current-schema-id` from the latest table metadata) and use it for reading and writing data. Note, that in the common cases of schema evolution (adding nullable columns, adding required columns with an `initial-default`, renaming a column, dropping a column, or doing type promotion) then appending data with outdated schemas presents no issues under either SNAPSHOT or SERIALIZABLE isolation levels. Review Comment: > and use it for reading and writing data. Should we add here "Also, at commit writers _should_ check that the schema wasn't changed concurrently."? ########## format/spec.md: ########## @@ -1861,6 +1861,18 @@ Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers t Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`. +### Schema Evolution/Type Promotion + +Column projection rules are designed so that the table will remain readable even if writers use an outdated schema. At the beginning of a transaction Writers should load the latest schema (the schema pointed to by `current-schema-id` from the latest table metadata) and use it for reading and writing data. Note, that in the common cases of schema evolution (adding nullable columns, adding required columns with an `initial-default`, renaming a column, dropping a column, or doing type promotion) then appending data with outdated schemas presents no issues under either SNAPSHOT or SERIALIZABLE isolation levels. + +While writers are not required to bind to the latest schema there are edge cases to consider: Review Comment: > While writers are not required to bind to the latest schema there are edge cases to consider: While writers are not required to **use (?)** the latest schema there are edge cases to consider: ########## format/spec.md: ########## @@ -1861,6 +1861,18 @@ Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers t Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`. +### Schema Evolution/Type Promotion + +Column projection rules are designed so that the table will remain readable even if writers use an outdated schema. At the beginning of a transaction Writers should load the latest schema (the schema pointed to by `current-schema-id` from the latest table metadata) and use it for reading and writing data. Note, that in the common cases of schema evolution (adding nullable columns, adding required columns with an `initial-default`, renaming a column, dropping a column, or doing type promotion) then appending data with outdated schemas presents no issues under either SNAPSHOT or SERIALIZABLE isolation levels. + +While writers are not required to bind to the latest schema there are edge cases to consider: + +1. Assume two transactions that are started concurrently. The first modifies the `write-default` on the column. The second is a data write that makes use of `write-default` from the changed column in the first transaction. If the first transaction gets committed first, the result of the second transaction depends on isolation level. Under SNAPSHOT isolation the second transaction can be committed. However, the second transaction produces the serialization anomaly of using the outdated `write-default` default value. SERIALIZABLE isolation does not allow for such anomolies and the second transaction must fail in this mode. In this scenario, the transaction could be retried after updating to the new schema and rewriting the data using the new `write-default`. To generalize, for SERIALIZABLE isolation, writers must confirm that the schema did not have a change to a `write-default` value, this can be confirmed in two stages: First check if there was any schema change (for the REST catal og this can be done with `assert-ref-snapshot-id`); Second, if the schema changed determine if there was a change to a `write-default` value used in the transaction. No check for schema changes is needed For SNAPSHOT isolation. Review Comment: The T1, T2 notation is easier to interpret for me. I'd consider using it for the next paragraph too. ########## format/spec.md: ########## @@ -1861,6 +1861,18 @@ Java writes `-1` for "no current snapshot" with V1 and V2 tables and considers t Some implementations require that GZIP compressed files have the suffix `.gz.metadata.json` to be read correctly. The Java reference implementation can additionally read GZIP compressed files with the suffix `metadata.json.gz`. +### Schema Evolution/Type Promotion + +Column projection rules are designed so that the table will remain readable even if writers use an outdated schema. Writers should bind the latest schema at the beginning of a transaction. Note, that in the common cases of schema evolution (adding nullable columns, adding required columns with an `initial-default`, renaming a column, dropping a column, or doing type promotion) then appending data with outdated schemas presents no issues under either SNAPSHOT or SERIALIZABLE isolation levels. + +While writers are not required to bind to the latest schema there are edge cases to consider: + +1. Assume two transactions that are started concurrently. The first modifies the `write-default` on the column. The second is a data write that makes use of `write-default` from the changed column in the first transaction. If the first transaction gets committed first, the result of the second transaction depends on isolation level. Under SNAPSHOT isolation the second transaction can be committed. However, the second transaction produces the serialization anomaly of using the outdated `write-default` default value. SERIALIZABLE isolation does not allow for such anomolies and the second transaction must fail in this mode. The transaction could be retried after updating to the new schema and rewriting the data using the new `write-default`. + +2. Assume a sequence of the linear transactions: the first transaction adds a columnand populates it with new values. The second transactions is run using the schema prior to the new column being added and updates another column (e.g. `update table x set pre_existing_col='xyz'`). Transaction b) must fail under both SNAPSHOT and SERIALIZABLE isolation levels, since it would drop data from the new column added in the first transaction. If the transactions started concurrently, one of them should still fail with SNAPSHOT isolation because there is an overlap in the rows modified by the transactions. Review Comment: This example in entirety does not belong to "Schema Evolution/Type Promotion" section imho. As I said in previous comment, this is about data and not schema. However, if there is a concurrent schema change then we might be some issues if the user doesn't use the new schema when transaction is restarted. But we cover this in the first paragraph already that you should use latest schema and in case of serializable you must. From my point of view, this paragraph should be deleted and instead the one under https://iceberg.apache.org/spec/#optimistic-concurrency be expanded with a note about schema (with link to this new section). It should fit nicely between the existing lines. > If the snapshot on which an update is based is no longer current, the writer must retry the update based on the new current version. Some operations support retry by re-applying metadata changes and committing, under well-defined conditions. For example, a change that rewrites files can be applied to a new table snapshot if all of the rewritten files are still in the table. > > The conditions required by a write to successfully commit determines the isolation level. Writers can select what to validate and can make different isolation guarantees. Wdyt? -- 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]
