etseidl commented on code in PR #588:
URL: https://github.com/apache/parquet-format/pull/588#discussion_r3453666983
##########
CONTRIBUTING.md:
##########
@@ -131,62 +138,67 @@ For the purposes of this discussion we classify features
into the following buck
3. Forward incompatible. A file written under a newer version of the format
with
the feature enabled cannot be read under an older version of the format
(e.g.
- adding and using a new compression algorithm). It is expected any feature in
+ adding and using a new compression algorithm). Any feature in
this category will provide a signal to older readers, so they can
- unambiguously determine that they cannot properly read the file (e.g. via
- adding a new value to an existing enum).
+ unambiguously determine that they cannot properly read the file. There are
+ two mechanisms for providing this functionality:
+ 1. Adding values to existing enums/unions (e.g. Sort Order,
+ encodings, compression) is covered by this mechanism.
+ 2. Reserving a new bit in the [PARX](ParxMagicNumber.md) footer feature
bitmap covers other
+ structural changes that an older reader would not be able to accept.
New features are intended to be widely beneficial to users of Parquet, and
therefore it is hoped third-party implementations will adopt them quickly after
-they are introduced. It is assumed that writing new parts of the format, and
-especially forward incompatible features, will be configured with a feature
flag
-defaulted to "off", and at some future point the feature is turned on by
default
-(reading of the new feature will typically be enabled without configuration or
-defaulted to on). Some amount of lead time is desirable to ensure a critical
-mass of Parquet implementations support a feature to avoid compatibility issues
+they are introduced. It is expected that implementations will provide a
configuration
+mechanism for users to enable features. It is recommended that implementations
provide
+at least a way to enable all relevant features given a specification version
+(e.g. major and minor version). In addition, implementations might choose to
+enable features at a finer-grained level, with feature flags initially
defaulted to "off".
+
+Some amount of lead time is desirable to ensure a critical
+mass of Parquet implementations support a given specification version
across the ecosystem. Therefore, the Parquet PMC gives the following
-recommendations for managing features:
+recommendations for managing the default specification version used for
writing:
1. Backward compatibility is the concern of implementations but given the
ubiquity of Parquet and the length of time it has been used, libraries
should
support reading older versions of the format to the greatest extent
possible.
-2. Forward compatible features/changes may be enabled and used by default in
+2. Minor format versions may be enabled and used by default in
implementations once the parquet-format containing those changes has been
- formally released. For features that may pose a significant performance
+ formally released. For releases that may pose a significant performance
regression to older format readers, libraries should consider delaying
default
- enablement until 1 year after the release of the parquet-java implementation
- that contains the feature implementation.
+ enablement until 1 year after the parquet-java implementation for that
format
+ version is released.
-3. Forward incompatible features/changes should not be turned on by default
- until 2 years after the parquet-java implementation containing the feature
is
- released. It is recommended that changing the default value for a forward
- incompatible feature flag should be clearly advertised to consumers (e.g.
via
+3. Major version upgrades should not be enabled by default
+ until 2 years after the parquet-java implementation for the specification
has been
Review Comment:
Given the structural and procedural changes here, I wonder if the 2 year
suggestion can be softened. Maybe just say these changes should not be enabled
as defaults until sufficient time has been given for downstream consumers to
adapt.
##########
ParxMagicNumber.md:
##########
@@ -0,0 +1,89 @@
+# PARX Parquet Format Specification
+
+This specification details a new magic number and associated fixed length
footer metadata changes
+that accompany the footer.
+
+## Motivation
+
+Most parts of the parquet specification lend themselves naturally to
comptability checks
+when a new feature is added (e.g. encodings and compression values have an
enum value added)
+and fail appropriately.
+However, some semantic changes or footer changes are impossible to communicate
appropriately
+within existing structures (e.g. changing the serialization of the footer).
The motivation
+for the new magic number and layout is to accomodate the latter set of changes
by introducing
+a new extensible mechanism for readers to detect these changes and fail
accordingly.
+
+## Design Motivations
+
+* Provide a mechanism to only introduce a single new magic number for parquet
that can
+ last at least a decade.
+* Provide integrity checks for the footer.
+* Provide the ability for readers to have a granular understanding of
structural and semantic
+ backward incompatible features that are required to read a particular file.
+
+## File Layout
+
+A PARX file has the same overall structure as a standard Parquet file, with
two differences:
+the leading and trailing magic bytes are `PARX` instead of `PAR1/PARE`, and
the trailing footer is
+16 bytes instead of 8.
+
+The file layout is as follows:
+
+```
++-----------+-------------------+--------------------+-------------+
+| 'PARX' | File Data | Footer Metadata | Footer tail |
+| (4 bytes) | (variable length) | (variable length) | (16 bytes) |
++-----------+----------+--------+--------------------+-------------+
+```
+
+All multi-byte integer fields are **little-endian**.
+
+
+### PARX Footer Tail — 16 bytes
+
+```
++------------------+-----------+----------+--------+
+| metadata_len | flags | crc32 | 'PARX' |
++------------------+-----------+----------+--------+
+ offset 0 offset 4 offset 8 offset 12
+```
+
+| Field | Type | Offset | Description
|
+|----------------|---------|--------|-------------------------------------------------------------------|
+| `metadata_len` | i32 LE | 0 | Byte length of the Thrift-encoded
`FileMetaData` block |
+| `flags` | u32 LE | 4 | Feature flags (see [Feature
Flags](#feature-flags)) |
+| `crc32` | u32 LE | 8 | CRC32 checksum (see [Integrity
Check](#integrity-check)) |
+| `magic` | [u8; 4] | 12 | Always the bytes `P A R X` (0x50 0x41
0x52 0x58) |
+
+## Feature Flags
+
+The `flags` field is a 32-bit bitfield. A reader **must** reject any file
whose `flags` field
+contains bits that are not recognized or not supported, because unknown flags
may imply structural changes
+to the metadata or semantic changes to the file layout that the reader cannot
properly interpret.
+
+The PARX format is independent of the `version` field in `FileMetaData`; a
file may use the PARX
+magic number regardless of which specification version its metadata declares.
+
+| Bit Index| Name | Description
|
+|----------|-------------------------|-------------------------------------------------------------------------------------------------------------|
+| 0 | `ENCRYPTED_FOOTER` | The `FileMetaData` block is encrypted
(equivalent to the `PARE` format). |
Review Comment:
Should we add these to parquet.thrift as constants?
--
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]
