PFischbeck commented on code in PR #624:
URL: https://github.com/apache/parquet-format/pull/624#discussion_r4123520849
##########
src/main/thrift/parquet.thrift:
##########
@@ -314,6 +314,12 @@ struct Statistics {
* or DOUBLE, or logical type is FLOAT16.
* If this field is not present, readers MUST assume NaNs may be present
* (i.e. MUST assume nan_count > 0 and MAY NOT assume nan_count == 0).
+ * If the column is the element leaf of a VECTOR, whose elements MUST by
+ * convention always be finite (see LogicalTypes.md) nan_count MUST be
+ * zero when present.
+ * If the column is the element leaf of a VECTOR, whose elements MUST by
Review Comment:
I think due to some repeated editing, this paragraph is now present twice
with slight variations.
##########
src/main/thrift/parquet.thrift:
##########
@@ -1366,7 +1390,10 @@ struct ColumnIndex {
* A list containing the number of NaN values for each page. Only present
* for columns of physical type FLOAT or DOUBLE, or logical type FLOAT16.
* If this field is not present, readers MUST assume that there might be
- * NaN values in any page.
+ * NaN values in any page, except if the column is an element leaf of a
+ * VECTOR, whose elements MUST by convention always be finite (see
+ * LogicalTypes.md), writers SHOULD omit nan_counts field and
+ * readers MAY assume nan_counts == 0.
Review Comment:
This sentence structure is a bit confusing, due to the added `, writers
SHOULD omit...`. How about splitting the sentence:
```
* NaN values in any page, except if the column is an element leaf of a
* VECTOR, whose elements MUST by convention always be finite (see
* LogicalTypes.md). In this case, writers SHOULD omit nan_counts field and
* readers MAY assume nan_counts == 0.
```
##########
src/main/thrift/parquet.thrift:
##########
@@ -479,6 +485,22 @@ struct GeographyType {
struct FileType {
}
+/**
+* Fixed length vector logical type annotation
+*
+* Annotates the outer group of a canonical 3-level LIST structure whose
+* element leaf is a required numeric or boolean Parquet primitive type.
+* Group elements, including LIST, MAP, and VECTOR, are not allowed.
+* Every non-null vector MUST contain exactly num_elements of this type and
+* these elements MUST be finite: NaN, positive or negative infinity are not
+* allowed. Vector nullability is determined by the outer group being optional
+* or required.
+* See LogicalTypes.md for details.
Review Comment:
Agreed, this can be shortened, similar to how it is done for the `FileType`.
##########
LogicalTypes.md:
##########
@@ -1097,6 +1097,88 @@ optional group my_map (MAP_KEY_VALUE) {
}
```
+### Vectors
+
+`VECTOR` is used to annotate fixed-length ordered sequences of finite and
+non-null elements.
+
+`VectorType` annotation has one required parameter, `num_elements`, which is
+the number of elements in each non-null vector and must be greater than zero.
+
+`VECTOR` must always annotate the canonical 3-level structure:
Review Comment:
nit: should `must` be capitalized in this and the next paragraph?
##########
LogicalTypes.md:
##########
@@ -1097,6 +1097,88 @@ optional group my_map (MAP_KEY_VALUE) {
}
```
+### Vectors
+
+`VECTOR` is used to annotate fixed-length ordered sequences of finite and
+non-null elements.
+
+`VectorType` annotation has one required parameter, `num_elements`, which is
+the number of elements in each non-null vector and must be greater than zero.
+
+`VECTOR` must always annotate the canonical 3-level structure:
+
+```
+<vector-repetition> group <name> (VECTOR(<num_elements>)) {
+ repeated group list {
+ required <element-type> element;
+ }
+}
+```
+
+* The outer level must be a group with `logicalType` set to `VECTOR` and
+ `converted_type` set to `LIST`. Its repetition must either be `optional` or
+ `required` and determines whether the vector may be null. It must contain a
+ single field named `list`.
+* The middle level must be a repeated group named `list` with a single field
+ named `element`.
+* The element MUST be a `required` primitive field, individual elements MUST
+ NOT be null. Group elements, including `LIST`, `MAP`, and `VECTOR`, are not
+ allowed.
+
+The element types supported are:
+
+* unannotated `BOOLEAN`, `FLOAT` or `DOUBLE`
+* `INT32` or `INT64`, either unannotated or annotated with `INTEGER` or
+ `DECIMAL`
+* `FIXED_LEN_BYTE_ARRAY` annotated with `FLOAT16` or `DECIMAL`.
+
+The 2-level structures accepted for `LIST` under its backward-compatibility
+rules are not valid for `VECTOR`.
+
+Every numeric element MUST be finite. For `FLOAT`, `DOUBLE`, and `FLOAT16`,
+this excludes NaN, positive infinity, and negative infinity. Whole vectors
+may still be null when the outer group is `optional`.
+
+For example, a nullable vector containing 768 required `FLOAT` elements is:
+
+```
+optional group embedding (VECTOR(768)) {
+ repeated group list {
+ required float element;
+ }
+}
+```
+
+Every non-null vector must contain exactly `num_elements` elements.
+Because `num_elements` is greater than zero, a non-null vector cannot be empty.
+Writers MUST enforce element count, non-null-element, and finite-element
+requirements. Readers MAY rely on these requirements without validating them. A
+reader that detects a different element count, a null element, or a non-finite
+element MUST treat the data as invalid.
+
+For example, with `num_elements = 3` and a `required float element`,
+`[1.0, 2.0, 3.0]` is valid. `[1.0, null, 3.0]`, `[null, null, null]`,
+`[1.0, NaN, 3.0]`, and `[1.0, +Infinity, 3.0]` are invalid. A null vector
+is valid only when the outer group is `optional`.
Review Comment:
We could add an invalid example with 2 or 4 elements here.
--
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]
