If the goal of this exercise is to avoid confusion, I agree with Michah that updating parquet.thrift is best. Here [3] is a PR to update the thrift file to clarify that the field is not written by all writers and is not read by many.
In my opinion any backwards incompatible changes do nothing other risk making parquet files less compatible with the ecosystem While removing the field is a technically more elegant solution (would make code cleaner), it could only cause potential incompatibilities for users. I prefer to have more complex code but a better user experience. BTW the Rust parquet writer sets the file_offset field[1] but does not appear to use it on read. Instead it assumes column_metadata is present[2] Andrew [1]: https://github.com/apache/arrow-rs/blob/b3f06f6cc4d4f4431a1f86cfc9f30de3a1fc1a1b/parquet/src/column/writer/mod.rs#L904-L907 [2]: https://github.com/apache/arrow-rs/blob/ed018a34d996590544fe5e833cb601bf46e9758e/parquet/src/file/metadata.rs#L673-L672 [3]: https://github.com/apache/parquet-format/pull/439 On Tue, Jun 25, 2024 at 4:40 AM Alkis Evlogimenos <alkis.evlogime...@databricks.com.invalid> wrote: > We need a mechanism to remove fields. Typically this would involve some > time horizon. > > I suggest we establish a deprecation horizon now, say 3y, and start the > clocks ticking. Plus some convention for marking deprecated fields because > the thrift IDL lacks a way to do this in code. I propose the annotation `// > DEPRECATED-EOL-20270421` followed by a description on what happens in the > interim. For example for this field: > > ``` > /** Byte offset in file_path to the ColumnMetaData **/ > // DEPRECATED-EOL-20270421 > // Before 20240625 this field was required. Since then it was made > optional. New writers MUST write it util EOL to support old readers. > 2: optional i64 file_offset > ``` > > > On Tue, Jun 25, 2024 at 9:23 AM Micah Kornfield <emkornfi...@gmail.com> > wrote: > > > > > > > but I'm not clear on how that will > > > impact existing parsers. > > > > > > This can break older parsers, that validate required fields are in fact > > present. I think it would be best to just update documentation on the > > current state of affairs, and let implementations update accordingly if > > necessary. > > > > On Mon, Jun 24, 2024 at 3:21 PM Ed Seidl <etse...@live.com> wrote: > > > > > Resurrecting a thread from earlier in the month regarding inconsistent > > > use of the file_offset field [1][2]. It seems like the preferred path > > > forward is to deprecate this (AFAICT) unused field to prevent further > > > confusion. If there are no violent objections, I'll submit a PR to do > so > > > in a few days. > > > > > > One question I have, though, is how to handle the requiredness of the > > > file_offset (currently required) and meta_data (currently optional) > > > fields in ColumnChunk. I'd prefer to switch them, and make file_offset > > > optional and meta_data required, but I'm not clear on how that will > > > impact existing parsers. I believe most (all) implementations ignore > > > file_offset anyway, and expect meta_data to be present, so maybe this > is > > > a non-issue. > > > > > > Thanks, > > > Ed > > > > > > [1] https://lists.apache.org/thread/q5r43ks61q4wcbvwsk1jyw4h30fvg68t > > > [2] https://github.com/apache/parquet-java/pull/1369 > > > > > >
