Skip to content

Commit a9f4525

Browse files
author
xiao.dong
committed
remove useless unique_ptr
1 parent b80c0d2 commit a9f4525

4 files changed

Lines changed: 50 additions & 54 deletions

File tree

‎src/iceberg/manifest_reader.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ namespace iceberg {
3434
/// \brief Read manifest entries from a manifest file.
3535
class ICEBERG_EXPORT ManifestReader {
3636
public:
37-
virtual Result<std::vector<std::unique_ptr<ManifestEntry>>> Entries() const = 0;
37+
virtual Result<std::vector<ManifestEntry>> Entries() const = 0;
3838

3939
static Result<std::shared_ptr<ManifestReader>> MakeReader(
4040
const std::string& manifest_location, std::shared_ptr<FileIO> file_io,
@@ -44,7 +44,7 @@ class ICEBERG_EXPORT ManifestReader {
4444
/// \brief Read manifest files from a manifest list file.
4545
class ICEBERG_EXPORT ManifestListReader {
4646
public:
47-
virtual Result<std::vector<std::unique_ptr<ManifestFile>>> Files() const = 0;
47+
virtual Result<std::vector<ManifestFile>> Files() const = 0;
4848

4949
static Result<std::shared_ptr<ManifestListReader>> MakeReader(
5050
const std::string& manifest_list_location, std::shared_ptr<FileIO> file_io);

‎src/iceberg/manifest_reader_internal.cc‎

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ namespace iceberg {
3838
return InvalidArrowData("Nanoarrow error: {}", error.message); \
3939
}
4040

41-
Result<std::vector<std::unique_ptr<ManifestFile>>> ParseManifestListEntry(
42-
ArrowSchema* schema, ArrowArray* array_in, const Schema& iceberg_schema) {
41+
Result<std::vector<ManifestFile>> ParseManifestListEntry(ArrowSchema* schema,
42+
ArrowArray* array_in,
43+
const Schema& iceberg_schema) {
4344
if (schema->n_children != array_in->n_children) {
4445
return InvalidArgument("Columns size not match between schema:{} and array:{}",
4546
schema->n_children, array_in->n_children);
@@ -59,11 +60,8 @@ Result<std::vector<std::unique_ptr<ManifestFile>>> ParseManifestListEntry(
5960
status = ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error);
6061
ARROW_RETURN_IF_NOT_OK(status, error);
6162

62-
std::vector<std::unique_ptr<ManifestFile>> manifest_files;
63+
std::vector<ManifestFile> manifest_files;
6364
manifest_files.resize(array_in->length);
64-
for (auto& manifest_file : manifest_files) {
65-
manifest_file = std::make_unique<ManifestFile>();
66-
}
6765

6866
for (int64_t idx = 0; idx < array_in->n_children; idx++) {
6967
const auto& field = iceberg_schema.GetFieldByIndex(idx);
@@ -77,7 +75,7 @@ Result<std::vector<std::unique_ptr<ManifestFile>>> ParseManifestListEntry(
7775
for (size_t row_idx = 0; row_idx < view_of_column->length; row_idx++) { \
7876
if (!ArrowArrayViewIsNull(view_of_column, row_idx)) { \
7977
auto value = ArrowArrayViewGetIntUnsafe(view_of_column, row_idx); \
80-
manifest_files[row_idx]->field_name = static_cast<type>(value); \
78+
manifest_files[row_idx].field_name = static_cast<type>(value); \
8179
} \
8280
}
8381

@@ -86,7 +84,7 @@ Result<std::vector<std::unique_ptr<ManifestFile>>> ParseManifestListEntry(
8684
if (!ArrowArrayViewIsNull(view_of_column, row_idx)) {
8785
auto value = ArrowArrayViewGetStringUnsafe(view_of_column, row_idx);
8886
std::string path_str(value.data, value.size_bytes);
89-
manifest_files[row_idx]->manifest_path = path_str;
87+
manifest_files[row_idx].manifest_path = path_str;
9088
}
9189
}
9290
} else if (field_name == ManifestFile::kManifestLength.name()) {
@@ -97,7 +95,7 @@ Result<std::vector<std::unique_ptr<ManifestFile>>> ParseManifestListEntry(
9795
for (size_t row_idx = 0; row_idx < view_of_column->length; row_idx++) {
9896
if (!ArrowArrayViewIsNull(view_of_column, row_idx)) {
9997
auto value = ArrowArrayViewGetIntUnsafe(view_of_column, row_idx);
100-
manifest_files[row_idx]->content = static_cast<ManifestFile::Content>(value);
98+
manifest_files[row_idx].content = static_cast<ManifestFile::Content>(value);
10199
}
102100
}
103101
} else if (field_name == ManifestFile::kSequenceNumber.name()) {
@@ -180,14 +178,14 @@ Result<std::vector<std::unique_ptr<ManifestFile>>> ParseManifestListEntry(
180178
buffer.data.as_char, buffer.data.as_char + buffer.size_bytes);
181179
}
182180

183-
manifest_file->partitions.emplace_back(partition_field_summary);
181+
manifest_file.partitions.emplace_back(partition_field_summary);
184182
}
185183
}
186184
} else if (field_name == ManifestFile::kKeyMetadata.name()) {
187185
for (size_t row_idx = 0; row_idx < view_of_column->length; row_idx++) {
188186
if (!ArrowArrayViewIsNull(view_of_column, row_idx)) {
189187
auto buffer = ArrowArrayViewGetBytesUnsafe(view_of_column, row_idx);
190-
manifest_files[row_idx]->key_metadata = std::vector<uint8_t>(
188+
manifest_files[row_idx].key_metadata = std::vector<uint8_t>(
191189
buffer.data.as_char, buffer.data.as_char + buffer.size_bytes);
192190
}
193191
}
@@ -201,12 +199,10 @@ Result<std::vector<std::unique_ptr<ManifestFile>>> ParseManifestListEntry(
201199
return manifest_files;
202200
} // namespace iceberg
203201

204-
Result<std::vector<std::unique_ptr<ManifestEntry>>> ManifestReaderImpl::Entries() const {
205-
return {};
206-
}
202+
Result<std::vector<ManifestEntry>> ManifestReaderImpl::Entries() const { return {}; }
207203

208-
Result<std::vector<std::unique_ptr<ManifestFile>>> ManifestListReaderImpl::Files() const {
209-
std::vector<std::unique_ptr<ManifestFile>> manifest_files;
204+
Result<std::vector<ManifestFile>> ManifestListReaderImpl::Files() const {
205+
std::vector<ManifestFile> manifest_files;
210206
ICEBERG_ASSIGN_OR_RAISE(auto arrow_schema, reader_->Schema());
211207
internal::ArrowSchemaGuard schema_guard(&arrow_schema);
212208
while (true) {

‎src/iceberg/manifest_reader_internal.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class ManifestReaderImpl : public ManifestReader {
3333
std::shared_ptr<Schema> schema)
3434
: schema_(std::move(schema)), reader_(std::move(reader)) {}
3535

36-
Result<std::vector<std::unique_ptr<ManifestEntry>>> Entries() const override;
36+
Result<std::vector<ManifestEntry>> Entries() const override;
3737

3838
private:
3939
std::shared_ptr<Schema> schema_;
@@ -47,7 +47,7 @@ class ManifestListReaderImpl : public ManifestListReader {
4747
std::shared_ptr<Schema> schema)
4848
: schema_(std::move(schema)), reader_(std::move(reader)) {}
4949

50-
Result<std::vector<std::unique_ptr<ManifestFile>>> Files() const override;
50+
Result<std::vector<ManifestFile>> Files() const override;
5151

5252
private:
5353
std::shared_ptr<Schema> schema_;

‎test/manifest_list_reader_test.cc‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -57,53 +57,53 @@ TEST_F(ManifestListReaderTest, BasicTest) {
5757
ASSERT_EQ(read_result.value().size(), 4);
5858
std::string test_dir_prefix = "/tmp/db/db/iceberg_test/metadata/";
5959
for (const auto& file : read_result.value()) {
60-
auto manifest_path = file->manifest_path.substr(test_dir_prefix.size());
60+
auto manifest_path = file.manifest_path.substr(test_dir_prefix.size());
6161
if (manifest_path == "2bccd69e-d642-4816-bba0-261cd9bd0d93-m0.avro") {
62-
ASSERT_EQ(file->added_snapshot_id, 7412193043800610213);
63-
ASSERT_EQ(file->manifest_length, 7433);
64-
ASSERT_EQ(file->sequence_number, 4);
65-
ASSERT_EQ(file->min_sequence_number, 4);
66-
ASSERT_EQ(file->partitions.size(), 1);
67-
const auto& partition = file->partitions[0];
62+
ASSERT_EQ(file.added_snapshot_id, 7412193043800610213);
63+
ASSERT_EQ(file.manifest_length, 7433);
64+
ASSERT_EQ(file.sequence_number, 4);
65+
ASSERT_EQ(file.min_sequence_number, 4);
66+
ASSERT_EQ(file.partitions.size(), 1);
67+
const auto& partition = file.partitions[0];
6868
ASSERT_EQ(partition.contains_null, false);
6969
ASSERT_EQ(partition.contains_nan.value(), false);
7070
ASSERT_EQ(partition.lower_bound.value(),
7171
std::vector<uint8_t>({'x', ';', 0x07, 0x00}));
7272
ASSERT_EQ(partition.upper_bound.value(),
7373
std::vector<uint8_t>({'x', ';', 0x07, 0x00}));
7474
} else if (manifest_path == "9b6ffacd-ef10-4abf-a89c-01c733696796-m0.avro") {
75-
ASSERT_EQ(file->added_snapshot_id, 5485972788975780755);
76-
ASSERT_EQ(file->manifest_length, 7431);
77-
ASSERT_EQ(file->sequence_number, 3);
78-
ASSERT_EQ(file->min_sequence_number, 3);
79-
ASSERT_EQ(file->partitions.size(), 1);
80-
const auto& partition = file->partitions[0];
75+
ASSERT_EQ(file.added_snapshot_id, 5485972788975780755);
76+
ASSERT_EQ(file.manifest_length, 7431);
77+
ASSERT_EQ(file.sequence_number, 3);
78+
ASSERT_EQ(file.min_sequence_number, 3);
79+
ASSERT_EQ(file.partitions.size(), 1);
80+
const auto& partition = file.partitions[0];
8181
ASSERT_EQ(partition.contains_null, false);
8282
ASSERT_EQ(partition.contains_nan.value(), false);
8383
ASSERT_EQ(partition.lower_bound.value(),
8484
std::vector<uint8_t>({'(', 0x19, 0x07, 0x00}));
8585
ASSERT_EQ(partition.upper_bound.value(),
8686
std::vector<uint8_t>({'(', 0x19, 0x07, 0x00}));
8787
} else if (manifest_path == "2541e6b5-4923-4bd5-886d-72c6f7228400-m0.avro") {
88-
ASSERT_EQ(file->added_snapshot_id, 1679468743751242972);
89-
ASSERT_EQ(file->manifest_length, 7433);
90-
ASSERT_EQ(file->sequence_number, 2);
91-
ASSERT_EQ(file->min_sequence_number, 2);
92-
ASSERT_EQ(file->partitions.size(), 1);
93-
const auto& partition = file->partitions[0];
88+
ASSERT_EQ(file.added_snapshot_id, 1679468743751242972);
89+
ASSERT_EQ(file.manifest_length, 7433);
90+
ASSERT_EQ(file.sequence_number, 2);
91+
ASSERT_EQ(file.min_sequence_number, 2);
92+
ASSERT_EQ(file.partitions.size(), 1);
93+
const auto& partition = file.partitions[0];
9494
ASSERT_EQ(partition.contains_null, false);
9595
ASSERT_EQ(partition.contains_nan.value(), false);
9696
ASSERT_EQ(partition.lower_bound.value(),
9797
std::vector<uint8_t>({0xd0, 0xd4, 0x06, 0x00}));
9898
ASSERT_EQ(partition.upper_bound.value(),
9999
std::vector<uint8_t>({0xd0, 0xd4, 0x06, 0x00}));
100100
} else if (manifest_path == "3118c801-d2e0-4df6-8c7a-7d4eaade32f8-m0.avro") {
101-
ASSERT_EQ(file->added_snapshot_id, 1579605567338877265);
102-
ASSERT_EQ(file->manifest_length, 7431);
103-
ASSERT_EQ(file->sequence_number, 1);
104-
ASSERT_EQ(file->min_sequence_number, 1);
105-
ASSERT_EQ(file->partitions.size(), 1);
106-
const auto& partition = file->partitions[0];
101+
ASSERT_EQ(file.added_snapshot_id, 1579605567338877265);
102+
ASSERT_EQ(file.manifest_length, 7431);
103+
ASSERT_EQ(file.sequence_number, 1);
104+
ASSERT_EQ(file.min_sequence_number, 1);
105+
ASSERT_EQ(file.partitions.size(), 1);
106+
const auto& partition = file.partitions[0];
107107
ASSERT_EQ(partition.contains_null, false);
108108
ASSERT_EQ(partition.contains_nan.value(), false);
109109
ASSERT_EQ(partition.lower_bound.value(),
@@ -113,15 +113,15 @@ TEST_F(ManifestListReaderTest, BasicTest) {
113113
} else {
114114
ASSERT_TRUE(false) << "Unexpected manifest file: " << manifest_path;
115115
}
116-
ASSERT_EQ(file->partition_spec_id, 0);
117-
ASSERT_EQ(file->content, ManifestFile::Content::kData);
118-
ASSERT_EQ(file->added_files_count, 1);
119-
ASSERT_EQ(file->existing_files_count, 0);
120-
ASSERT_EQ(file->deleted_files_count, 0);
121-
ASSERT_EQ(file->added_rows_count, 1);
122-
ASSERT_EQ(file->existing_rows_count, 0);
123-
ASSERT_EQ(file->deleted_rows_count, 0);
124-
ASSERT_EQ(file->key_metadata.empty(), true);
116+
ASSERT_EQ(file.partition_spec_id, 0);
117+
ASSERT_EQ(file.content, ManifestFile::Content::kData);
118+
ASSERT_EQ(file.added_files_count, 1);
119+
ASSERT_EQ(file.existing_files_count, 0);
120+
ASSERT_EQ(file.deleted_files_count, 0);
121+
ASSERT_EQ(file.added_rows_count, 1);
122+
ASSERT_EQ(file.existing_rows_count, 0);
123+
ASSERT_EQ(file.deleted_rows_count, 0);
124+
ASSERT_EQ(file.key_metadata.empty(), true);
125125
}
126126
}
127127

0 commit comments

Comments
 (0)