wgtmac commented on code in PR #585:
URL: https://github.com/apache/parquet-format/pull/585#discussion_r3393072883


##########
LogicalTypes.md:
##########
@@ -635,7 +635,76 @@ The type has two type parameters:
 
 The sort order used for `GEOGRAPHY` is undefined. When writing data, no min/max
 statistics should be saved for this type and if such non-compliant statistics
-are found during reading, they must be ignored. 
+are found during reading, they must be ignored.
+
+### FILE
+
+`FILE` annotates a group that represents a reference to an external file, 
along with
+the minimum metadata required to read it. It is intended for use cases such as
+storing file inventories, manifests, and unstructured data references (e.g., 
images
+or audio files stored in object storage).
+
+The annotated group must contain the following fields, identified by name. 
Field IDs
+may also be used for projection:
+
+| Field    | Type   | Required |
+|----------|--------|----------|
+| `path`   | STRING | Yes      |
+| `size`   | INT64  | No       |
+| `offset` | INT64  | No       |
+| `etag`   | STRING | No       |
+
+#### Fields
+
+##### path
+
+An opaque path string to the referenced file (e.g., `s3://bucket/file.jpg`). 
No special
+encoding (e.g., URI encoding) is applied. This is the only required field.
+
+##### size
+
+The length of the content in bytes. Must be zero or a positive integer if 
provided.
+A value of 0 indicates an empty file. If not provided, the length of the 
referenced

Review Comment:
   Do we want to advise the reader to ignore negative value here?



##########
LogicalTypes.md:
##########
@@ -635,7 +635,76 @@ The type has two type parameters:
 
 The sort order used for `GEOGRAPHY` is undefined. When writing data, no min/max
 statistics should be saved for this type and if such non-compliant statistics
-are found during reading, they must be ignored. 
+are found during reading, they must be ignored.
+
+### FILE
+
+`FILE` annotates a group that represents a reference to an external file, 
along with
+the minimum metadata required to read it. It is intended for use cases such as
+storing file inventories, manifests, and unstructured data references (e.g., 
images
+or audio files stored in object storage).
+
+The annotated group must contain the following fields, identified by name. 
Field IDs
+may also be used for projection:
+
+| Field    | Type   | Required |
+|----------|--------|----------|
+| `path`   | STRING | Yes      |
+| `size`   | INT64  | No       |
+| `offset` | INT64  | No       |
+| `etag`   | STRING | No       |
+
+#### Fields
+
+##### path
+
+An opaque path string to the referenced file (e.g., `s3://bucket/file.jpg`). 
No special

Review Comment:
   If this is opaque, should we remove `No special encoding (e.g., URI 
encoding) is applied`? It seems valid if users want to apply URI encoding.



##########
src/main/thrift/parquet.thrift:
##########
@@ -468,6 +468,21 @@ struct GeographyType {
   2: optional EdgeInterpolationAlgorithm algorithm;
 }
 
+/**
+ * File logical type annotation
+ *
+ * Annotates a group that represents a reference to an external file.
+ * The group must contain the following fields identified by name:

Review Comment:
   > identified by name
   
   Do we need to mention its case sensitivity?



##########
LogicalTypes.md:
##########
@@ -635,7 +635,76 @@ The type has two type parameters:
 
 The sort order used for `GEOGRAPHY` is undefined. When writing data, no min/max
 statistics should be saved for this type and if such non-compliant statistics
-are found during reading, they must be ignored. 
+are found during reading, they must be ignored.
+
+### FILE
+
+`FILE` annotates a group that represents a reference to an external file, 
along with
+the minimum metadata required to read it. It is intended for use cases such as
+storing file inventories, manifests, and unstructured data references (e.g., 
images
+or audio files stored in object storage).
+
+The annotated group must contain the following fields, identified by name. 
Field IDs
+may also be used for projection:

Review Comment:
   ```suggestion
   The annotated group must contain the following fields, identified by name 
(not by order).
   Field IDs (if exist) may also be used for projection:
   ```



##########
LogicalTypes.md:
##########
@@ -635,7 +635,76 @@ The type has two type parameters:
 
 The sort order used for `GEOGRAPHY` is undefined. When writing data, no min/max
 statistics should be saved for this type and if such non-compliant statistics
-are found during reading, they must be ignored. 
+are found during reading, they must be ignored.
+
+### FILE
+
+`FILE` annotates a group that represents a reference to an external file, 
along with
+the minimum metadata required to read it. It is intended for use cases such as
+storing file inventories, manifests, and unstructured data references (e.g., 
images
+or audio files stored in object storage).
+
+The annotated group must contain the following fields, identified by name. 
Field IDs
+may also be used for projection:
+
+| Field    | Type   | Required |
+|----------|--------|----------|
+| `path`   | STRING | Yes      |
+| `size`   | INT64  | No       |
+| `offset` | INT64  | No       |
+| `etag`   | STRING | No       |
+
+#### Fields
+
+##### path
+
+An opaque path string to the referenced file (e.g., `s3://bucket/file.jpg`). 
No special
+encoding (e.g., URI encoding) is applied. This is the only required field.
+
+##### size
+
+The length of the content in bytes. Must be zero or a positive integer if 
provided.
+A value of 0 indicates an empty file. If not provided, the length of the 
referenced
+content is unknown and the entirety of the content can be read.
+
+##### offset
+
+A byte offset indicating the start of a content slice within the referenced 
file.
+If not provided, readers must treat the value as 0.
+If provided and non-zero, readers must seek to this offset and read `size` 
bytes to retrieve the referenced data.
+If `offset` is provided, `size` must also be provided.
+
+##### etag
+
+An eTag value provided by the storage system (e.g., from S3 or Azure Blob 
Storage).
+Can be used to detect whether the referenced file has been updated. If the 
reference
+points to a byte range within a file, the eTag applies to the entire file.
+
+#### Validation
+
+* The `path` field is required and must be present. Readers must reject a 
`FILE`-annotated
+  group that does not contain `path`.
+* If `offset` is present and non-zero, `size` must also be provided.
+* Additional metadata about the file (e.g., content type, modification 
timestamp) should

Review Comment:
   Why adding this restriction? Do we want to enforce Parquet reader 
implementations to validate against this rule? I can foresee that users may use 
another group type (a.k.a struct) to wrap a `FILE` type and a `VARIANT` type 
for extra metadata.



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