wgtmac commented on code in PR #235:
URL: https://github.com/apache/iceberg-cpp/pull/235#discussion_r2361833386
##########
src/iceberg/avro/avro_reader.cc:
##########
@@ -173,6 +173,24 @@ class AvroReader::Impl {
return arrow_schema;
}
+ Result<std::unordered_map<std::string, std::string>> Metadata() {
+ if (reader_ == nullptr) {
+ return InvalidArgument("Reader is not opened");
Review Comment:
Usually `InvalidArgument` means an invalid function parameter. Perhaps just
`Invalid`?
##########
src/iceberg/avro/avro_reader.cc:
##########
@@ -173,6 +173,24 @@ class AvroReader::Impl {
return arrow_schema;
}
+ Result<std::unordered_map<std::string, std::string>> Metadata() {
+ if (reader_ == nullptr) {
+ return InvalidArgument("Reader is not opened");
+ }
+
+ auto metadata = reader_->metadata();
Review Comment:
```suggestion
const auto& metadata = reader_->metadata();
```
We don't want to copy the whole map here
##########
src/iceberg/parquet/parquet_reader.cc:
##########
@@ -185,6 +186,31 @@ class ParquetReader::Impl {
return arrow_schema;
}
+ Result<std::unordered_map<std::string, std::string>> Metadata() {
+ if (reader_ == nullptr) {
+ return InvalidArgument("Reader is not opened");
+ }
+
+ auto metadata = reader_->parquet_reader()->metadata();
+ if (!metadata) {
+ return InvalidArgument("Failed to get Parquet file metadata");
+ }
+
+ auto kv_metadata = metadata->key_value_metadata();
+ if (!kv_metadata) {
+ return std::unordered_map<std::string, std::string>{};
+ }
+
+ std::unordered_map<std::string, std::string> metadata_map;
+ metadata_map.reserve(kv_metadata->size());
+
+ for (int i = 0; i < kv_metadata->size(); ++i) {
+ metadata_map.try_emplace(kv_metadata->key(i), kv_metadata->value(i));
Review Comment:
Do we need to check the return value?
##########
src/iceberg/parquet/parquet_reader.cc:
##########
@@ -185,6 +186,31 @@ class ParquetReader::Impl {
return arrow_schema;
}
+ Result<std::unordered_map<std::string, std::string>> Metadata() {
+ if (reader_ == nullptr) {
+ return InvalidArgument("Reader is not opened");
Review Comment:
ditto
##########
src/iceberg/avro/avro_reader.cc:
##########
@@ -173,6 +173,24 @@ class AvroReader::Impl {
return arrow_schema;
}
+ Result<std::unordered_map<std::string, std::string>> Metadata() {
+ if (reader_ == nullptr) {
+ return InvalidArgument("Reader is not opened");
+ }
+
+ auto metadata = reader_->metadata();
+
+ std::unordered_map<std::string, std::string> metadata_map;
+ metadata_map.reserve(metadata.size());
+
+ for (const auto& pair : metadata) {
+ metadata_map.try_emplace(pair.first,
Review Comment:
nit: can we use ranges with transform?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
