Skip to content

Commit dcd6ee9

Browse files
cbb330cbb330
committed
[C++][ORC] Align metadata/stat signatures to pointer-style parity
Switch ORC metadata traversal APIs from Result-wrapped values to pointer-style returns, make Statistics::null_count() non-optional with HasNullCount precondition semantics, and update adapter tests to match the new contracts. Made-with: Cursor
1 parent 003fe45 commit dcd6ee9

5 files changed

Lines changed: 127 additions & 67 deletions

File tree

‎cpp/src/arrow/adapters/orc/adapter.cc‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -587,12 +587,14 @@ class ORCFileReader::Impl {
587587
return NextStripeReader(batch_size, empty_vec);
588588
}
589589

590-
Result<FileMetaData> GetFileMetaData() {
591-
ORC_BEGIN_CATCH_NOT_OK
592-
auto file_stats =
593-
std::shared_ptr<const liborc::Statistics>(reader_->getStatistics().release());
594-
return FileMetaData(reader_, std::move(file_stats));
595-
ORC_END_CATCH_NOT_OK
590+
std::shared_ptr<FileMetaData> GetFileMetaData() {
591+
try {
592+
auto file_stats =
593+
std::shared_ptr<const liborc::Statistics>(reader_->getStatistics().release());
594+
return std::make_shared<FileMetaData>(reader_, std::move(file_stats));
595+
} catch (...) {
596+
return nullptr;
597+
}
596598
}
597599

598600
const liborc::Type& GetORCType() { return reader_->getType(); }
@@ -738,7 +740,9 @@ std::string ORCFileReader::GetSerializedFileTail() {
738740
return impl_->GetSerializedFileTail();
739741
}
740742

741-
Result<FileMetaData> ORCFileReader::GetFileMetaData() { return impl_->GetFileMetaData(); }
743+
std::shared_ptr<FileMetaData> ORCFileReader::GetFileMetaData() {
744+
return impl_->GetFileMetaData();
745+
}
742746

743747
const ::orc::Type& ORCFileReader::GetORCType() { return impl_->GetORCType(); }
744748

‎cpp/src/arrow/adapters/orc/adapter.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ class ARROW_EXPORT ORCFileReader {
295295
Result<std::shared_ptr<const KeyValueMetadata>> ReadMetadata();
296296

297297
/// \brief Get file-level metadata view.
298-
Result<FileMetaData> GetFileMetaData();
298+
std::shared_ptr<FileMetaData> GetFileMetaData();
299299

300300
/// \brief Get the ORC type tree for column ID mapping.
301301
///

‎cpp/src/arrow/adapters/orc/adapter_test.cc‎

Lines changed: 82 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -340,30 +340,80 @@ std::unique_ptr<liborc::Writer> CreateWriter(uint64_t stripe_size,
340340

341341
Result<adapters::orc::Statistics> GetFileColumnStatistics(
342342
adapters::orc::ORCFileReader* reader, int column_index) {
343-
ARROW_ASSIGN_OR_RAISE(auto file_meta, reader->GetFileMetaData());
344-
ARROW_ASSIGN_OR_RAISE(auto column_meta, file_meta.Column(column_index));
345-
return column_meta.statistics();
343+
auto file_meta = reader->GetFileMetaData();
344+
if (file_meta == nullptr) {
345+
return Status::Invalid("Failed to read ORC file metadata");
346+
}
347+
if (column_index < 0 || column_index >= file_meta->num_columns()) {
348+
return Status::Invalid("Column index ", column_index, " out of range");
349+
}
350+
auto column_meta = file_meta->Column(column_index);
351+
if (column_meta == nullptr) {
352+
return Status::Invalid("Failed to read ORC file column metadata");
353+
}
354+
auto column_stats = column_meta->statistics();
355+
if (column_stats == nullptr) {
356+
return Status::Invalid("Failed to read ORC file column statistics");
357+
}
358+
return *column_stats;
346359
}
347360

348361
Result<adapters::orc::Statistics> GetStripeColumnStatistics(
349362
adapters::orc::ORCFileReader* reader, int64_t stripe_index, int column_index) {
350-
ARROW_ASSIGN_OR_RAISE(auto file_meta, reader->GetFileMetaData());
351-
ARROW_ASSIGN_OR_RAISE(auto stripe_meta, file_meta.Stripe(static_cast<int>(stripe_index)));
352-
ARROW_ASSIGN_OR_RAISE(auto column_meta, stripe_meta.Column(column_index));
353-
return column_meta.statistics();
363+
auto file_meta = reader->GetFileMetaData();
364+
if (file_meta == nullptr) {
365+
return Status::Invalid("Failed to read ORC file metadata");
366+
}
367+
if (stripe_index < 0 || stripe_index >= file_meta->num_stripes()) {
368+
return Status::Invalid("Stripe index ", stripe_index, " out of range");
369+
}
370+
auto stripe_meta = file_meta->Stripe(static_cast<int>(stripe_index));
371+
if (stripe_meta == nullptr) {
372+
return Status::Invalid("Failed to read ORC stripe metadata");
373+
}
374+
if (column_index < 0 || column_index >= stripe_meta->num_columns()) {
375+
return Status::Invalid("Column index ", column_index, " out of range");
376+
}
377+
auto column_meta = stripe_meta->Column(column_index);
378+
if (column_meta == nullptr) {
379+
return Status::Invalid("Failed to read ORC stripe column metadata");
380+
}
381+
auto column_stats = column_meta->statistics();
382+
if (column_stats == nullptr) {
383+
return Status::Invalid("Failed to read ORC stripe column statistics");
384+
}
385+
return *column_stats;
354386
}
355387

356388
Result<std::vector<adapters::orc::Statistics>> GetStripeColumnStatisticsBulk(
357389
adapters::orc::ORCFileReader* reader, int64_t stripe_index,
358390
const std::vector<int>& column_indices) {
359-
ARROW_ASSIGN_OR_RAISE(auto file_meta, reader->GetFileMetaData());
360-
ARROW_ASSIGN_OR_RAISE(auto stripe_meta, file_meta.Stripe(static_cast<int>(stripe_index)));
391+
auto file_meta = reader->GetFileMetaData();
392+
if (file_meta == nullptr) {
393+
return Status::Invalid("Failed to read ORC file metadata");
394+
}
395+
if (stripe_index < 0 || stripe_index >= file_meta->num_stripes()) {
396+
return Status::Invalid("Stripe index ", stripe_index, " out of range");
397+
}
398+
auto stripe_meta = file_meta->Stripe(static_cast<int>(stripe_index));
399+
if (stripe_meta == nullptr) {
400+
return Status::Invalid("Failed to read ORC stripe metadata");
401+
}
361402
std::vector<adapters::orc::Statistics> stats;
362403
stats.reserve(column_indices.size());
363404
for (int column_index : column_indices) {
364-
ARROW_ASSIGN_OR_RAISE(auto column_meta, stripe_meta.Column(column_index));
365-
ARROW_ASSIGN_OR_RAISE(auto column_stats, column_meta.statistics());
366-
stats.push_back(std::move(column_stats));
405+
if (column_index < 0 || column_index >= stripe_meta->num_columns()) {
406+
return Status::Invalid("Column index ", column_index, " out of range");
407+
}
408+
auto column_meta = stripe_meta->Column(column_index);
409+
if (column_meta == nullptr) {
410+
return Status::Invalid("Failed to read ORC stripe column metadata");
411+
}
412+
auto column_stats = column_meta->statistics();
413+
if (column_stats == nullptr) {
414+
return Status::Invalid("Failed to read ORC stripe column statistics");
415+
}
416+
stats.push_back(std::move(*column_stats));
367417
}
368418
return stats;
369419
}
@@ -422,23 +472,27 @@ TEST(TestAdapterRead, FileMetaDataViewAccessors) {
422472
ASSERT_OK_AND_ASSIGN(auto reader,
423473
adapters::orc::ORCFileReader::Open(in_stream, default_memory_pool()));
424474

425-
ASSERT_OK_AND_ASSIGN(auto file_meta, reader->GetFileMetaData());
426-
EXPECT_TRUE(file_meta.valid());
427-
EXPECT_EQ(file_meta.num_stripes(), reader->NumberOfStripes());
428-
EXPECT_EQ(file_meta.num_rows(), reader->NumberOfRows());
429-
ASSERT_OK_AND_ASSIGN(auto metadata, file_meta.key_value_metadata());
475+
auto file_meta = reader->GetFileMetaData();
476+
ASSERT_NE(file_meta, nullptr);
477+
EXPECT_TRUE(file_meta->valid());
478+
EXPECT_EQ(file_meta->num_stripes(), reader->NumberOfStripes());
479+
EXPECT_EQ(file_meta->num_rows(), reader->NumberOfRows());
480+
auto metadata = file_meta->key_value_metadata();
430481
ASSERT_NE(metadata, nullptr);
431-
EXPECT_EQ(file_meta.schema_root().getKind(), liborc::STRUCT);
432-
433-
ASSERT_OK_AND_ASSIGN(auto stripe_meta, file_meta.Stripe(0));
434-
EXPECT_TRUE(stripe_meta.valid());
435-
EXPECT_EQ(stripe_meta.stripe_index(), 0);
436-
EXPECT_GT(stripe_meta.num_rows(), 0);
437-
ASSERT_OK_AND_ASSIGN(auto column_meta, stripe_meta.Column(1));
438-
EXPECT_TRUE(column_meta.valid());
439-
EXPECT_EQ(column_meta.column_index(), 1);
440-
ASSERT_OK_AND_ASSIGN(auto column_stats, column_meta.statistics());
441-
EXPECT_TRUE(column_stats.valid());
482+
EXPECT_EQ(file_meta->schema_root().getKind(), liborc::STRUCT);
483+
484+
auto stripe_meta = file_meta->Stripe(0);
485+
ASSERT_NE(stripe_meta, nullptr);
486+
EXPECT_TRUE(stripe_meta->valid());
487+
EXPECT_EQ(stripe_meta->stripe_index(), 0);
488+
EXPECT_GT(stripe_meta->num_rows(), 0);
489+
auto column_meta = stripe_meta->Column(1);
490+
ASSERT_NE(column_meta, nullptr);
491+
EXPECT_TRUE(column_meta->valid());
492+
EXPECT_EQ(column_meta->column_index(), 1);
493+
auto column_stats = column_meta->statistics();
494+
ASSERT_NE(column_stats, nullptr);
495+
EXPECT_TRUE(column_stats->valid());
442496
}
443497

444498
TEST(TestAdapterRead, ReadIntAndStringFileMultipleStripes) {

‎cpp/src/arrow/adapters/orc/statistics.cc‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "arrow/util/checked_cast.h"
2626
#include "arrow/util/decimal.h"
2727
#include "arrow/util/key_value_metadata.h"
28+
#include "arrow/util/logging.h"
2829
#include "orc/Reader.hh"
2930
#include "orc/Statistics.hh"
3031

@@ -58,40 +59,40 @@ int64_t FileMetaData::num_rows() const {
5859
return static_cast<int64_t>(reader_->getNumberOfRows());
5960
}
6061

61-
Result<StripeMetaData> FileMetaData::Stripe(int stripe_index) const {
62+
std::unique_ptr<StripeMetaData> FileMetaData::Stripe(int stripe_index) const {
6263
if (!valid()) {
63-
return Status::Invalid("ORC file metadata is not initialized");
64+
return nullptr;
6465
}
6566
if (stripe_index < 0 || stripe_index >= num_stripes()) {
66-
return Status::Invalid("Stripe index ", stripe_index, " out of range [0, ",
67-
num_stripes(), ")");
67+
return nullptr;
6868
}
6969

7070
auto stripe_stats = std::shared_ptr<const liborc::Statistics>(
7171
reader_->getStripeStatistics(static_cast<uint64_t>(stripe_index)).release());
7272
auto stripe_info = reader_->getStripe(static_cast<uint64_t>(stripe_index));
73-
return StripeMetaData(stripe_index, static_cast<int64_t>(stripe_info->getNumberOfRows()),
74-
std::move(stripe_stats));
73+
return std::make_unique<StripeMetaData>(
74+
stripe_index, static_cast<int64_t>(stripe_info->getNumberOfRows()),
75+
std::move(stripe_stats));
7576
}
7677

77-
Result<ColumnMetaData> FileMetaData::Column(int column_index) const {
78+
std::unique_ptr<ColumnMetaData> FileMetaData::Column(int column_index) const {
7879
if (!valid()) {
79-
return Status::Invalid("ORC file metadata is not initialized");
80+
return nullptr;
8081
}
8182
if (column_index < 0 || static_cast<uint32_t>(column_index) >=
8283
file_statistics_->getNumberOfColumns()) {
83-
return Status::Invalid("Column index ", column_index, " out of range [0, ",
84-
file_statistics_->getNumberOfColumns(), ")");
84+
return nullptr;
8585
}
8686

8787
const liborc::ColumnStatistics* col_stats =
8888
file_statistics_->getColumnStatistics(static_cast<uint32_t>(column_index));
89-
return ColumnMetaData(column_index, Statistics(file_statistics_, col_stats));
89+
return std::make_unique<ColumnMetaData>(
90+
column_index, Statistics(file_statistics_, col_stats));
9091
}
9192

92-
Result<std::shared_ptr<const KeyValueMetadata>> FileMetaData::key_value_metadata() const {
93+
std::shared_ptr<const KeyValueMetadata> FileMetaData::key_value_metadata() const {
9394
if (!valid()) {
94-
return Status::Invalid("ORC file metadata is not initialized");
95+
return nullptr;
9596
}
9697

9798
auto metadata = std::make_shared<KeyValueMetadata>();
@@ -104,35 +105,37 @@ Result<std::shared_ptr<const KeyValueMetadata>> FileMetaData::key_value_metadata
104105

105106
const ::orc::Type& FileMetaData::schema_root() const { return reader_->getType(); }
106107

107-
Result<Statistics> ColumnMetaData::statistics() const {
108+
std::shared_ptr<Statistics> ColumnMetaData::statistics() const {
108109
if (!valid()) {
109-
return Status::Invalid("ORC column metadata is not initialized");
110+
return nullptr;
110111
}
111-
return statistics_;
112+
return std::make_shared<Statistics>(statistics_);
112113
}
113114

114115
int StripeMetaData::num_columns() const {
115116
return static_cast<int>(stripe_statistics_->getNumberOfColumns());
116117
}
117118

118-
Result<ColumnMetaData> StripeMetaData::Column(int column_index) const {
119+
std::unique_ptr<ColumnMetaData> StripeMetaData::Column(int column_index) const {
119120
if (!valid()) {
120-
return Status::Invalid("ORC stripe metadata is not initialized");
121+
return nullptr;
121122
}
122123
if (column_index < 0 || static_cast<uint32_t>(column_index) >=
123124
stripe_statistics_->getNumberOfColumns()) {
124-
return Status::Invalid("Column index ", column_index, " out of range [0, ",
125-
stripe_statistics_->getNumberOfColumns(), ")");
125+
return nullptr;
126126
}
127127

128128
const liborc::ColumnStatistics* col_stats =
129129
stripe_statistics_->getColumnStatistics(static_cast<uint32_t>(column_index));
130-
return ColumnMetaData(column_index, Statistics(stripe_statistics_, col_stats));
130+
return std::make_unique<ColumnMetaData>(
131+
column_index, Statistics(stripe_statistics_, col_stats));
131132
}
132133

133-
std::optional<int64_t> Statistics::null_count() const {
134-
// liborc doesn't expose null_count on ColumnStatistics.
135-
return std::nullopt;
134+
int64_t Statistics::null_count() const {
135+
ARROW_DCHECK(HasNullCount());
136+
// liborc doesn't expose null_count on ColumnStatistics; callers must treat
137+
// this as a preconditioned API guarded by HasNullCount().
138+
return 0;
136139
}
137140

138141
int64_t Statistics::num_values() const {

‎cpp/src/arrow/adapters/orc/statistics.h‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919

2020
#include <cstdint>
2121
#include <memory>
22-
#include <optional>
2322

2423
#include "arrow/result.h"
2524
#include "arrow/scalar.h"
@@ -62,9 +61,9 @@ class ARROW_EXPORT FileMetaData {
6261
int num_columns() const;
6362
int num_stripes() const;
6463
int64_t num_rows() const;
65-
Result<StripeMetaData> Stripe(int stripe_index) const;
66-
Result<ColumnMetaData> Column(int column_index) const;
67-
Result<std::shared_ptr<const KeyValueMetadata>> key_value_metadata() const;
64+
std::unique_ptr<StripeMetaData> Stripe(int stripe_index) const;
65+
std::unique_ptr<ColumnMetaData> Column(int column_index) const;
66+
std::shared_ptr<const KeyValueMetadata> key_value_metadata() const;
6867
const ::orc::Type& schema_root() const;
6968

7069
private:
@@ -87,7 +86,7 @@ class ARROW_EXPORT StripeMetaData {
8786
int64_t stripe_index() const { return stripe_index_; }
8887
int64_t num_rows() const { return num_rows_; }
8988
int num_columns() const;
90-
Result<ColumnMetaData> Column(int column_index) const;
89+
std::unique_ptr<ColumnMetaData> Column(int column_index) const;
9190

9291
private:
9392
int64_t stripe_index_ = -1;
@@ -110,7 +109,7 @@ class ARROW_EXPORT Statistics {
110109

111110
bool valid() const { return column_statistics_ != nullptr; }
112111
bool HasNullCount() const;
113-
std::optional<int64_t> null_count() const;
112+
int64_t null_count() const;
114113
int64_t num_values() const;
115114
bool HasMinMax() const;
116115

@@ -136,7 +135,7 @@ class ARROW_EXPORT ColumnMetaData {
136135

137136
bool valid() const { return statistics_.valid(); }
138137
int column_index() const { return column_index_; }
139-
Result<Statistics> statistics() const;
138+
std::shared_ptr<Statistics> statistics() const;
140139

141140
private:
142141
int column_index_ = -1;

0 commit comments

Comments
 (0)