You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by "mbrobbel (via GitHub)" <gi...@apache.org> on 2023/05/11 11:03:03 UTC

[GitHub] [parquet-format] mbrobbel opened a new pull request, #210: MINOR: Use `true` instead of `1` as default value for `is_compressed` bool field

mbrobbel opened a new pull request, #210:
URL: https://github.com/apache/parquet-format/pull/210

   I noticed that the default value for the optional boolean `is_compressed` field of the `DataPageHeaderV2` struct has a default value of `1`. According to the Thrift docs a boolean value is either `true` or `false`. 
   
   This currently works because the Apache Thrift compiler internally handles bools as ints:
   - https://github.com/apache/thrift/blob/3880a09565a9a1dad028b3679746eafac268c819/compiler/cpp/src/thrift/thriftl.ll#L208-L209
   - https://github.com/apache/thrift/blob/3880a09565a9a1dad028b3679746eafac268c819/compiler/cpp/src/thrift/main.cc#L748
   It may however not work with other Thrift compilers that are more strict about this.
   
   Based on the docs and [a test](https://github.com/apache/thrift/blob/3880a09565a9a1dad028b3679746eafac268c819/test/ThriftTest.thrift#L406) in the Thrift repository it seems that using `true` here is the correct way of defining a default for an optional bool field.
   
   


-- 
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: dev-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [parquet-format] wgtmac commented on pull request #210: PARQUET-2299: Use `true` instead of `1` as default value for `is_compressed` bool field

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #210:
URL: https://github.com/apache/parquet-format/pull/210#issuecomment-1545235777

   Thanks @mbrobbel! I have also added you as a contributor to the Apache Parquet JIRA project and assigned this to you.


-- 
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: dev-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [parquet-format] wgtmac merged pull request #210: PARQUET-2299: Use `true` instead of `1` as default value for `is_compressed` bool field

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #210:
URL: https://github.com/apache/parquet-format/pull/210


-- 
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: dev-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [parquet-format] wgtmac commented on pull request #210: MINOR: Use `true` instead of `1` as default value for `is_compressed` bool field

Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #210:
URL: https://github.com/apache/parquet-format/pull/210#issuecomment-1544259149

   Thanks! Would you mind creating a JIRA for this?


-- 
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: dev-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org