openinx commented on code in PR #17918:
URL: https://github.com/apache/iceberg/pull/17918#discussion_r4287151467


##########
format/spec.md:
##########
@@ -322,6 +325,35 @@ 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 as 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` |

Review Comment:
   Is `content_type` purely informational metadata, or does the query engine 
actually use it during query processing or execution?



##########
format/spec.md:
##########
@@ -322,6 +325,35 @@ 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 as 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:
   does this mean that the `FILE` type will only works for the parquet file 
format ?  perhaps I missed some of the historical discussion before,  what are 
the our long term plan for the other file format, such as Orc, Avro?



##########
format/spec.md:
##########
@@ -322,6 +325,35 @@ 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 as 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` |

Review Comment:
   for my understanding, this leading `+` is more representing the offset 
number, rather than the static number.  
   
   if we read the following sentence: 
   >A `file` field reserves the root field's ID plus six consecutive IDs for 
its sub-fields, assigned by the offsets above
   
   so i'd prefer to keep the `+`. 



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