mapleFU commented on code in PR #36955:
URL: https://github.com/apache/arrow/pull/36955#discussion_r1280110250
##########
cpp/src/parquet/column_writer.cc:
##########
@@ -2328,7 +2328,13 @@ std::shared_ptr<ColumnWriter>
ColumnWriter::Make(ColumnChunkMetaDataBuilder* met
const ColumnDescriptor* descr = metadata->descr();
const bool use_dictionary = properties->dictionary_enabled(descr->path()) &&
descr->physical_type() != Type::BOOLEAN;
- Encoding::type encoding = properties->encoding(descr->path());
+ Encoding::type default_encoding =
+ (descr->physical_type() == Type::BOOLEAN &&
+ properties->data_page_version() == ParquetDataPageVersion::V2)
Review Comment:
WriterProperties seems to have `ParquetVersion::PARQUET_1_0` support, maybe
we can set `default_encoding` within `WriterProperties`? Or we can extract a
`default_encoding` function here?
##########
cpp/src/parquet/properties.h:
##########
@@ -135,14 +136,13 @@ static constexpr int64_t DEFAULT_WRITE_BATCH_SIZE = 1024;
static constexpr int64_t DEFAULT_MAX_ROW_GROUP_LENGTH = 1024 * 1024;
static constexpr bool DEFAULT_ARE_STATISTICS_ENABLED = true;
static constexpr int64_t DEFAULT_MAX_STATISTICS_SIZE = 4096;
-static constexpr Encoding::type DEFAULT_ENCODING = Encoding::PLAIN;
static const char DEFAULT_CREATED_BY[] = CREATED_BY_VERSION;
static constexpr Compression::type DEFAULT_COMPRESSION_TYPE =
Compression::UNCOMPRESSED;
static constexpr bool DEFAULT_IS_PAGE_INDEX_ENABLED = false;
class PARQUET_EXPORT ColumnProperties {
public:
- ColumnProperties(Encoding::type encoding = DEFAULT_ENCODING,
+ ColumnProperties(std::optional<Encoding::type> encoding = std::nullopt,
Review Comment:
Personally I'm ok with this, but should we make constructor compatible?
--
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]
