You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/09/30 19:01:13 UTC
[GitHub] [arrow] tachyonwill opened a new pull request #11281: PARQUET-2067: Fix Parquet null count stats for enclosing null lists
tachyonwill opened a new pull request #11281:
URL: https://github.com/apache/arrow/pull/11281
For repeated columns, the null count should include cases where the
enclosing list(s) is null of empty. For example `[[0, 1], None, [2,
None, 3]]` should have a null count of 2.
Co-authored-by: Micah Kornfield <mi...@google.com>
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#issuecomment-935731426
@tachyonwill Do you have an id on https://issues.apache.org/ so that this issue can be assigned 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tachyonwill commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
tachyonwill commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r719776114
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -3896,6 +3896,41 @@ INSTANTIATE_TEST_SUITE_P(WriteDictionary, TestArrowWriteDictionary,
::testing::Values(ParquetDataPageVersion::V1,
ParquetDataPageVersion::V2));
+TEST(TestArrowWriteNestedDictionary, TestNullCount) {
+ std::shared_ptr<::arrow::Array> dictionary = ArrayFromJSON(::arrow::utf8(), R"(["z"])");
Review comment:
If you are asking why I added a dictionary specific test, it is to test the changes to WriteArrowDictionary; the other tests were not sufficient.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r722150734
##########
File path: cpp/src/parquet/arrow/arrow_statistics_test.cc
##########
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "gtest/gtest.h"
+
+#include "arrow/table.h"
+#include "arrow/testing/gtest_util.h"
+
+#include "parquet/api/reader.h"
+#include "parquet/api/writer.h"
+
+#include "parquet/arrow/schema.h"
+#include "parquet/arrow/writer.h"
+#include "parquet/file_writer.h"
+#include "parquet/test_util.h"
+
+using arrow::ArrayFromJSON;
+using arrow::Buffer;
+using arrow::default_memory_pool;
+using arrow::ResizableBuffer;
+using arrow::Table;
+
+using arrow::io::BufferReader;
+
+namespace parquet {
+namespace arrow {
+
+// PARQUET-2067: Tests that nulls from parent fields are included in null statistics when
+// array is dictionary encoded.
+TEST(TestArrowWriteNestedDictionary, TestNullCount) {
Review comment:
Can this be folded into ParameterizedStatisticsTest below? It doesn't seem there's any specific code.
##########
File path: cpp/src/parquet/statistics.h
##########
@@ -282,8 +282,8 @@ class TypedStatistics : public Statistics {
/// \brief Batch statistics update with supplied validity bitmap
virtual void UpdateSpaced(const T* values, const uint8_t* valid_bits,
Review comment:
Thank 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tachyonwill commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
tachyonwill commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r721740579
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -4191,5 +4226,107 @@ TEST(TestArrowReadDeltaEncoding, DeltaBinaryPacked) {
}
#endif
+struct StatisticsTestParam {
+ std::shared_ptr<::arrow::Table> table;
+ int expected_null_count;
+ // This is the non-null count and not the num_values in the page headers.
+ int expected_value_count;
+ std::string expected_min;
+ std::string expected_max;
+};
+
+class ParameterizedStatisticsTest : public ::testing::TestWithParam<StatisticsTestParam> {
+};
+
+std::string GetManyEmptyLists() {
+ std::string many_empty_lists = "[";
+ for (int i = 0; i < 2000; ++i) {
+ many_empty_lists += "[],";
+ }
+ many_empty_lists += "[1,2,3,4,5,6,7,8,null]]";
+ return many_empty_lists;
+}
+
+TEST_P(ParameterizedStatisticsTest, NoNullCountWrittenForRepeatedFields) {
+ std::shared_ptr<::arrow::ResizableBuffer> serialized_data = AllocateBuffer();
Review comment:
done.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #11281:
URL: https://github.com/apache/arrow/pull/11281
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r719689921
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -3896,6 +3896,41 @@ INSTANTIATE_TEST_SUITE_P(WriteDictionary, TestArrowWriteDictionary,
::testing::Values(ParquetDataPageVersion::V1,
ParquetDataPageVersion::V2));
+TEST(TestArrowWriteNestedDictionary, TestNullCount) {
+ std::shared_ptr<::arrow::Array> dictionary = ArrayFromJSON(::arrow::utf8(), R"(["z"])");
Review comment:
any reason to use dictionary here?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tachyonwill commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
tachyonwill commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r721740710
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -4191,5 +4226,107 @@ TEST(TestArrowReadDeltaEncoding, DeltaBinaryPacked) {
}
#endif
+struct StatisticsTestParam {
Review comment:
created arrow_statistics_test.cc
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tachyonwill commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
tachyonwill commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r722467015
##########
File path: cpp/src/parquet/arrow/arrow_statistics_test.cc
##########
@@ -0,0 +1,171 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "gtest/gtest.h"
+
+#include "arrow/table.h"
+#include "arrow/testing/gtest_util.h"
+
+#include "parquet/api/reader.h"
+#include "parquet/api/writer.h"
+
+#include "parquet/arrow/schema.h"
+#include "parquet/arrow/writer.h"
+#include "parquet/file_writer.h"
+#include "parquet/test_util.h"
+
+using arrow::ArrayFromJSON;
+using arrow::Buffer;
+using arrow::default_memory_pool;
+using arrow::ResizableBuffer;
+using arrow::Table;
+
+using arrow::io::BufferReader;
+
+namespace parquet {
+namespace arrow {
+
+// PARQUET-2067: Tests that nulls from parent fields are included in null statistics when
+// array is dictionary encoded.
+TEST(TestArrowWriteNestedDictionary, TestNullCount) {
Review comment:
done
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #11281: PARQUET-2067: Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#issuecomment-931584990
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r721424568
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -3896,6 +3896,41 @@ INSTANTIATE_TEST_SUITE_P(WriteDictionary, TestArrowWriteDictionary,
::testing::Values(ParquetDataPageVersion::V1,
ParquetDataPageVersion::V2));
+TEST(TestArrowWriteNestedDictionary, TestNullCount) {
+ std::shared_ptr<::arrow::Array> dictionary = ArrayFromJSON(::arrow::utf8(), R"(["z"])");
+ std::shared_ptr<::arrow::Array> indices =
+ ArrayFromJSON(::arrow::int32(), R"([0, null, 0])");
+ std::shared_ptr<::arrow::Array> offsets =
+ ArrayFromJSON(::arrow::int32(), R"([null, 0, null, null, null, 3])");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<::arrow::Array> dict_encoded,
+ ::arrow::DictionaryArray::FromArrays(indices, dictionary));
+ // values looks like [null, ["z", null, "z"], null, null, null]
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<Array> values,
+ ::arrow::ListArray::FromArrays(*offsets, *dict_encoded, default_memory_pool()));
+ std::shared_ptr<::arrow::Schema> schema =
+ ::arrow::schema({::arrow::field("values", values->type())});
+ std::shared_ptr<::arrow::Table> table = ::arrow::Table::Make(schema, {values});
+
+ std::shared_ptr<::arrow::ResizableBuffer> serialized_data = AllocateBuffer();
+ auto out_stream = std::make_shared<::arrow::io::BufferOutputStream>(serialized_data);
+ std::unique_ptr<FileWriter> writer;
+ ASSERT_OK(FileWriter::Open(*table->schema(), default_memory_pool(), out_stream,
+ default_writer_properties(),
+ default_arrow_writer_properties(), &writer));
+ ASSERT_OK(writer->WriteTable(*table, std::numeric_limits<int64_t>::max()));
+ ASSERT_OK(writer->Close());
+ if (!out_stream->closed()) {
Review comment:
It is probably not necessary to test for `out_stream->closed()`, is it?
##########
File path: cpp/src/parquet/statistics.h
##########
@@ -282,8 +282,8 @@ class TypedStatistics : public Statistics {
/// \brief Batch statistics update with supplied validity bitmap
virtual void UpdateSpaced(const T* values, const uint8_t* valid_bits,
Review comment:
Can you add a description of the parameters to the docstring? It is not immediately obvious what `num_spaced_values` is...
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -4191,5 +4226,107 @@ TEST(TestArrowReadDeltaEncoding, DeltaBinaryPacked) {
}
#endif
+struct StatisticsTestParam {
+ std::shared_ptr<::arrow::Table> table;
+ int expected_null_count;
+ // This is the non-null count and not the num_values in the page headers.
+ int expected_value_count;
+ std::string expected_min;
+ std::string expected_max;
+};
+
+class ParameterizedStatisticsTest : public ::testing::TestWithParam<StatisticsTestParam> {
+};
+
+std::string GetManyEmptyLists() {
+ std::string many_empty_lists = "[";
+ for (int i = 0; i < 2000; ++i) {
+ many_empty_lists += "[],";
+ }
+ many_empty_lists += "[1,2,3,4,5,6,7,8,null]]";
+ return many_empty_lists;
+}
+
+TEST_P(ParameterizedStatisticsTest, NoNullCountWrittenForRepeatedFields) {
+ std::shared_ptr<::arrow::ResizableBuffer> serialized_data = AllocateBuffer();
+ auto out_stream = std::make_shared<::arrow::io::BufferOutputStream>(serialized_data);
+ std::unique_ptr<FileWriter> writer;
+ ASSERT_OK(FileWriter::Open(*GetParam().table->schema(), default_memory_pool(),
+ out_stream, default_writer_properties(),
+ default_arrow_writer_properties(), &writer));
+ ASSERT_OK(writer->WriteTable(*GetParam().table, std::numeric_limits<int64_t>::max()));
+ ASSERT_OK(writer->Close());
+ if (!out_stream->closed()) {
+ ASSERT_OK(out_stream->Close());
+ }
+
+ auto buffer_reader = std::make_shared<::arrow::io::BufferReader>(serialized_data);
+ auto parquet_reader = ParquetFileReader::Open(std::move(buffer_reader));
+ std::shared_ptr<FileMetaData> metadata = parquet_reader->metadata();
+ std::shared_ptr<Statistics> stats = metadata->RowGroup(0)->ColumnChunk(0)->statistics();
+ EXPECT_EQ(stats->null_count(), GetParam().expected_null_count);
+ EXPECT_EQ(stats->num_values(), GetParam().expected_value_count);
+ ASSERT_TRUE(stats->HasMinMax());
+ EXPECT_EQ(stats->EncodeMin(), GetParam().expected_min);
+ EXPECT_EQ(stats->EncodeMax(), GetParam().expected_max);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ StatsTests, ParameterizedStatisticsTest,
+ ::testing::Values(
+ StatisticsTestParam{/*table=*/Table::Make(
+ ::arrow::schema({::arrow::field("a", ::arrow::utf8())}),
+ {ArrayFromJSON(::arrow::utf8(), R"(["1", null, "3"])")}),
+ /*expected=*/1, /* empty list counts as null as well */
Review comment:
Should be `/*expected_null_count=*/`
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -3896,6 +3896,41 @@ INSTANTIATE_TEST_SUITE_P(WriteDictionary, TestArrowWriteDictionary,
::testing::Values(ParquetDataPageVersion::V1,
ParquetDataPageVersion::V2));
+TEST(TestArrowWriteNestedDictionary, TestNullCount) {
+ std::shared_ptr<::arrow::Array> dictionary = ArrayFromJSON(::arrow::utf8(), R"(["z"])");
+ std::shared_ptr<::arrow::Array> indices =
+ ArrayFromJSON(::arrow::int32(), R"([0, null, 0])");
+ std::shared_ptr<::arrow::Array> offsets =
+ ArrayFromJSON(::arrow::int32(), R"([null, 0, null, null, null, 3])");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<::arrow::Array> dict_encoded,
+ ::arrow::DictionaryArray::FromArrays(indices, dictionary));
+ // values looks like [null, ["z", null, "z"], null, null, null]
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<Array> values,
+ ::arrow::ListArray::FromArrays(*offsets, *dict_encoded, default_memory_pool()));
Review comment:
You can directly create of list-of-dictionary array using `ArrayFromJSON`. For example:
```c++
auto type = list(dictionary(int32(), utf8()));
auto values = ArrayFromJSON(type, R"([null, ["z", null, "z"], null, null, null])");
```
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -4191,5 +4226,107 @@ TEST(TestArrowReadDeltaEncoding, DeltaBinaryPacked) {
}
#endif
+struct StatisticsTestParam {
Review comment:
Can you perhaps create a separate `statistics_test.cc`? This test file is getting much too long already.
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -4191,5 +4226,107 @@ TEST(TestArrowReadDeltaEncoding, DeltaBinaryPacked) {
}
#endif
+struct StatisticsTestParam {
+ std::shared_ptr<::arrow::Table> table;
+ int expected_null_count;
+ // This is the non-null count and not the num_values in the page headers.
+ int expected_value_count;
+ std::string expected_min;
+ std::string expected_max;
+};
+
+class ParameterizedStatisticsTest : public ::testing::TestWithParam<StatisticsTestParam> {
+};
+
+std::string GetManyEmptyLists() {
+ std::string many_empty_lists = "[";
+ for (int i = 0; i < 2000; ++i) {
+ many_empty_lists += "[],";
+ }
+ many_empty_lists += "[1,2,3,4,5,6,7,8,null]]";
+ return many_empty_lists;
+}
+
+TEST_P(ParameterizedStatisticsTest, NoNullCountWrittenForRepeatedFields) {
+ std::shared_ptr<::arrow::ResizableBuffer> serialized_data = AllocateBuffer();
Review comment:
Would be nice to add a comment with a reference to the JIRA ticket.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tachyonwill commented on pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
tachyonwill commented on pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#issuecomment-936801035
> @tachyonwill Do you have an id on https://issues.apache.org/ so that this issue can be assigned to you?
willb_google
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tachyonwill commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
tachyonwill commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r721740296
##########
File path: cpp/src/parquet/statistics.h
##########
@@ -282,8 +282,8 @@ class TypedStatistics : public Statistics {
/// \brief Batch statistics update with supplied validity bitmap
virtual void UpdateSpaced(const T* values, const uint8_t* valid_bits,
Review comment:
Done.
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -3896,6 +3896,41 @@ INSTANTIATE_TEST_SUITE_P(WriteDictionary, TestArrowWriteDictionary,
::testing::Values(ParquetDataPageVersion::V1,
ParquetDataPageVersion::V2));
+TEST(TestArrowWriteNestedDictionary, TestNullCount) {
+ std::shared_ptr<::arrow::Array> dictionary = ArrayFromJSON(::arrow::utf8(), R"(["z"])");
+ std::shared_ptr<::arrow::Array> indices =
+ ArrayFromJSON(::arrow::int32(), R"([0, null, 0])");
+ std::shared_ptr<::arrow::Array> offsets =
+ ArrayFromJSON(::arrow::int32(), R"([null, 0, null, null, null, 3])");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<::arrow::Array> dict_encoded,
+ ::arrow::DictionaryArray::FromArrays(indices, dictionary));
+ // values looks like [null, ["z", null, "z"], null, null, null]
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<Array> values,
+ ::arrow::ListArray::FromArrays(*offsets, *dict_encoded, default_memory_pool()));
Review comment:
done.
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -4191,5 +4226,107 @@ TEST(TestArrowReadDeltaEncoding, DeltaBinaryPacked) {
}
#endif
+struct StatisticsTestParam {
+ std::shared_ptr<::arrow::Table> table;
+ int expected_null_count;
+ // This is the non-null count and not the num_values in the page headers.
+ int expected_value_count;
+ std::string expected_min;
+ std::string expected_max;
+};
+
+class ParameterizedStatisticsTest : public ::testing::TestWithParam<StatisticsTestParam> {
+};
+
+std::string GetManyEmptyLists() {
+ std::string many_empty_lists = "[";
+ for (int i = 0; i < 2000; ++i) {
+ many_empty_lists += "[],";
+ }
+ many_empty_lists += "[1,2,3,4,5,6,7,8,null]]";
+ return many_empty_lists;
+}
+
+TEST_P(ParameterizedStatisticsTest, NoNullCountWrittenForRepeatedFields) {
+ std::shared_ptr<::arrow::ResizableBuffer> serialized_data = AllocateBuffer();
+ auto out_stream = std::make_shared<::arrow::io::BufferOutputStream>(serialized_data);
+ std::unique_ptr<FileWriter> writer;
+ ASSERT_OK(FileWriter::Open(*GetParam().table->schema(), default_memory_pool(),
+ out_stream, default_writer_properties(),
+ default_arrow_writer_properties(), &writer));
+ ASSERT_OK(writer->WriteTable(*GetParam().table, std::numeric_limits<int64_t>::max()));
+ ASSERT_OK(writer->Close());
+ if (!out_stream->closed()) {
+ ASSERT_OK(out_stream->Close());
+ }
+
+ auto buffer_reader = std::make_shared<::arrow::io::BufferReader>(serialized_data);
+ auto parquet_reader = ParquetFileReader::Open(std::move(buffer_reader));
+ std::shared_ptr<FileMetaData> metadata = parquet_reader->metadata();
+ std::shared_ptr<Statistics> stats = metadata->RowGroup(0)->ColumnChunk(0)->statistics();
+ EXPECT_EQ(stats->null_count(), GetParam().expected_null_count);
+ EXPECT_EQ(stats->num_values(), GetParam().expected_value_count);
+ ASSERT_TRUE(stats->HasMinMax());
+ EXPECT_EQ(stats->EncodeMin(), GetParam().expected_min);
+ EXPECT_EQ(stats->EncodeMax(), GetParam().expected_max);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ StatsTests, ParameterizedStatisticsTest,
+ ::testing::Values(
+ StatisticsTestParam{/*table=*/Table::Make(
+ ::arrow::schema({::arrow::field("a", ::arrow::utf8())}),
+ {ArrayFromJSON(::arrow::utf8(), R"(["1", null, "3"])")}),
+ /*expected=*/1, /* empty list counts as null as well */
Review comment:
done.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] tachyonwill commented on a change in pull request #11281: PARQUET-2067: [C++][Parquet] Fix Parquet null count stats for enclosing null lists
Posted by GitBox <gi...@apache.org>.
tachyonwill commented on a change in pull request #11281:
URL: https://github.com/apache/arrow/pull/11281#discussion_r721742849
##########
File path: cpp/src/parquet/arrow/arrow_reader_writer_test.cc
##########
@@ -3896,6 +3896,41 @@ INSTANTIATE_TEST_SUITE_P(WriteDictionary, TestArrowWriteDictionary,
::testing::Values(ParquetDataPageVersion::V1,
ParquetDataPageVersion::V2));
+TEST(TestArrowWriteNestedDictionary, TestNullCount) {
+ std::shared_ptr<::arrow::Array> dictionary = ArrayFromJSON(::arrow::utf8(), R"(["z"])");
+ std::shared_ptr<::arrow::Array> indices =
+ ArrayFromJSON(::arrow::int32(), R"([0, null, 0])");
+ std::shared_ptr<::arrow::Array> offsets =
+ ArrayFromJSON(::arrow::int32(), R"([null, 0, null, null, null, 3])");
+ ASSERT_OK_AND_ASSIGN(std::shared_ptr<::arrow::Array> dict_encoded,
+ ::arrow::DictionaryArray::FromArrays(indices, dictionary));
+ // values looks like [null, ["z", null, "z"], null, null, null]
+ ASSERT_OK_AND_ASSIGN(
+ std::shared_ptr<Array> values,
+ ::arrow::ListArray::FromArrays(*offsets, *dict_encoded, default_memory_pool()));
+ std::shared_ptr<::arrow::Schema> schema =
+ ::arrow::schema({::arrow::field("values", values->type())});
+ std::shared_ptr<::arrow::Table> table = ::arrow::Table::Make(schema, {values});
+
+ std::shared_ptr<::arrow::ResizableBuffer> serialized_data = AllocateBuffer();
+ auto out_stream = std::make_shared<::arrow::io::BufferOutputStream>(serialized_data);
+ std::unique_ptr<FileWriter> writer;
+ ASSERT_OK(FileWriter::Open(*table->schema(), default_memory_pool(), out_stream,
+ default_writer_properties(),
+ default_arrow_writer_properties(), &writer));
+ ASSERT_OK(writer->WriteTable(*table, std::numeric_limits<int64_t>::max()));
+ ASSERT_OK(writer->Close());
+ if (!out_stream->closed()) {
Review comment:
made it unconditional.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org