Re: [PR] GH-473: Add shredding version [parquet-format]
wgtmac commented on PR #474: URL: https://github.com/apache/parquet-format/pull/474#issuecomment-2689851641 What is the progress of this? Should we merge or close it in order to release the parquet-format 2.11.0? @emkornfield @rdblue -- 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
Re: [PR] GH-473: Add shredding version [parquet-format]
emkornfield commented on code in PR #474:
URL: https://github.com/apache/parquet-format/pull/474#discussion_r1882488101
##
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.
In discussions on footer optimization most of the community seemed to lean
towards not putting in extra metadata to allow for quick failure. Putting this
value in the schema I think also disallows simple merging of non-shredded
values between two different versions (and as a strong indication that the
variant is shredded, I think it is an interesting API consideration on whether
we separate API for interrogating shredded columns or return the schema "as
is"). The reason I want to version the shredding is to ensure we aren't
reliant on detecting column name differences to determine version but maybe
even this is premature.
> 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.
We can add it but I think it is just as reasonable require if there is ever
a V2, that all values within a row group are consistent which I think gives
uniformity at a reasonable level?
> 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.
Right, but I don't think the inverse is true, it is likely we can have
different shredding versions based on the same binary encoded format which is
why I think versioning them separately makes sense.
--
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
Re: [PR] GH-473: Add shredding version [parquet-format]
emkornfield commented on code in PR #474:
URL: https://github.com/apache/parquet-format/pull/474#discussion_r1882488101
##
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.
In discussions on footer optimization most of the community seemed to lean
towards not putting in extra metadata to allow for quick failure. Putting this
value in the schema I think also disallows simple merging of non-shredded
values between two different versions. The reason I want to version the
shredding is to ensure we aren't reliant on detecting column name differences
to determine version but maybe even this is premature.
> 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.
We can add it but I think it is just as reasonable require if there is ever
a V2, that all values within a row group are consistent which I think gives
uniformity at a reasonable level?
> 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.
Right, but I don't think the inverse is true, it is likely we can have
different shredding versions based on the same binary encoded format which is
why I think versioning them separately makes sense.
--
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
Re: [PR] GH-473: Add shredding version [parquet-format]
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
Re: [PR] GH-473: Add shredding version [parquet-format]
emkornfield commented on code in PR #474:
URL: https://github.com/apache/parquet-format/pull/474#discussion_r1879063140
##
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:
encoding version is already embedded in metadata, so I thought wouldn't be
needed here. Shredding version is only encoded implicitly based on the spec
without this.
I think shredding should probably evolve differently from encoding
(hopefully we won't need to evolve any for some time). But shredding seems
like we need at least some more experimentation to determine if there are other
forms that might be better.
I might not be fully understanding the question here though.
--
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
Re: [PR] GH-473: Add shredding version [parquet-format]
rdblue commented on code in PR #474:
URL: https://github.com/apache/parquet-format/pull/474#discussion_r1879054459
##
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:
What about encoding version? If we include the encoding version, that would
help us change it in the future. For shredding, do we want to have a single
version that includes shredding or a separate version for shredding?
--
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
