pitrou commented on code in PR #38163:
URL: https://github.com/apache/arrow/pull/38163#discussion_r1352747786
##########
cpp/src/parquet/column_writer_test.cc:
##########
@@ -767,9 +784,12 @@ TEST_F(TestValuesWriterInt32Type, OptionalNullValueChunk) {
class TestBooleanValuesWriter : public TestPrimitiveWriter<BooleanType> {
public:
- void TestWithEncoding(ParquetVersion::type version, Encoding::type encoding)
{
+ void TestWithEncoding(ParquetVersion::type version,
+ ParquetDataPageVersion data_page_version,
+ Encoding::type encoding) {
this->SetUpSchema(Repetition::REQUIRED);
auto writer = this->BuildWriter(SMALL_SIZE, ColumnProperties(), version,
+ ParquetDataPageVersion::V1,
Review Comment:
Why not `data_page_version`?
##########
cpp/src/parquet/column_writer_test.cc:
##########
@@ -789,12 +809,18 @@ class TestBooleanValuesWriter : public
TestPrimitiveWriter<BooleanType> {
// PARQUET-764
// Correct bitpacking for boolean write at non-byte boundaries
TEST_F(TestBooleanValuesWriter, AlternateBooleanValues) {
- TestWithEncoding(ParquetVersion::PARQUET_1_0, Encoding::PLAIN);
+ for (auto data_page_version :
+ {ParquetDataPageVersion::V1, ParquetDataPageVersion::V2}) {
+ TestWithEncoding(ParquetVersion::PARQUET_1_0, data_page_version,
Encoding::PLAIN);
+ }
}
// Default encoding for boolean is RLE when using V2 pages
TEST_F(TestBooleanValuesWriter, RleEncodedBooleanValues) {
- TestWithEncoding(ParquetVersion::PARQUET_2_4, Encoding::RLE);
+ TestWithEncoding(ParquetVersion::PARQUET_2_4, ParquetDataPageVersion::V1,
+ Encoding::PLAIN);
+ TestWithEncoding(ParquetVersion::PARQUET_2_4, ParquetDataPageVersion::V2,
+ Encoding::RLE);
Review Comment:
Well, `RLE` is always in the encodings regardless, since it's used to RL/DL,
right?
--
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]
