emkornfield commented on code in PR #258: URL: https://github.com/apache/parquet-format/pull/258#discussion_r1632039686
########## CONTRIBUTING.md: ########## @@ -29,3 +29,125 @@ Recommendations and requirements for how to best contribute to Parquet. We striv ### License By contributing your code, you agree to license your contribution under the terms of the APLv2: https://github.com/apache/parquet-format/blob/master/LICENSE + +### Additions/Changes to the Format + +Note: This section applies to actual functional changes to the specification. +Fixing typos, grammar, and clarifying concepts that would not change the +semantics of the specification can be done as long a comitter feels comfortable +to merge them. When in doubt starting a discussion on the dev mailing list is +encouraged. + +The general steps for adding features to the format are as follows: + +1. Discuss changes on on the developer mailing list ([email protected]). + Often times it is helpful to link to a draft pull request to make the + discussion concrete. This step is complete when there lazy consensus. Part + of the consensus is whether it sufficient to provide 2 working + implementations as outlined in step 2 or if demonstration of the feature + with a down-stream query engine is necessary to justify the feature (e.g. + demonstrate performance improvements in Arrow's DataSet library or + Apache Data Fusion). + +2. Once a change has lazy consensus two implementations of the feature + demonstrating interopability must also be provided. One implementation MUST + be [parquet-java](http://github.com/apache/parquet-java). It is preferred + that the second implementation be + [parquet-cpp](https://github.com/apache/arrow) or + [parquet-rs](https://github.com/apache/arrow-rs), however at the discretion + of the PMC any open source Parquet implementation may be acceptable. + Implementations whose contributors actively participate in the community + (e.g. keep their feature matrix up-to-date on parquet-site) are more likely + to be considered. If discussed as a requirement in step one demonstration + of integration with a query engine is also required for this step. + +Unless otherwise discussed, it is expected the implementations will develop from +the main branch (i.e. backporting is not expected). + +3. After the first two steps are complete a formal vote is held on the Parquet + mailing list to officially ratify the feature. After the vote passes the + format change is merged into the parquet-format repository and it is expected + the changes from step 2 will also be merged soon after. Before merging into + Parquet-java a parquet-format release must be performed. + +#### General guidelines/preferences on additions. + +1. To the greatest extent possible changes should have an option for forwards + compatibility (old readers can still read files). +2. New encodings should be fully specified in this repository and ideally not + rely on an external dependencies for implementation (i.e. Parquet is the + source of truth for the encoding). +3. New compression mechanisms must have a pure Java implementation that can be + used as dependency in parquet-java. + +### Releases + +The Parquet community aims to do releases of the format package only as needed +when new features are introduced. If multiple new features are being proposed +simultaneously some features might be consolidated into the same release. +Guidance is provided below on when implementations should enable features added +to the specification. Due to confusion in the past over parquet versioning it +is not expected that there will be a 3.0 release of the specification in the +foreseeable future. + +### Compatibility and Feature Enablement + +For the purposes of this discussion we classify features into the following buckets: + +1. Backwards compatible. A file written under an older version of the format + should be readable under a newer version of the format. +2. Forwards compatible. A file written under a newer version of the format with + the enabled feature can be read under an older version of the format, but + some information might be missing or performance might be suboptimal. +3. Forward incompatible. A file written under a new version of the format with + the feature enabled cannot be read under and older version of the format + (e.g. Adding a new compression algorithm. + +The Parquet community hopes that new features are widely beneficial to users of Review Comment: > Specifically trying to proscribe the rollout cadence across loosly (at best) coordinated implementations in a document is unlikely to change behaviors and is likely to create confusion Is there specific language here that you find problematic here? With the latest revision, my intent was to make the recommendation only about preserving compatibility. i.e. forward incompatible changes must wait at least 2 years before an implementation decides to turn them on by default. I wonder if changing the phrasing from MAY to MUST NOT USE before would help clarify? > instead I would imagine that as new features were supported in parquet-rs there would be an appropriate writer configuration that users would decide to enable/disable as they deemed appropriate. I agree. Maybe this is a terminology issue. I view feature flag as a type of configuration, and I think I removed all recommendations for when, if ever, an implementation should turn something on by default (please let me know if something is still ambiguous or maybe I missed something in rephrasing). > Another potential improvement (orthogonal to this disucssion) would be to label new format additions with the date in which they were added to the spec, and perhaps track the date when support was added to other implementations Agreed, I'll add this to the process I thought the feature matrix that was already underreview would be a good place for this? I would like to avoid putting too many dates directly in the spec to avoid confusion. -- 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]
