rdblue commented on code in PR #474: URL: https://github.com/apache/parquet-format/pull/474#discussion_r1881178197
########## src/main/thrift/parquet.thrift: ########## @@ -384,6 +384,11 @@ struct BsonType { * Embedded Variant logical type annotation */ struct VariantType { + // If the Variant is shredded the version of the shredding specification used. + // Required, if the the column is shredded. + // + // See VariantShredding.md for differences between versions. + 1: optional i8 shredding_version Review Comment: Would that mean that we allow mixed versions based on the metadata for each record? That seems like unnecessary complication to me. And it also means that older clients would fail at read time when they encounter a newer record, rather than failing quickly at the schema check stage. I think it makes sense to put the encoding version here and expect uniform encoding throughout a Parquet file. Writers should produce the latest encoding, not carry records through. For the shredding question, I would rather have one version of variant instead of evolving them separately. I think that would get confusing and there would be dependencies between them. For example, can you shred a type defined by a newer version of the encoding? Probably not, so bumping the encoding version also requires bumping the shredding version. So is it worth it to have one version number that can increase independently? I would combine them into a single variant version for shredding and encoding. -- 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: issues-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@parquet.apache.org For additional commands, e-mail: issues-h...@parquet.apache.org