stevenzwu commented on code in PR #17918: URL: https://github.com/apache/iceberg/pull/17918#discussion_r4215019690
########## format/spec.md: ########## @@ -1566,38 +1600,40 @@ Lists must use the [3-level representation](https://github.com/apache/parquet-fo | **`variant`** | `group` with `metadata` and `value` fields. `metadata` and `value` must not be assigned field IDs and the fields are accessed through names. | `VARIANT` | See Parquet docs for [Variant encoding](https://github.com/apache/parquet-format/blob/master/VariantEncoding.md) and [Variant shredding encoding](https://github.com/apache/parquet-format/blob/master/VariantShredding.md). | | **`geometry`** | `binary` | `GEOMETRY` | WKB format, see [Appendix G](#appendix-g-geospatial-notes). | | **`geography`** | `binary` | `GEOGRAPHY` | WKB format, see [Appendix G](#appendix-g-geospatial-notes). | +| **`file`** | `group` with the `file` sub-fields. Sub-fields must be assigned field IDs. | `FILE` | See Parquet docs for the [`FILE` type](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file)) and [File Type](#file-type). | When reading an `unknown` column, any corresponding column must be ignored and replaced with `null` values. ### ORC **Data Type Mappings** -| Type | ORC type | ORC type attributes | Notes | Review Comment: is it possible to avoid the reformat/realignment of the table to minimize the diff? ########## format/spec.md: ########## @@ -322,6 +325,36 @@ For `geography` types, an additional parameter A specifies an algorithm for inte * `andoyer`: Thomas, Paul D. Mathematical models for navigation systems. US Naval Oceanographic Office, 1965. * `karney`: [Karney, Charles FF. "Algorithms for geodesics." Journal of Geodesy 87 (2013): 43-55](https://link.springer.com/content/pdf/10.1007/s00190-012-0578-z.pdf), and [GeographicLib](https://geographiclib.sourceforge.io/) +#### File Type + +A **`file`** represents a range of bytes that may be stored inline as a value or a reference to an external file. The `file` type and its value semantics are defined by the `FILE` logical type in the [Parquet project](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file). + +A `file` value has a fixed set of sub-fields. The sub-fields are implicit: they are not represented in the Iceberg schema and cannot be added, removed, reordered, or promoted. Their names, types, and field-ID offsets are: + +| Sub-field | ID offset | Type | +|----------------|-----------|----------| +| `uri` | +1 | `string` | +| `offset` | +2 | `long` | +| `size` | +3 | `long` | +| `content_type` | +4 | `string` | +| `checksum` | +5 | `string` | +| `inline` | +6 | `binary` | + +Adding a `file` field reserves the root field's ID plus six consecutive IDs for its sub-fields, assigned by the offsets above. Writers must advance `last-column-id` past all seven IDs and must not assign these IDs to any other field. Review Comment: `last-column-id` is the highest assigned column ID, so this should set it to the highest reserved ID (the file's ID + 6). "Past all seven IDs" reads as one past the block. The actor is whoever assigns the ID, not a data writer. The base is whatever ID the `file` type receives, including a list element or a map value. `list<file>` has to reserve the same block starting at the element ID, or the next assigned column collides with `uri`. ########## format/spec.md: ########## @@ -322,6 +325,36 @@ For `geography` types, an additional parameter A specifies an algorithm for inte * `andoyer`: Thomas, Paul D. Mathematical models for navigation systems. US Naval Oceanographic Office, 1965. * `karney`: [Karney, Charles FF. "Algorithms for geodesics." Journal of Geodesy 87 (2013): 43-55](https://link.springer.com/content/pdf/10.1007/s00190-012-0578-z.pdf), and [GeographicLib](https://geographiclib.sourceforge.io/) +#### File Type + +A **`file`** represents a range of bytes that may be stored inline as a value or a reference to an external file. The `file` type and its value semantics are defined by the `FILE` logical type in the [Parquet project](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file). + +A `file` value has a fixed set of sub-fields. The sub-fields are implicit: they are not represented in the Iceberg schema and cannot be added, removed, reordered, or promoted. Their names, types, and field-ID offsets are: Review Comment: The sub-fields need a nullability and a presence rule. The table does not say they are optional, which is what the earlier thread concluded, and "cannot be removed" can be read as requiring every data file to materialize all six columns. Parquet FILE makes every field optional in the schema and in the data. A missing `offset` resolves as 0, and a missing `content_type` resolves as `application/octet-stream`. Iceberg projection resolves a field ID that is absent from the data file as null. Those disagree for a projected `offset` or `content_type`. Please state both: - Each sub-field is optional. The fixed set is the logical definition. Say whether a data file may omit an unused sub-field. - Projecting a sub-field by its reserved ID returns null when it is missing or null. Parquet's resolution defaults apply when resolving the byte range, not when reading the sub-field. The format rows should match. Parquet and ORC say sub-fields must be assigned field IDs; if omission is allowed, that applies to the sub-fields that are written. ########## format/spec.md: ########## @@ -322,6 +325,36 @@ For `geography` types, an additional parameter A specifies an algorithm for inte * `andoyer`: Thomas, Paul D. Mathematical models for navigation systems. US Naval Oceanographic Office, 1965. * `karney`: [Karney, Charles FF. "Algorithms for geodesics." Journal of Geodesy 87 (2013): 43-55](https://link.springer.com/content/pdf/10.1007/s00190-012-0578-z.pdf), and [GeographicLib](https://geographiclib.sourceforge.io/) +#### File Type + +A **`file`** represents a range of bytes that may be stored inline as a value or a reference to an external file. The `file` type and its value semantics are defined by the `FILE` logical type in the [Parquet project](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file). + +A `file` value has a fixed set of sub-fields. The sub-fields are implicit: they are not represented in the Iceberg schema and cannot be added, removed, reordered, or promoted. Their names, types, and field-ID offsets are: + +| Sub-field | ID offset | Type | +|----------------|-----------|----------| +| `uri` | +1 | `string` | +| `offset` | +2 | `long` | +| `size` | +3 | `long` | +| `content_type` | +4 | `string` | +| `checksum` | +5 | `string` | +| `inline` | +6 | `binary` | + +Adding a `file` field reserves the root field's ID plus six consecutive IDs for its sub-fields, assigned by the offsets above. Writers must advance `last-column-id` past all seven IDs and must not assign these IDs to any other field. + +The `uri` field may contain absolute or relative references. Relative resolution within a URI (e.g. `.` and `..`) and other file system navigation conventions are not supported. Implementations that receive a relative path should resolve the path against the table location (see [Path Resolution](#path-resolution)). + +A `file` value has no whole-value statistics. Each sub-field's statistics are tracked in `content_stats` under the sub-field's reserved ID, as for any field of the sub-field's type. Writers should produce statistics for `uri`, `content_type`, and `inline` fields; other fields may be omitted. + +A `file` column is subject to the following restrictions: + +* Non-null values for `initial-default` or `write-default` are invalid. +* No type promotion to or from `file` is defined. Review Comment: This conflicts with the promotion table, where `unknown` promotes to any type. Either `unknown` cannot promote to `file`, or that promotion is the exception. ########## format/spec.md: ########## @@ -321,6 +323,32 @@ For `geography` types, an additional parameter A specifies an algorithm for inte * `andoyer`: Thomas, Paul D. Mathematical models for navigation systems. US Naval Oceanographic Office, 1965. * `karney`: [Karney, Charles FF. "Algorithms for geodesics." Journal of Geodesy 87 (2013): 43-55](https://link.springer.com/content/pdf/10.1007/s00190-012-0578-z.pdf), and [GeographicLib](https://geographiclib.sourceforge.io/) +#### File Type + +A **`file`** represents a range of bytes that may be stored inline in the value or in an external file. The `file` type and its value semantics are defined by the `FILE` logical type in the [Parquet project](https://github.com/apache/parquet-format/pull/585). + +A `file` value has a fixed set of sub-fields. The sub-fields are implicit: they are not represented in the Iceberg schema and cannot be added, removed, reordered, or promoted. Their names, types, and field-ID offsets are: + +| Sub-field | ID offset | Type | +|----------------|-----------|----------| +| `uri` | +1 | `string` | +| `offset` | +2 | `long` | +| `size` | +3 | `long` | +| `content_type` | +4 | `string` | +| `checksum` | +5 | `string` | +| `inline` | +6 | `binary` | + +Adding a `file` field reserves the root field's ID plus six consecutive IDs for its sub-fields, assigned by the offsets above. Writers must advance `last-column-id` past all seven IDs and must not assign these IDs to any other field. + +A `file` value has no whole-value statistics. Each sub-field's statistics are tracked in `content_stats` under the sub-field's reserved ID, as for any field of the sub-field's type. Writers should produce statistics for `uri`, `content_type`, and `inline` fields; other fields may be omitted. Review Comment: The new sentence still says to produce statistics for `inline`. In `content_stats`, statistics for a `binary` include `lower_bound`, `upper_bound`, and `total_bytes`, which is the part this thread was dropping. Value and null counts are what distinguish inline from external. Could this name the metrics? Bounds for `uri` and `content_type`; counts where they are useful; no bounds for `inline` or `checksum`. `size` is the sub-field a reader would skip on, and it is in the "may be omitted" group. Is that intentional? Sub-fields also are not in the schema, and content stats recommends naming each struct after the field's full schema name. These need a name too (`<column>.uri`, and so on). ########## format/spec.md: ########## @@ -322,6 +325,36 @@ For `geography` types, an additional parameter A specifies an algorithm for inte * `andoyer`: Thomas, Paul D. Mathematical models for navigation systems. US Naval Oceanographic Office, 1965. * `karney`: [Karney, Charles FF. "Algorithms for geodesics." Journal of Geodesy 87 (2013): 43-55](https://link.springer.com/content/pdf/10.1007/s00190-012-0578-z.pdf), and [GeographicLib](https://geographiclib.sourceforge.io/) +#### File Type + +A **`file`** represents a range of bytes that may be stored inline as a value or a reference to an external file. The `file` type and its value semantics are defined by the `FILE` logical type in the [Parquet project](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file). Review Comment: Yes — the nested-type sentence above already says "as a reference". ########## format/spec.md: ########## @@ -322,6 +325,36 @@ For `geography` types, an additional parameter A specifies an algorithm for inte * `andoyer`: Thomas, Paul D. Mathematical models for navigation systems. US Naval Oceanographic Office, 1965. * `karney`: [Karney, Charles FF. "Algorithms for geodesics." Journal of Geodesy 87 (2013): 43-55](https://link.springer.com/content/pdf/10.1007/s00190-012-0578-z.pdf), and [GeographicLib](https://geographiclib.sourceforge.io/) +#### File Type + +A **`file`** represents a range of bytes that may be stored inline as a value or a reference to an external file. The `file` type and its value semantics are defined by the `FILE` logical type in the [Parquet project](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file). + +A `file` value has a fixed set of sub-fields. The sub-fields are implicit: they are not represented in the Iceberg schema and cannot be added, removed, reordered, or promoted. Their names, types, and field-ID offsets are: + +| Sub-field | ID offset | Type | +|----------------|-----------|----------| +| `uri` | +1 | `string` | +| `offset` | +2 | `long` | +| `size` | +3 | `long` | +| `content_type` | +4 | `string` | +| `checksum` | +5 | `string` | +| `inline` | +6 | `binary` | + +Adding a `file` field reserves the root field's ID plus six consecutive IDs for its sub-fields, assigned by the offsets above. Writers must advance `last-column-id` past all seven IDs and must not assign these IDs to any other field. + +The `uri` field may contain absolute or relative references. Relative resolution within a URI (e.g. `.` and `..`) and other file system navigation conventions are not supported. Implementations that receive a relative path should resolve the path against the table location (see [Path Resolution](#path-resolution)). + +A `file` value has no whole-value statistics. Each sub-field's statistics are tracked in `content_stats` under the sub-field's reserved ID, as for any field of the sub-field's type. Writers should produce statistics for `uri`, `content_type`, and `inline` fields; other fields may be omitted. + +A `file` column is subject to the following restrictions: + +* Non-null values for `initial-default` or `write-default` are invalid. +* No type promotion to or from `file` is defined. +* Whole-value equality, ordering, and hashing are not defined. +* A `file` column cannot be an identifier field or a source for partition or sort transforms. Review Comment: The sort thread agreed that a sub-field's reserved ID can be a partition or sort source. This only has the negative rule for the `file` column. Those IDs are not in the schema, so an implementation can reject `source-id = <file id> + 3` and still follow this text. Please allow a partition or sort `source-id` to name a sub-field's reserved ID, with the transform rules of that sub-field's type. A `file` also has no whole-value equality, so it should be excluded as a map key. Map keys are currently any type. ########## format/spec.md: ########## @@ -1566,38 +1600,40 @@ Lists must use the [3-level representation](https://github.com/apache/parquet-fo | **`variant`** | `group` with `metadata` and `value` fields. `metadata` and `value` must not be assigned field IDs and the fields are accessed through names. | `VARIANT` | See Parquet docs for [Variant encoding](https://github.com/apache/parquet-format/blob/master/VariantEncoding.md) and [Variant shredding encoding](https://github.com/apache/parquet-format/blob/master/VariantShredding.md). | | **`geometry`** | `binary` | `GEOMETRY` | WKB format, see [Appendix G](#appendix-g-geospatial-notes). | | **`geography`** | `binary` | `GEOGRAPHY` | WKB format, see [Appendix G](#appendix-g-geospatial-notes). | +| **`file`** | `group` with the `file` sub-fields. Sub-fields must be assigned field IDs. | `FILE` | See Parquet docs for the [`FILE` type](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file)) and [File Type](#file-type). | Review Comment: Stray `)` after the Parquet link. ########## format/spec.md: ########## @@ -1511,6 +1544,7 @@ Maps with non-string keys must use an array representation with the `map` logica |**`variant`**|`record` with `metadata` and `value` fields. `metadata` and `value` must not be assigned field IDs and the fields are accessed through names. |Shredding is not supported in Avro.| |**`geometry`**|`bytes`|WKB format, see [Appendix G](#appendix-g-geospatial-notes)| |**`geography`**|`bytes`|WKB format, see [Appendix G](#appendix-g-geospatial-notes)| +|**`file`**|`record` with the `file` sub-fields. |See [File Type](#file-type). Avro has no `FILE` logical type; type identity comes from the Iceberg schema.| Review Comment: Parquet and ORC require the reserved field IDs on the sub-fields. Avro needs the same: `field-id` on each sub-field that is written. Type identity coming from the Iceberg schema does not carry those IDs, so an Avro reader cannot project `uri` or `size` by ID without them. ########## format/spec.md: ########## @@ -322,6 +325,36 @@ For `geography` types, an additional parameter A specifies an algorithm for inte * `andoyer`: Thomas, Paul D. Mathematical models for navigation systems. US Naval Oceanographic Office, 1965. * `karney`: [Karney, Charles FF. "Algorithms for geodesics." Journal of Geodesy 87 (2013): 43-55](https://link.springer.com/content/pdf/10.1007/s00190-012-0578-z.pdf), and [GeographicLib](https://geographiclib.sourceforge.io/) +#### File Type + +A **`file`** represents a range of bytes that may be stored inline as a value or a reference to an external file. The `file` type and its value semantics are defined by the `FILE` logical type in the [Parquet project](https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#file). + +A `file` value has a fixed set of sub-fields. The sub-fields are implicit: they are not represented in the Iceberg schema and cannot be added, removed, reordered, or promoted. Their names, types, and field-ID offsets are: + +| Sub-field | ID offset | Type | +|----------------|-----------|----------| +| `uri` | +1 | `string` | +| `offset` | +2 | `long` | +| `size` | +3 | `long` | +| `content_type` | +4 | `string` | +| `checksum` | +5 | `string` | +| `inline` | +6 | `binary` | + +Adding a `file` field reserves the root field's ID plus six consecutive IDs for its sub-fields, assigned by the offsets above. Writers must advance `last-column-id` past all seven IDs and must not assign these IDs to any other field. + +The `uri` field may contain absolute or relative references. Relative resolution within a URI (e.g. `.` and `..`) and other file system navigation conventions are not supported. Implementations that receive a relative path should resolve the path against the table location (see [Path Resolution](#path-resolution)). Review Comment: Readers have to resolve a relative `uri` the same way, so this should be must, using the absolute/relative test and the `/` join in Path Resolution. Metadata paths already say must. RFC 3986 relative resolution (`.` / `..`) stays unsupported, and that should be a requirement on writers rather than "are not supported", which does not say what a reader does when those forms show up. Writers are not mentioned. A `uri` under the table location only moves with the table if it is stored relative, the same way metadata paths are relativized. An absolute `uri` still points at the old location after a relocate, because data files are not rewritten. Worth saying that writers should relativize, or that only relative values relocate. -- 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]
