wgtmac commented on code in PR #40957:
URL: https://github.com/apache/arrow/pull/40957#discussion_r1548864310
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -2374,11 +2377,8 @@ std::shared_ptr<ColumnWriter>
ColumnWriter::Make(ColumnChunkMetaDataBuilder* met
descr->physical_type() != Type::BOOLEAN;
Encoding::type encoding = properties->encoding(descr->path());
if (encoding == Encoding::UNKNOWN) {
- encoding = (descr->physical_type() == Type::BOOLEAN &&
- properties->version() != ParquetVersion::PARQUET_1_0 &&
- properties->data_page_version() == ParquetDataPageVersion::V2)
- ? Encoding::RLE
- : Encoding::PLAIN;
+ encoding = ChooseFallbackEncoding(descr->physical_type(),
properties->version(),
Review Comment:
nit: rename it to `ChooseNonDictEncoding`. It would be weird to see
`fallback` here.
##########
cpp/src/parquet/encoding.h:
##########
@@ -466,4 +467,8 @@ std::unique_ptr<typename EncodingTraits<DType>::Decoder>
MakeTypedDecoder(
return std::unique_ptr<OutType>(dynamic_cast<OutType*>(base.release()));
}
+Encoding::type ChooseFallbackEncoding(Type::type data_type,
Review Comment:
```suggestion
PARQUET_EXPORT Encoding::type ChooseFallbackEncoding(Type::type data_type,
```
IIUC, this may fix the Windows CI.
##########
cpp/src/parquet/encoding.cc:
##########
@@ -3988,4 +3988,21 @@ std::unique_ptr<Decoder> MakeDictDecoder(Type::type
type_num,
}
} // namespace detail
+
+// Informed heavily by
https://github.com/apache/parquet-format/blob/master/Encodings.md
+// Should we also consider if we're in dictionary encoding mode? or assume no
for the
+// moment?
+Encoding::type ChooseFallbackEncoding(Type::type data_type,
+ ParquetVersion::type parquet_version,
+ ParquetDataPageVersion datapage_version)
{
+ if (data_type == Type::BOOLEAN && parquet_version !=
ParquetVersion::PARQUET_1_0 &&
+ datapage_version == ParquetDataPageVersion::V2) {
+ return Encoding::RLE;
+ } else if (data_type == Type::BYTE_ARRAY) {
Review Comment:
Do you want to support other data types as well?
--
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]