-
Notifications
You must be signed in to change notification settings - Fork 496
PARQUET-686: Clarifications about min-max stats. #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,17 +28,6 @@ namespace java org.apache.parquet.format | |
| * with the encodings to control the on disk storage format. | ||
| * For example INT16 is not included as a type since a good encoding of INT32 | ||
| * would handle this. | ||
| * | ||
| * When a logical type is not present, the type-defined sort order of these | ||
| * physical types are: | ||
| * * BOOLEAN - false, true | ||
| * * INT32 - signed comparison | ||
| * * INT64 - signed comparison | ||
| * * INT96 - signed comparison | ||
| * * FLOAT - signed comparison | ||
| * * DOUBLE - signed comparison | ||
| * * BYTE_ARRAY - unsigned byte-wise comparison | ||
| * * FIXED_LEN_BYTE_ARRAY - unsigned byte-wise comparison | ||
| */ | ||
| enum Type { | ||
| BOOLEAN = 0; | ||
|
|
@@ -219,12 +208,12 @@ struct Statistics { | |
| * Values are encoded using PLAIN encoding, except that variable-length byte | ||
| * arrays do not include a length prefix. | ||
| * | ||
| * These fields encode min and max values determined by SIGNED comparison | ||
| * These fields encode min and max values determined by signed comparison | ||
| * only. New files should use the correct order for a column's logical type | ||
| * and store the values in the min_value and max_value fields. | ||
| * | ||
| * To support older readers, these may be set when the column order is | ||
| * SIGNED. | ||
| * signed. | ||
| */ | ||
| 1: optional binary max; | ||
| 2: optional binary min; | ||
|
|
@@ -582,7 +571,9 @@ struct RowGroup { | |
| struct TypeDefinedOrder {} | ||
|
|
||
| /** | ||
| * Union to specify the order used for min, max, and sorting values in a column. | ||
| * Union to specify the order used for the min_value and max_value fields for a | ||
| * column. This union takes the role of an enhanced enum that allows rich | ||
| * elements (which will be needed for a collation-based ordering in the future). | ||
| * | ||
| * Possible values are: | ||
| * * TypeDefinedOrder - the column uses the order defined by its logical or | ||
|
|
@@ -592,6 +583,40 @@ struct TypeDefinedOrder {} | |
| * for this column should be ignored. | ||
| */ | ||
| union ColumnOrder { | ||
|
|
||
| /** | ||
| * The sort orders for logical types are: | ||
| * UTF8 - unsigned byte-wise comparison | ||
| * INT8 - signed comparison | ||
| * INT16 - signed comparison | ||
| * INT32 - signed comparison | ||
| * INT64 - signed comparison | ||
| * UINT8 - unsigned comparison | ||
| * UINT16 - unsigned comparison | ||
| * UINT32 - unsigned comparison | ||
| * UINT64 - unsigned comparison | ||
| * DECIMAL - signed comparison | ||
| * DATE - signed comparison | ||
| * TIME_MILLIS - signed comparison | ||
| * TIME_MICROS - signed comparison | ||
| * TIMESTAMP_MILLIS - signed comparison | ||
| * TIMESTAMP_MICROS - signed comparison | ||
| * INTERVAL - unsigned comparison | ||
| * JSON - undefined | ||
| * BSON - undefined | ||
| * LIST - undefined | ||
| * MAP - undefined | ||
| * | ||
| * In the absence of logical types, the sort order is determined by the physical type: | ||
| * BOOLEAN - false, true | ||
| * INT32 - signed comparison | ||
| * INT64 - signed comparison | ||
| * INT96 - signed comparison | ||
| * FLOAT - signed comparison | ||
| * DOUBLE - signed comparison | ||
| * BYTE_ARRAY - unsigned byte-wise comparison | ||
| * FIXED_LEN_BYTE_ARRAY - unsigned byte-wise comparison | ||
| */ | ||
| 1: TypeDefinedOrder TYPE_ORDER; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add a custom sort order for Impala timestamp values until
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't remember in which context the discussion was but I think INT96 timestamps should be sortable correctly with signed comparison. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you are referring to Impala timestamps, then I believe signed comparison is not sufficient. The comparator should compare the date value first (last 4 bytes) and then the nanoseconds (first 8 bytes)?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is something that probably someone else also should review (cc @rdblue ;) ) but I guess that due to storing the values as little endian, this should be correct. Please don't take this for granted, I hadn't had to deal with endianness in the last years explicitly, so I might have good this wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I doubt if little endian layout that works at the byte level can help with this multi-byte value comparison.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is the timestamp '2000-01-01 12:34:56' stored as an int96: Since 117253024523396126668760320 = 0x60FD4B3229000059682500, the 12 bytes are 00 60 FD 4B 32 29 00 00 | 59 68 25 00, where | shows the boundary between the time and the date parts. 00 60 FD 4B 32 29 00 00 is the time part, if we reverse the bytes we get 0x000029324BFD6000 = 45296 * 10^9 nanoseconds = 45296 seconds = 12 hours + 34 minutes + 56 seconds. 59 68 25 00 is the date part, if we reverse the bytes we get 0x00256859 = 2451545 as the Julian day number, which corresponds to 2000-01-01. To correctly sort these values without interpreting them as timestamps, the bigger unit (date) should precede the smaller unit (time). In this case they are in the opposite order, but they are also stored with little-endian byte-order (individually), which means that they will be in the correct order if we interpret the whole value in a little-endian manner. So, for correct ordering based purely on numerical value, in comparisons the example above should not be interpreted as 0x0060FD4B3229000059682500 = 117253024523396126668760320 like parquet-tools did, but as 0x00256859000029324BFD6000 = 45223023200227578716446720 instead. Or, to put it more simply, a byte-by-byte comparison starting from the end of the values results in the correct ordering. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the clarification! An Int96 intrinsic hardware type would have handled the value as is. Otherwise a byte-by-byte comparison in the reverse order is needed. |
||
| } | ||
|
|
||
|
|
@@ -626,11 +651,16 @@ struct FileMetaData { | |
| 6: optional string created_by | ||
|
|
||
| /** | ||
| * Sort order used for each column in this file. | ||
| * Sort order used for the min_value and max_value fields of each column in | ||
| * this file. Each sort order corresponds to one column, determined by its | ||
| * position in the list, matching the position of the column in the schema. | ||
| * | ||
| * Without column_orders, the meaning of the min_value and max_value fields is | ||
| * undefined. To ensure well-defined behaviour, if min_value and max_value are | ||
| * written to a Parquet file, column_orders must be written as well. | ||
| * | ||
| * If this list is not present, then the order for each column is assumed to | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed this sentence since it no longer applied. (It was a leftover from the early stages of PR #46 when this list was meant to modify the behavior of the @rdblue @julienledem How should we define the meaning of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the column order for a column is not set, Impala only reads stats for numeric types (parquet-column-stats.cc#L130). We treat the values as having been written by older versions of parquet-mr, thus only using stats for types that did not have known issues in the past.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And which fields does Impala read the statistics from if the column order is not set, The fact that the statistics are in the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The check is done for new-style stats, too, since if the column order is unknown, there does not seem to be a safe ordering to assume for all other types. This should not happen with new readers, but some may still use the new fields and not set the column order. For old fields, we ignore them if column_order is set to anything but TYPE_ORDER. Otherwise we only read them for numeric types. |
||
| * be Signed. In addition, min and max values for INTERVAL or DECIMAL stored | ||
| * as fixed or bytes should be ignored. | ||
| * The obsolete min and max fields are always sorted by signed comparison | ||
| * regardless of column_orders. | ||
| */ | ||
| 7: optional list<ColumnOrder> column_orders; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lekv Since INT96 timestamps have the INT96 physical type but no logical type, according to these rules signed comparison should be applied to them. But if I remember correctly, it was discussed that signed comparison was not adequate for INT96 timestamps. How did you handle this case in Impala?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I came across this line of code. According to this, INT96 should use unsigned comparison. I think this comment here should read "unsigned" as well. @julienledem , what is your take on this?