You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "wgtmac (via GitHub)" <gi...@apache.org> on 2025/08/04 15:35:29 UTC
[PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
wgtmac opened a new pull request, #159:
URL: https://github.com/apache/iceberg-cpp/pull/159
(no comment)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2253468625
##########
test/parquet_schema_test.cc:
##########
@@ -17,50 +17,494 @@
* under the License.
*/
+#include <arrow/type.h>
#include <gtest/gtest.h>
+#include <parquet/arrow/reader.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
-#include <parquet/types.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/schema.h"
+#include "matchers.h"
namespace iceberg::parquet {
namespace {
-::parquet::schema::NodePtr MakeInt32Node(const std::string& name, int field_id = -1) {
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+::parquet::schema::NodePtr MakeInt32Node(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::INT32, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeInt64Node(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::INT64, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeStringNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::String(), ::parquet::Type::BYTE_ARRAY,
+ /*primitive_length=*/-1, field_id);
+}
+
+::parquet::schema::NodePtr MakeDoubleNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::DOUBLE, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeFloatNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
return ::parquet::schema::PrimitiveNode::Make(
- name, ::parquet::Repetition::REQUIRED, ::parquet::LogicalType::None(),
- ::parquet::Type::INT32, /*primitive_length=*/-1, field_id);
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::FLOAT, /*primitive_length=*/-1,
+ field_id);
}
::parquet::schema::NodePtr MakeGroupNode(const std::string& name,
const ::parquet::schema::NodeVector& fields,
- int field_id = -1) {
- return ::parquet::schema::GroupNode::Make(name, ::parquet::Repetition::REQUIRED, fields,
- /*logical_type=*/nullptr, field_id);
+ int field_id = -1, bool optional = true) {
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ fields, /*logical_type=*/nullptr, field_id);
}
+::parquet::schema::NodePtr MakeListNode(const std::string& name,
+ const ::parquet::schema::NodePtr& element_node,
+ int field_id = -1, bool optional = true) {
+ auto list_group = ::parquet::schema::GroupNode::Make(
+ "element", ::parquet::Repetition::REPEATED, {element_node});
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ {list_group}, ::parquet::LogicalType::List(), field_id);
+}
+
+::parquet::schema::NodePtr MakeMapNode(const std::string& name,
+ const ::parquet::schema::NodePtr& key_node,
+ const ::parquet::schema::NodePtr& value_node,
+ int field_id = -1, bool optional = true) {
+ auto key_value_group = ::parquet::schema::GroupNode::Make(
+ "key_value", ::parquet::Repetition::REPEATED, {key_node, value_node});
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ {key_value_group}, ::parquet::LogicalType::Map(), field_id);
+}
+
+// Helper to create SchemaManifest from Parquet schema
+::parquet::arrow::SchemaManifest MakeSchemaManifest(
+ const ::parquet::schema::NodePtr& parquet_schema) {
+ auto parquet_schema_descriptor = std::make_shared<::parquet::SchemaDescriptor>();
+ parquet_schema_descriptor->Init(parquet_schema);
+
+ ::parquet::arrow::SchemaManifest manifest;
+ auto status = ::parquet::arrow::SchemaManifest::Make(
+ parquet_schema_descriptor.get(), /*key_value_metadata=*/nullptr,
+ ::parquet::default_arrow_reader_properties(), &manifest);
+ if (!status.ok()) {
+ throw std::runtime_error("Failed to create SchemaManifest: " + status.ToString());
+ }
+ return manifest;
+}
+
+#define ASSERT_PROJECTED_FIELD(field_projection, index) \
+ ASSERT_EQ(field_projection.kind, FieldProjection::Kind::kProjected); \
+ ASSERT_EQ(std::get<1>(field_projection.from), index);
+
+#define ASSERT_PROJECTED_NULL_FIELD(field_projection) \
+ ASSERT_EQ(field_projection.kind, FieldProjection::Kind::kNull);
+
} // namespace
TEST(HasFieldIds, PrimitiveNode) {
EXPECT_FALSE(HasFieldIds(MakeInt32Node("test_field")));
EXPECT_TRUE(HasFieldIds(MakeInt32Node("test_field", /*field_id=*/1)));
}
-TEST(HasFieldIds, GroupNode) {
- auto group_node_without_field_id =
- MakeGroupNode("test_group", {MakeInt32Node("c1"), MakeInt32Node("c2")});
- EXPECT_FALSE(HasFieldIds(group_node_without_field_id));
+// TEST(HasFieldIds, GroupNode) {
Review Comment:
Weird, I'm not sure what's going on here...NodePtr is a shared_ptr and this seems to work elsewhere...
https://github.com/llvm/llvm-project/issues/122405 also seems to be related - maybe this check is a bit flaky. We can uncomment and use NOLINT on the specific line?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2262956228
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,393 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
Review Comment:
Do we also accept the other types?
<img width="709" height="135" alt="image" src="https://github.com/user-attachments/assets/70c6ec9e-aa98-4343-8456-35fee0da99f1" />
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2254657827
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
Review Comment:
We can leave a little todo(v3) or something 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2267811396
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
Review Comment:
I forget offhand if this will return a `DictionaryType` for dictionary encoded columns, if so then you need to check for the `DictionaryType` and then switch on the `ValueType` of it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2263051973
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,393 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
Review Comment:
This is still an open PR: https://github.com/apache/arrow/pull/45351
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2267812174
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ const auto& time_type =
+ internal::checked_cast<const ::arrow::Time64Type&>(*arrow_type);
+ if (time_type.unit() == ::arrow::TimeUnit::MICRO) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
Review Comment:
What about LargeList and ListView?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2254084748
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
+ return {};
+ }
+ break;
+ case TypeId::kMap:
+ if (arrow_type->id() == ::arrow::Type::MAP) {
+ return {};
+ }
+ break;
+ default:
+ break;
+ }
+
+ return InvalidSchema("Cannot read Iceberg type: {} from Parquet type: {}",
+ expected_type, arrow_type->ToString());
+}
+
+// Forward declaration
+Result<FieldProjection> ProjectNested(
+ const Type& expected_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields);
+
+Result<FieldProjection> ProjectStruct(
+ const StructType& struct_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ struct FieldContext {
+ size_t local_index;
+ const ::parquet::arrow::SchemaField& parquet_field;
+ };
+ std::unordered_map<int32_t, FieldContext> field_context_map;
+ field_context_map.reserve(parquet_fields.size());
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ const ::parquet::arrow::SchemaField& parquet_field = parquet_fields[i];
+ auto field_id = GetFieldId(parquet_field);
+ if (!field_id) {
+ continue;
+ }
+ if (const auto [iter, inserted] = field_context_map.emplace(
+ std::piecewise_construct, std::forward_as_tuple(field_id.value()),
+ std::forward_as_tuple(i, parquet_field));
+ !inserted) [[unlikely]] {
+ return InvalidSchema("Duplicate field id found in Parquet schema: {}",
Review Comment:
```suggestion
return InvalidSchema("Duplicate field id '{}' found in Parquet schema",
```
And add schema as string?
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
+ return {};
+ }
+ break;
+ case TypeId::kMap:
+ if (arrow_type->id() == ::arrow::Type::MAP) {
+ return {};
+ }
+ break;
+ default:
+ break;
+ }
+
+ return InvalidSchema("Cannot read Iceberg type: {} from Parquet type: {}",
+ expected_type, arrow_type->ToString());
+}
+
+// Forward declaration
+Result<FieldProjection> ProjectNested(
+ const Type& expected_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields);
+
+Result<FieldProjection> ProjectStruct(
+ const StructType& struct_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ struct FieldContext {
+ size_t local_index;
+ const ::parquet::arrow::SchemaField& parquet_field;
+ };
+ std::unordered_map<int32_t, FieldContext> field_context_map;
+ field_context_map.reserve(parquet_fields.size());
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ const ::parquet::arrow::SchemaField& parquet_field = parquet_fields[i];
+ auto field_id = GetFieldId(parquet_field);
+ if (!field_id) {
+ continue;
+ }
+ if (const auto [iter, inserted] = field_context_map.emplace(
+ std::piecewise_construct, std::forward_as_tuple(field_id.value()),
+ std::forward_as_tuple(i, parquet_field));
+ !inserted) [[unlikely]] {
+ return InvalidSchema("Duplicate field id found in Parquet schema: {}",
+ field_id.value());
+ }
+ }
+
+ FieldProjection result;
+ result.children.reserve(struct_type.fields().size());
+
+ for (const auto& expected_field : struct_type.fields()) {
+ int32_t field_id = expected_field.field_id();
+ FieldProjection child_projection;
+
+ if (auto iter = field_context_map.find(field_id); iter != field_context_map.cend()) {
+ const auto& parquet_field = iter->second.parquet_field;
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*expected_field.type(), parquet_field));
+ if (expected_field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(child_projection, ProjectNested(*expected_field.type(),
+ parquet_field.children));
+ } else {
+ child_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(parquet_field.column_index);
+ }
+ child_projection.from = iter->second.local_index;
+ child_projection.kind = FieldProjection::Kind::kProjected;
+ } else if (MetadataColumns::IsMetadataColumn(field_id)) {
+ child_projection.kind = FieldProjection::Kind::kMetadata;
+ } else if (expected_field.optional()) {
+ child_projection.kind = FieldProjection::Kind::kNull;
+ } else {
+ return InvalidSchema("Missing required field with id: {}", field_id);
+ }
+
+ result.children.emplace_back(std::move(child_projection));
+ }
+
+ PruneFieldProjection(result);
+ return result;
+}
+
+Result<FieldProjection> ProjectList(
+ const ListType& list_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ if (parquet_fields.size() != 1) {
+ return InvalidSchema("List type must have exactly one field, got {}",
+ parquet_fields.size());
+ }
+
+ const auto& parquet_field = parquet_fields[0];
+ auto element_field_id = GetFieldId(parquet_field);
+ if (!element_field_id) {
+ return InvalidSchema("List element field missing field id");
+ }
+
+ const auto& expected_element_field = list_type.fields().back();
Review Comment:
why this variable is named as "expected_element_field" rather than "element_field"?
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
+ return {};
+ }
+ break;
+ case TypeId::kMap:
+ if (arrow_type->id() == ::arrow::Type::MAP) {
+ return {};
+ }
+ break;
+ default:
+ break;
+ }
+
+ return InvalidSchema("Cannot read Iceberg type: {} from Parquet type: {}",
+ expected_type, arrow_type->ToString());
+}
+
+// Forward declaration
+Result<FieldProjection> ProjectNested(
+ const Type& expected_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields);
+
+Result<FieldProjection> ProjectStruct(
+ const StructType& struct_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ struct FieldContext {
+ size_t local_index;
+ const ::parquet::arrow::SchemaField& parquet_field;
+ };
+ std::unordered_map<int32_t, FieldContext> field_context_map;
+ field_context_map.reserve(parquet_fields.size());
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ const ::parquet::arrow::SchemaField& parquet_field = parquet_fields[i];
+ auto field_id = GetFieldId(parquet_field);
+ if (!field_id) {
+ continue;
+ }
+ if (const auto [iter, inserted] = field_context_map.emplace(
+ std::piecewise_construct, std::forward_as_tuple(field_id.value()),
+ std::forward_as_tuple(i, parquet_field));
+ !inserted) [[unlikely]] {
+ return InvalidSchema("Duplicate field id found in Parquet schema: {}",
+ field_id.value());
+ }
+ }
+
+ FieldProjection result;
+ result.children.reserve(struct_type.fields().size());
+
+ for (const auto& expected_field : struct_type.fields()) {
+ int32_t field_id = expected_field.field_id();
+ FieldProjection child_projection;
+
+ if (auto iter = field_context_map.find(field_id); iter != field_context_map.cend()) {
+ const auto& parquet_field = iter->second.parquet_field;
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*expected_field.type(), parquet_field));
+ if (expected_field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(child_projection, ProjectNested(*expected_field.type(),
+ parquet_field.children));
+ } else {
+ child_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(parquet_field.column_index);
+ }
+ child_projection.from = iter->second.local_index;
+ child_projection.kind = FieldProjection::Kind::kProjected;
+ } else if (MetadataColumns::IsMetadataColumn(field_id)) {
+ child_projection.kind = FieldProjection::Kind::kMetadata;
+ } else if (expected_field.optional()) {
+ child_projection.kind = FieldProjection::Kind::kNull;
Review Comment:
(So only struct can contain these `kMetadata` and `kNull` projections. List entry and map value cannot?
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
+ return {};
+ }
+ break;
+ case TypeId::kMap:
+ if (arrow_type->id() == ::arrow::Type::MAP) {
+ return {};
+ }
+ break;
+ default:
+ break;
+ }
+
+ return InvalidSchema("Cannot read Iceberg type: {} from Parquet type: {}",
+ expected_type, arrow_type->ToString());
+}
+
+// Forward declaration
+Result<FieldProjection> ProjectNested(
+ const Type& expected_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields);
+
+Result<FieldProjection> ProjectStruct(
+ const StructType& struct_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ struct FieldContext {
+ size_t local_index;
+ const ::parquet::arrow::SchemaField& parquet_field;
+ };
+ std::unordered_map<int32_t, FieldContext> field_context_map;
+ field_context_map.reserve(parquet_fields.size());
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ const ::parquet::arrow::SchemaField& parquet_field = parquet_fields[i];
+ auto field_id = GetFieldId(parquet_field);
+ if (!field_id) {
+ continue;
+ }
+ if (const auto [iter, inserted] = field_context_map.emplace(
+ std::piecewise_construct, std::forward_as_tuple(field_id.value()),
+ std::forward_as_tuple(i, parquet_field));
+ !inserted) [[unlikely]] {
+ return InvalidSchema("Duplicate field id found in Parquet schema: {}",
+ field_id.value());
+ }
+ }
+
+ FieldProjection result;
+ result.children.reserve(struct_type.fields().size());
+
+ for (const auto& expected_field : struct_type.fields()) {
+ int32_t field_id = expected_field.field_id();
+ FieldProjection child_projection;
+
+ if (auto iter = field_context_map.find(field_id); iter != field_context_map.cend()) {
+ const auto& parquet_field = iter->second.parquet_field;
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*expected_field.type(), parquet_field));
+ if (expected_field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(child_projection, ProjectNested(*expected_field.type(),
+ parquet_field.children));
+ } else {
+ child_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(parquet_field.column_index);
+ }
+ child_projection.from = iter->second.local_index;
+ child_projection.kind = FieldProjection::Kind::kProjected;
+ } else if (MetadataColumns::IsMetadataColumn(field_id)) {
+ child_projection.kind = FieldProjection::Kind::kMetadata;
+ } else if (expected_field.optional()) {
+ child_projection.kind = FieldProjection::Kind::kNull;
+ } else {
+ return InvalidSchema("Missing required field with id: {}", field_id);
+ }
+
+ result.children.emplace_back(std::move(child_projection));
+ }
+
+ PruneFieldProjection(result);
+ return result;
+}
+
+Result<FieldProjection> ProjectList(
+ const ListType& list_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ if (parquet_fields.size() != 1) {
+ return InvalidSchema("List type must have exactly one field, got {}",
+ parquet_fields.size());
+ }
+
+ const auto& parquet_field = parquet_fields[0];
+ auto element_field_id = GetFieldId(parquet_field);
+ if (!element_field_id) {
+ return InvalidSchema("List element field missing field id");
+ }
+
+ const auto& expected_element_field = list_type.fields().back();
+ if (expected_element_field.field_id() != element_field_id.value()) {
+ return InvalidSchema("List element field id mismatch, expected {}, got {}",
+ expected_element_field.field_id(), element_field_id.value());
+ }
+
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*expected_element_field.type(), parquet_field));
+
+ FieldProjection element_projection;
+ if (expected_element_field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(
+ element_projection,
+ ProjectNested(*expected_element_field.type(), parquet_field.children));
+ } else {
+ element_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(parquet_field.column_index);
+ }
+
+ element_projection.kind = FieldProjection::Kind::kProjected;
+ element_projection.from = size_t{0};
+
+ FieldProjection result;
+ result.children.emplace_back(std::move(element_projection));
+ return result;
+}
+
+Result<FieldProjection> ProjectMap(
+ const MapType& map_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ if (parquet_fields.size() != 2) {
+ return InvalidSchema("Map type must have exactly two fields, got {}",
+ parquet_fields.size());
+ }
+
+ auto key_field_id = GetFieldId(parquet_fields[0]);
+ if (!key_field_id) {
+ return InvalidSchema("Map key field missing field id");
+ }
+ auto value_field_id = GetFieldId(parquet_fields[1]);
+ if (!value_field_id) {
+ return InvalidSchema("Map value field missing field id");
+ }
+
+ const auto& expected_key_field = map_type.key();
+ const auto& expected_value_field = map_type.value();
+ if (expected_key_field.field_id() != key_field_id.value()) {
+ return InvalidSchema("Map key field id mismatch, expected {}, got {}",
+ expected_key_field.field_id(), key_field_id.value());
+ }
+ if (expected_value_field.field_id() != value_field_id.value()) {
+ return InvalidSchema("Map value field id mismatch, expected {}, got {}",
+ expected_value_field.field_id(), value_field_id.value());
+ }
+
+ FieldProjection result;
+
Review Comment:
reserve children to 2?
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
+ return {};
+ }
+ break;
+ case TypeId::kMap:
+ if (arrow_type->id() == ::arrow::Type::MAP) {
+ return {};
+ }
+ break;
+ default:
+ break;
+ }
+
+ return InvalidSchema("Cannot read Iceberg type: {} from Parquet type: {}",
+ expected_type, arrow_type->ToString());
+}
+
+// Forward declaration
+Result<FieldProjection> ProjectNested(
+ const Type& expected_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields);
+
+Result<FieldProjection> ProjectStruct(
+ const StructType& struct_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ struct FieldContext {
+ size_t local_index;
+ const ::parquet::arrow::SchemaField& parquet_field;
+ };
+ std::unordered_map<int32_t, FieldContext> field_context_map;
+ field_context_map.reserve(parquet_fields.size());
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ const ::parquet::arrow::SchemaField& parquet_field = parquet_fields[i];
+ auto field_id = GetFieldId(parquet_field);
+ if (!field_id) {
+ continue;
+ }
+ if (const auto [iter, inserted] = field_context_map.emplace(
+ std::piecewise_construct, std::forward_as_tuple(field_id.value()),
+ std::forward_as_tuple(i, parquet_field));
+ !inserted) [[unlikely]] {
+ return InvalidSchema("Duplicate field id found in Parquet schema: {}",
+ field_id.value());
+ }
+ }
+
+ FieldProjection result;
+ result.children.reserve(struct_type.fields().size());
+
+ for (const auto& expected_field : struct_type.fields()) {
+ int32_t field_id = expected_field.field_id();
+ FieldProjection child_projection;
+
+ if (auto iter = field_context_map.find(field_id); iter != field_context_map.cend()) {
+ const auto& parquet_field = iter->second.parquet_field;
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*expected_field.type(), parquet_field));
+ if (expected_field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(child_projection, ProjectNested(*expected_field.type(),
+ parquet_field.children));
+ } else {
+ child_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(parquet_field.column_index);
+ }
+ child_projection.from = iter->second.local_index;
+ child_projection.kind = FieldProjection::Kind::kProjected;
+ } else if (MetadataColumns::IsMetadataColumn(field_id)) {
+ child_projection.kind = FieldProjection::Kind::kMetadata;
+ } else if (expected_field.optional()) {
+ child_projection.kind = FieldProjection::Kind::kNull;
+ } else {
+ return InvalidSchema("Missing required field with id: {}", field_id);
+ }
+
+ result.children.emplace_back(std::move(child_projection));
+ }
+
+ PruneFieldProjection(result);
+ return result;
+}
+
+Result<FieldProjection> ProjectList(
+ const ListType& list_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ if (parquet_fields.size() != 1) {
+ return InvalidSchema("List type must have exactly one field, got {}",
+ parquet_fields.size());
+ }
+
+ const auto& parquet_field = parquet_fields[0];
+ auto element_field_id = GetFieldId(parquet_field);
+ if (!element_field_id) {
+ return InvalidSchema("List element field missing field id");
+ }
+
+ const auto& expected_element_field = list_type.fields().back();
+ if (expected_element_field.field_id() != element_field_id.value()) {
+ return InvalidSchema("List element field id mismatch, expected {}, got {}",
+ expected_element_field.field_id(), element_field_id.value());
+ }
+
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*expected_element_field.type(), parquet_field));
+
+ FieldProjection element_projection;
+ if (expected_element_field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(
+ element_projection,
+ ProjectNested(*expected_element_field.type(), parquet_field.children));
+ } else {
+ element_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(parquet_field.column_index);
+ }
+
+ element_projection.kind = FieldProjection::Kind::kProjected;
+ element_projection.from = size_t{0};
+
+ FieldProjection result;
+ result.children.emplace_back(std::move(element_projection));
+ return result;
+}
+
+Result<FieldProjection> ProjectMap(
+ const MapType& map_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ if (parquet_fields.size() != 2) {
+ return InvalidSchema("Map type must have exactly two fields, got {}",
+ parquet_fields.size());
+ }
+
+ auto key_field_id = GetFieldId(parquet_fields[0]);
+ if (!key_field_id) {
+ return InvalidSchema("Map key field missing field id");
+ }
+ auto value_field_id = GetFieldId(parquet_fields[1]);
+ if (!value_field_id) {
+ return InvalidSchema("Map value field missing field id");
+ }
+
+ const auto& expected_key_field = map_type.key();
+ const auto& expected_value_field = map_type.value();
+ if (expected_key_field.field_id() != key_field_id.value()) {
+ return InvalidSchema("Map key field id mismatch, expected {}, got {}",
+ expected_key_field.field_id(), key_field_id.value());
+ }
+ if (expected_value_field.field_id() != value_field_id.value()) {
+ return InvalidSchema("Map value field id mismatch, expected {}, got {}",
+ expected_value_field.field_id(), value_field_id.value());
+ }
+
+ FieldProjection result;
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ FieldProjection sub_projection;
+ const auto& sub_node = parquet_fields[i];
+ const auto& expected_sub_field = map_type.fields()[i];
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*expected_sub_field.type(), sub_node));
+ if (expected_sub_field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(
+ sub_projection, ProjectNested(*expected_sub_field.type(), sub_node.children));
+ } else {
+ sub_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(sub_node.column_index);
+ }
+ sub_projection.kind = FieldProjection::Kind::kProjected;
+ sub_projection.from = i;
+ result.children.emplace_back(std::move(sub_projection));
+ }
+
+ return result;
+}
+
+Result<FieldProjection> ProjectNested(
+ const Type& expected_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ if (!expected_type.is_nested()) {
+ return InvalidSchema("Expected a nested type, but got {}", expected_type);
+ }
+
+ switch (expected_type.type_id()) {
+ case TypeId::kStruct:
+ return ProjectStruct(internal::checked_cast<const StructType&>(expected_type),
+ parquet_fields);
+ case TypeId::kList:
+ return ProjectList(internal::checked_cast<const ListType&>(expected_type),
+ parquet_fields);
+ case TypeId::kMap:
+ if (parquet_fields.size() != 1 ||
+ parquet_fields[0].field->type()->id() != ::arrow::Type::STRUCT ||
+ parquet_fields[0].children.size() != 2) {
+ return InvalidSchema(
+ "Map type must have exactly one struct field with two children");
+ }
+ return ProjectMap(internal::checked_cast<const MapType&>(expected_type),
+ parquet_fields[0].children);
+ default:
+ return InvalidSchema("Unsupported nested type: {}", expected_type);
+ }
+}
+
+void CollectColumnIds(const FieldProjection& field_projection,
+ std::vector<int32_t>* column_ids) {
+ if (field_projection.attributes) {
+ auto parquet_attributes = internal::checked_cast<const ParquetExtraAttributes&>(
Review Comment:
```suggestion
const auto& parquet_attributes = internal::checked_cast<const ParquetExtraAttributes&>(
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2253652410
##########
test/parquet_schema_test.cc:
##########
@@ -50,17 +136,375 @@ TEST(HasFieldIds, PrimitiveNode) {
TEST(HasFieldIds, GroupNode) {
auto group_node_without_field_id =
MakeGroupNode("test_group", {MakeInt32Node("c1"), MakeInt32Node("c2")});
- EXPECT_FALSE(HasFieldIds(group_node_without_field_id));
+ EXPECT_FALSE(HasFieldIds(group_node_without_field_id)); // NOLINT
auto group_node_with_full_field_id = MakeGroupNode(
"test_group",
{MakeInt32Node("c1", /*field_id=*/2), MakeInt32Node("c2", /*field_id=*/3)},
/*field_id=*/1);
- EXPECT_TRUE(HasFieldIds(group_node_with_full_field_id));
+ EXPECT_TRUE(HasFieldIds(group_node_with_full_field_id)); // NOLINT
auto group_node_with_partial_field_id = MakeGroupNode(
"test_group", {MakeInt32Node("c1", /*field_id=*/1), MakeInt32Node("c2")});
- EXPECT_TRUE(HasFieldIds(group_node_with_partial_field_id));
+ EXPECT_TRUE(HasFieldIds(group_node_with_partial_field_id)); // NOLINT
Review Comment:
nits: could we add a comment saying why there's a nolint (and/or use `NOLINT(name-of-lint)` to be specific), and maybe NOLINTBEGIN/NOLINTEND would be cleaner (up 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2253672707
##########
test/parquet_schema_test.cc:
##########
@@ -50,17 +136,375 @@ TEST(HasFieldIds, PrimitiveNode) {
TEST(HasFieldIds, GroupNode) {
auto group_node_without_field_id =
MakeGroupNode("test_group", {MakeInt32Node("c1"), MakeInt32Node("c2")});
- EXPECT_FALSE(HasFieldIds(group_node_without_field_id));
+ EXPECT_FALSE(HasFieldIds(group_node_without_field_id)); // NOLINT
auto group_node_with_full_field_id = MakeGroupNode(
"test_group",
{MakeInt32Node("c1", /*field_id=*/2), MakeInt32Node("c2", /*field_id=*/3)},
/*field_id=*/1);
- EXPECT_TRUE(HasFieldIds(group_node_with_full_field_id));
+ EXPECT_TRUE(HasFieldIds(group_node_with_full_field_id)); // NOLINT
auto group_node_with_partial_field_id = MakeGroupNode(
"test_group", {MakeInt32Node("c1", /*field_id=*/1), MakeInt32Node("c2")});
- EXPECT_TRUE(HasFieldIds(group_node_with_partial_field_id));
+ EXPECT_TRUE(HasFieldIds(group_node_with_partial_field_id)); // NOLINT
Review Comment:
Agreed. I'm still investigating so it is not finalized yet.
FTR: bumping clang-tidy version from 19 to 22 does eliminate errors from 8 to 1.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2254028652
##########
test/parquet_schema_test.cc:
##########
@@ -50,17 +136,375 @@ TEST(HasFieldIds, PrimitiveNode) {
TEST(HasFieldIds, GroupNode) {
auto group_node_without_field_id =
MakeGroupNode("test_group", {MakeInt32Node("c1"), MakeInt32Node("c2")});
- EXPECT_FALSE(HasFieldIds(group_node_without_field_id));
+ EXPECT_FALSE(HasFieldIds(group_node_without_field_id)); // NOLINT
auto group_node_with_full_field_id = MakeGroupNode(
"test_group",
{MakeInt32Node("c1", /*field_id=*/2), MakeInt32Node("c2", /*field_id=*/3)},
/*field_id=*/1);
- EXPECT_TRUE(HasFieldIds(group_node_with_full_field_id));
+ EXPECT_TRUE(HasFieldIds(group_node_with_full_field_id)); // NOLINT
auto group_node_with_partial_field_id = MakeGroupNode(
"test_group", {MakeInt32Node("c1", /*field_id=*/1), MakeInt32Node("c2")});
- EXPECT_TRUE(HasFieldIds(group_node_with_partial_field_id));
+ EXPECT_TRUE(HasFieldIds(group_node_with_partial_field_id)); // NOLINT
Review Comment:
Ah, nice. So it does seem like a false positive then
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2267799110
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ const auto& time_type =
+ internal::checked_cast<const ::arrow::Time64Type&>(*arrow_type);
+ if (time_type.unit() == ::arrow::TimeUnit::MICRO) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
Review Comment:
We should probably also allow https://github.com/apache/arrow/blob/main/cpp/src/arrow/extension/uuid.h#L35
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2267799110
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ const auto& time_type =
+ internal::checked_cast<const ::arrow::Time64Type&>(*arrow_type);
+ if (time_type.unit() == ::arrow::TimeUnit::MICRO) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
Review Comment:
We should probably also allow https://github.com/apache/arrow/blob/main/cpp/src/arrow/extension/uuid.h#L35
You can validate via `arrow_type->id() == ::arrow::Type::EXTENSION` and the `extension_name()` == `"arrow.uuid"`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2254572871
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
Review Comment:
I'm not sure. I think we can delay this support until we implement V3 unknown type.
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
+ return {};
+ }
+ break;
+ case TypeId::kMap:
+ if (arrow_type->id() == ::arrow::Type::MAP) {
+ return {};
+ }
+ break;
+ default:
+ break;
+ }
+
+ return InvalidSchema("Cannot read Iceberg type: {} from Parquet type: {}",
+ expected_type, arrow_type->ToString());
+}
+
+// Forward declaration
+Result<FieldProjection> ProjectNested(
+ const Type& expected_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields);
+
+Result<FieldProjection> ProjectStruct(
+ const StructType& struct_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ struct FieldContext {
+ size_t local_index;
+ const ::parquet::arrow::SchemaField& parquet_field;
+ };
+ std::unordered_map<int32_t, FieldContext> field_context_map;
+ field_context_map.reserve(parquet_fields.size());
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ const ::parquet::arrow::SchemaField& parquet_field = parquet_fields[i];
+ auto field_id = GetFieldId(parquet_field);
+ if (!field_id) {
+ continue;
+ }
+ if (const auto [iter, inserted] = field_context_map.emplace(
+ std::piecewise_construct, std::forward_as_tuple(field_id.value()),
+ std::forward_as_tuple(i, parquet_field));
+ !inserted) [[unlikely]] {
+ return InvalidSchema("Duplicate field id found in Parquet schema: {}",
Review Comment:
Unfortunately we don't have full schema here and we don't even know the field names.
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
+ return {};
+ }
+ break;
+ case TypeId::kMap:
+ if (arrow_type->id() == ::arrow::Type::MAP) {
+ return {};
+ }
+ break;
+ default:
+ break;
+ }
+
+ return InvalidSchema("Cannot read Iceberg type: {} from Parquet type: {}",
+ expected_type, arrow_type->ToString());
+}
+
+// Forward declaration
+Result<FieldProjection> ProjectNested(
+ const Type& expected_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields);
+
+Result<FieldProjection> ProjectStruct(
+ const StructType& struct_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ struct FieldContext {
+ size_t local_index;
+ const ::parquet::arrow::SchemaField& parquet_field;
+ };
+ std::unordered_map<int32_t, FieldContext> field_context_map;
+ field_context_map.reserve(parquet_fields.size());
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ const ::parquet::arrow::SchemaField& parquet_field = parquet_fields[i];
+ auto field_id = GetFieldId(parquet_field);
+ if (!field_id) {
+ continue;
+ }
+ if (const auto [iter, inserted] = field_context_map.emplace(
+ std::piecewise_construct, std::forward_as_tuple(field_id.value()),
+ std::forward_as_tuple(i, parquet_field));
+ !inserted) [[unlikely]] {
+ return InvalidSchema("Duplicate field id found in Parquet schema: {}",
+ field_id.value());
+ }
+ }
+
+ FieldProjection result;
+ result.children.reserve(struct_type.fields().size());
+
+ for (const auto& expected_field : struct_type.fields()) {
+ int32_t field_id = expected_field.field_id();
+ FieldProjection child_projection;
+
+ if (auto iter = field_context_map.find(field_id); iter != field_context_map.cend()) {
+ const auto& parquet_field = iter->second.parquet_field;
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*expected_field.type(), parquet_field));
+ if (expected_field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(child_projection, ProjectNested(*expected_field.type(),
+ parquet_field.children));
+ } else {
+ child_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(parquet_field.column_index);
+ }
+ child_projection.from = iter->second.local_index;
+ child_projection.kind = FieldProjection::Kind::kProjected;
+ } else if (MetadataColumns::IsMetadataColumn(field_id)) {
+ child_projection.kind = FieldProjection::Kind::kMetadata;
+ } else if (expected_field.optional()) {
+ child_projection.kind = FieldProjection::Kind::kNull;
Review Comment:
Right. I think direct children of map and list types do not support schema evolution per the spec (but if the child is a struct, its fields support schema evolution).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2254758132
##########
test/parquet_schema_test.cc:
##########
@@ -17,50 +17,494 @@
* under the License.
*/
-#include <gtest/gtest.h>
+#include <arrow/type.h>
+#include <parquet/arrow/reader.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
-#include <parquet/types.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/schema.h"
+#include "matchers.h"
namespace iceberg::parquet {
namespace {
-::parquet::schema::NodePtr MakeInt32Node(const std::string& name, int field_id = -1) {
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+::parquet::schema::NodePtr MakeInt32Node(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::INT32, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeInt64Node(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::INT64, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeStringNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
return ::parquet::schema::PrimitiveNode::Make(
- name, ::parquet::Repetition::REQUIRED, ::parquet::LogicalType::None(),
- ::parquet::Type::INT32, /*primitive_length=*/-1, field_id);
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::String(), ::parquet::Type::BYTE_ARRAY,
+ /*primitive_length=*/-1, field_id);
+}
+
+::parquet::schema::NodePtr MakeDoubleNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::DOUBLE, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeFloatNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::FLOAT, /*primitive_length=*/-1,
+ field_id);
}
::parquet::schema::NodePtr MakeGroupNode(const std::string& name,
const ::parquet::schema::NodeVector& fields,
- int field_id = -1) {
- return ::parquet::schema::GroupNode::Make(name, ::parquet::Repetition::REQUIRED, fields,
- /*logical_type=*/nullptr, field_id);
+ int field_id = -1, bool optional = true) {
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ fields, /*logical_type=*/nullptr, field_id);
}
+::parquet::schema::NodePtr MakeListNode(const std::string& name,
+ const ::parquet::schema::NodePtr& element_node,
+ int field_id = -1, bool optional = true) {
+ auto list_group = ::parquet::schema::GroupNode::Make(
+ "element", ::parquet::Repetition::REPEATED, {element_node});
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ {list_group}, ::parquet::LogicalType::List(), field_id);
+}
+
+::parquet::schema::NodePtr MakeMapNode(const std::string& name,
+ const ::parquet::schema::NodePtr& key_node,
+ const ::parquet::schema::NodePtr& value_node,
+ int field_id = -1, bool optional = true) {
+ auto key_value_group = ::parquet::schema::GroupNode::Make(
+ "key_value", ::parquet::Repetition::REPEATED, {key_node, value_node});
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ {key_value_group}, ::parquet::LogicalType::Map(), field_id);
+}
+
+// Helper to create SchemaManifest from Parquet schema
+::parquet::arrow::SchemaManifest MakeSchemaManifest(
+ const ::parquet::schema::NodePtr& parquet_schema) {
+ auto parquet_schema_descriptor = std::make_shared<::parquet::SchemaDescriptor>();
+ parquet_schema_descriptor->Init(parquet_schema);
+
+ ::parquet::arrow::SchemaManifest manifest;
+ auto status = ::parquet::arrow::SchemaManifest::Make(
+ parquet_schema_descriptor.get(), /*key_value_metadata=*/nullptr,
+ ::parquet::default_arrow_reader_properties(), &manifest);
+ if (!status.ok()) {
+ throw std::runtime_error("Failed to create SchemaManifest: " + status.ToString());
+ }
+ return manifest;
+}
+
+#define ASSERT_PROJECTED_FIELD(field_projection, index) \
+ ASSERT_EQ(field_projection.kind, FieldProjection::Kind::kProjected); \
+ ASSERT_EQ(std::get<1>(field_projection.from), index);
+
+#define ASSERT_PROJECTED_NULL_FIELD(field_projection) \
+ ASSERT_EQ(field_projection.kind, FieldProjection::Kind::kNull);
+
} // namespace
-TEST(HasFieldIds, PrimitiveNode) {
+TEST(HasFieldIdsTest, PrimitiveNode) {
EXPECT_FALSE(HasFieldIds(MakeInt32Node("test_field")));
EXPECT_TRUE(HasFieldIds(MakeInt32Node("test_field", /*field_id=*/1)));
}
-TEST(HasFieldIds, GroupNode) {
- auto group_node_without_field_id =
- MakeGroupNode("test_group", {MakeInt32Node("c1"), MakeInt32Node("c2")});
- EXPECT_FALSE(HasFieldIds(group_node_without_field_id));
+TEST(HasFieldIdsTest, GroupNode) {
Review Comment:
I guess the original test name `HasFieldIds` conflicts with the function name `HasFieldIds` so the `TEST` expansion may have confused the linter. @lidavidm @mapleFU
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2268664195
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ const auto& time_type =
+ internal::checked_cast<const ::arrow::Time64Type&>(*arrow_type);
+ if (time_type.unit() == ::arrow::TimeUnit::MICRO) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
Review Comment:
ListView is not supported by parquet-cpp yet. I think we should just support simple list and binary type variants in the early versions of iceberg-cpp. Once parquet-cpp has full support, we can leverage them later.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2262953240
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,393 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
Review Comment:
Should we also check for `::arrow::TimeUnit::MICRO` 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2253266405
##########
test/parquet_schema_test.cc:
##########
@@ -17,50 +17,494 @@
* under the License.
*/
+#include <arrow/type.h>
#include <gtest/gtest.h>
+#include <parquet/arrow/reader.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
-#include <parquet/types.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/schema.h"
+#include "matchers.h"
namespace iceberg::parquet {
namespace {
-::parquet::schema::NodePtr MakeInt32Node(const std::string& name, int field_id = -1) {
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+::parquet::schema::NodePtr MakeInt32Node(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::INT32, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeInt64Node(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::INT64, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeStringNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::String(), ::parquet::Type::BYTE_ARRAY,
+ /*primitive_length=*/-1, field_id);
+}
+
+::parquet::schema::NodePtr MakeDoubleNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::DOUBLE, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeFloatNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
return ::parquet::schema::PrimitiveNode::Make(
- name, ::parquet::Repetition::REQUIRED, ::parquet::LogicalType::None(),
- ::parquet::Type::INT32, /*primitive_length=*/-1, field_id);
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::FLOAT, /*primitive_length=*/-1,
+ field_id);
}
::parquet::schema::NodePtr MakeGroupNode(const std::string& name,
const ::parquet::schema::NodeVector& fields,
- int field_id = -1) {
- return ::parquet::schema::GroupNode::Make(name, ::parquet::Repetition::REQUIRED, fields,
- /*logical_type=*/nullptr, field_id);
+ int field_id = -1, bool optional = true) {
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ fields, /*logical_type=*/nullptr, field_id);
}
+::parquet::schema::NodePtr MakeListNode(const std::string& name,
+ const ::parquet::schema::NodePtr& element_node,
+ int field_id = -1, bool optional = true) {
+ auto list_group = ::parquet::schema::GroupNode::Make(
+ "element", ::parquet::Repetition::REPEATED, {element_node});
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ {list_group}, ::parquet::LogicalType::List(), field_id);
+}
+
+::parquet::schema::NodePtr MakeMapNode(const std::string& name,
+ const ::parquet::schema::NodePtr& key_node,
+ const ::parquet::schema::NodePtr& value_node,
+ int field_id = -1, bool optional = true) {
+ auto key_value_group = ::parquet::schema::GroupNode::Make(
+ "key_value", ::parquet::Repetition::REPEATED, {key_node, value_node});
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ {key_value_group}, ::parquet::LogicalType::Map(), field_id);
+}
+
+// Helper to create SchemaManifest from Parquet schema
+::parquet::arrow::SchemaManifest MakeSchemaManifest(
+ const ::parquet::schema::NodePtr& parquet_schema) {
+ auto parquet_schema_descriptor = std::make_shared<::parquet::SchemaDescriptor>();
+ parquet_schema_descriptor->Init(parquet_schema);
+
+ ::parquet::arrow::SchemaManifest manifest;
+ auto status = ::parquet::arrow::SchemaManifest::Make(
+ parquet_schema_descriptor.get(), /*key_value_metadata=*/nullptr,
+ ::parquet::default_arrow_reader_properties(), &manifest);
+ if (!status.ok()) {
+ throw std::runtime_error("Failed to create SchemaManifest: " + status.ToString());
+ }
+ return manifest;
+}
+
+#define ASSERT_PROJECTED_FIELD(field_projection, index) \
+ ASSERT_EQ(field_projection.kind, FieldProjection::Kind::kProjected); \
+ ASSERT_EQ(std::get<1>(field_projection.from), index);
+
+#define ASSERT_PROJECTED_NULL_FIELD(field_projection) \
+ ASSERT_EQ(field_projection.kind, FieldProjection::Kind::kNull);
+
} // namespace
TEST(HasFieldIds, PrimitiveNode) {
EXPECT_FALSE(HasFieldIds(MakeInt32Node("test_field")));
EXPECT_TRUE(HasFieldIds(MakeInt32Node("test_field", /*field_id=*/1)));
}
-TEST(HasFieldIds, GroupNode) {
- auto group_node_without_field_id =
- MakeGroupNode("test_group", {MakeInt32Node("c1"), MakeInt32Node("c2")});
- EXPECT_FALSE(HasFieldIds(group_node_without_field_id));
+// TEST(HasFieldIds, GroupNode) {
Review Comment:
The linter is satisfied if I comment out these lines. @lidavidm
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#issuecomment-3152997784
I'm guessing one of the GTest or GMock macros expands to something weird.
https://github.com/google/googletest/issues/4531
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#issuecomment-3162763055
@Fokko @zeroshade Could you help review this? Thanks!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2268660374
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
Review Comment:
No it won't. Reading dictionary is supported via an option to create a RecordReader: https://github.com/apache/arrow/blob/2dd3ccda6437f79aa34641bd3197dd7392ae4aec/cpp/src/parquet/column_reader.h#L266
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2252924180
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
Review Comment:
Can we use `std::from_chars` here?
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,392 @@
* under the License.
*/
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ try {
+ field_id = std::stoi(field_id_str);
+ } catch (const std::invalid_argument& e) {
+ return std::nullopt;
+ } catch (const std::out_of_range& e) {
+ return std::nullopt;
+ }
+ return field_id < 0 ? std::nullopt : std::make_optional(field_id);
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
Review Comment:
Can any field evolve from null?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2256826054
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,393 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ return {};
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kFixed:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() ==
+ internal::checked_cast<const FixedType&>(expected_type).length()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kStruct:
+ if (arrow_type->id() == ::arrow::Type::STRUCT) {
+ return {};
+ }
+ break;
+ case TypeId::kList:
+ if (arrow_type->id() == ::arrow::Type::LIST) {
+ return {};
+ }
+ break;
+ case TypeId::kMap:
+ if (arrow_type->id() == ::arrow::Type::MAP) {
+ return {};
+ }
+ break;
+ default:
+ break;
+ }
+
+ return InvalidSchema("Cannot read Iceberg type: {} from Parquet type: {}",
+ expected_type, arrow_type->ToString());
+}
+
+// Forward declaration
+Result<FieldProjection> ProjectNested(
+ const Type& nested_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields);
+
+Result<FieldProjection> ProjectStruct(
+ const StructType& struct_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ struct FieldContext {
+ size_t local_index;
+ const ::parquet::arrow::SchemaField& parquet_field;
+ };
+ std::unordered_map<int32_t, FieldContext> field_context_map;
+ field_context_map.reserve(parquet_fields.size());
+
+ for (size_t i = 0; i < parquet_fields.size(); ++i) {
+ const ::parquet::arrow::SchemaField& parquet_field = parquet_fields[i];
+ auto field_id = GetFieldId(parquet_field);
+ if (!field_id) {
+ continue;
+ }
+ if (const auto [iter, inserted] = field_context_map.emplace(
+ std::piecewise_construct, std::forward_as_tuple(field_id.value()),
+ std::forward_as_tuple(i, parquet_field));
+ !inserted) [[unlikely]] {
+ return InvalidSchema("Duplicate field id {} found in Parquet schema",
+ field_id.value());
+ }
+ }
+
+ FieldProjection result;
+ result.children.reserve(struct_type.fields().size());
+
+ for (const auto& field : struct_type.fields()) {
+ int32_t field_id = field.field_id();
+ FieldProjection child_projection;
+
+ if (auto iter = field_context_map.find(field_id); iter != field_context_map.cend()) {
+ const auto& parquet_field = iter->second.parquet_field;
+ ICEBERG_RETURN_UNEXPECTED(
+ ValidateParquetSchemaEvolution(*field.type(), parquet_field));
+ if (field.type()->is_nested()) {
+ ICEBERG_ASSIGN_OR_RAISE(child_projection,
+ ProjectNested(*field.type(), parquet_field.children));
+ } else {
+ child_projection.attributes =
+ std::make_shared<ParquetExtraAttributes>(parquet_field.column_index);
+ }
+ child_projection.from = iter->second.local_index;
+ child_projection.kind = FieldProjection::Kind::kProjected;
+ } else if (MetadataColumns::IsMetadataColumn(field_id)) {
+ child_projection.kind = FieldProjection::Kind::kMetadata;
+ } else if (field.optional()) {
+ child_projection.kind = FieldProjection::Kind::kNull;
+ } else {
+ return InvalidSchema("Missing required field with id: {}", field_id);
+ }
+
+ result.children.emplace_back(std::move(child_projection));
+ }
+
+ PruneFieldProjection(result);
+ return result;
+}
+
+Result<FieldProjection> ProjectList(
+ const ListType& list_type,
+ const std::vector<::parquet::arrow::SchemaField>& parquet_fields) {
+ if (parquet_fields.size() != 1) {
+ return InvalidSchema("List type must have exactly one field, got {}",
+ parquet_fields.size());
+ }
+
+ const auto& parquet_field = parquet_fields[0];
Review Comment:
nit: use `parquet_fields.back()`? Since you use `list_type.fields().back()` below.
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,393 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
Review Comment:
Below is the FindKey signature, can we just pass kParquetFieldIdKey without the .data() part?
```
/// \brief Perform linear search for key, returning -1 if not found
int FindKey(std::string_view key) const;
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2257516159
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,393 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey.data());
Review Comment:
Nice catch!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2268654735
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ const auto& time_type =
+ internal::checked_cast<const ::arrow::Time64Type&>(*arrow_type);
+ if (time_type.unit() == ::arrow::TimeUnit::MICRO) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
Review Comment:
I don't think parquet-cpp has supported these types.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection to parquet [iceberg-cpp]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#issuecomment-3152994800
> Hmm, not sure what that lint failure is talking about
Me neither :/
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2263131645
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,393 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
Review Comment:
Good catch! I have added an exhaustive test case to make sure I don't miss any primitive type.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2255712343
##########
test/parquet_schema_test.cc:
##########
@@ -17,50 +17,494 @@
* under the License.
*/
-#include <gtest/gtest.h>
+#include <arrow/type.h>
+#include <parquet/arrow/reader.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
-#include <parquet/types.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/schema.h"
+#include "matchers.h"
namespace iceberg::parquet {
namespace {
-::parquet::schema::NodePtr MakeInt32Node(const std::string& name, int field_id = -1) {
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+::parquet::schema::NodePtr MakeInt32Node(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::INT32, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeInt64Node(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::INT64, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeStringNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
return ::parquet::schema::PrimitiveNode::Make(
- name, ::parquet::Repetition::REQUIRED, ::parquet::LogicalType::None(),
- ::parquet::Type::INT32, /*primitive_length=*/-1, field_id);
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::String(), ::parquet::Type::BYTE_ARRAY,
+ /*primitive_length=*/-1, field_id);
+}
+
+::parquet::schema::NodePtr MakeDoubleNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::DOUBLE, /*primitive_length=*/-1,
+ field_id);
+}
+
+::parquet::schema::NodePtr MakeFloatNode(const std::string& name, int field_id = -1,
+ bool optional = true) {
+ return ::parquet::schema::PrimitiveNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ ::parquet::LogicalType::None(), ::parquet::Type::FLOAT, /*primitive_length=*/-1,
+ field_id);
}
::parquet::schema::NodePtr MakeGroupNode(const std::string& name,
const ::parquet::schema::NodeVector& fields,
- int field_id = -1) {
- return ::parquet::schema::GroupNode::Make(name, ::parquet::Repetition::REQUIRED, fields,
- /*logical_type=*/nullptr, field_id);
+ int field_id = -1, bool optional = true) {
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ fields, /*logical_type=*/nullptr, field_id);
}
+::parquet::schema::NodePtr MakeListNode(const std::string& name,
+ const ::parquet::schema::NodePtr& element_node,
+ int field_id = -1, bool optional = true) {
+ auto list_group = ::parquet::schema::GroupNode::Make(
+ "element", ::parquet::Repetition::REPEATED, {element_node});
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ {list_group}, ::parquet::LogicalType::List(), field_id);
+}
+
+::parquet::schema::NodePtr MakeMapNode(const std::string& name,
+ const ::parquet::schema::NodePtr& key_node,
+ const ::parquet::schema::NodePtr& value_node,
+ int field_id = -1, bool optional = true) {
+ auto key_value_group = ::parquet::schema::GroupNode::Make(
+ "key_value", ::parquet::Repetition::REPEATED, {key_node, value_node});
+ return ::parquet::schema::GroupNode::Make(
+ name, optional ? ::parquet::Repetition::OPTIONAL : ::parquet::Repetition::REQUIRED,
+ {key_value_group}, ::parquet::LogicalType::Map(), field_id);
+}
+
+// Helper to create SchemaManifest from Parquet schema
+::parquet::arrow::SchemaManifest MakeSchemaManifest(
+ const ::parquet::schema::NodePtr& parquet_schema) {
+ auto parquet_schema_descriptor = std::make_shared<::parquet::SchemaDescriptor>();
+ parquet_schema_descriptor->Init(parquet_schema);
+
+ ::parquet::arrow::SchemaManifest manifest;
+ auto status = ::parquet::arrow::SchemaManifest::Make(
+ parquet_schema_descriptor.get(), /*key_value_metadata=*/nullptr,
+ ::parquet::default_arrow_reader_properties(), &manifest);
+ if (!status.ok()) {
+ throw std::runtime_error("Failed to create SchemaManifest: " + status.ToString());
+ }
+ return manifest;
+}
+
+#define ASSERT_PROJECTED_FIELD(field_projection, index) \
+ ASSERT_EQ(field_projection.kind, FieldProjection::Kind::kProjected); \
+ ASSERT_EQ(std::get<1>(field_projection.from), index);
+
+#define ASSERT_PROJECTED_NULL_FIELD(field_projection) \
+ ASSERT_EQ(field_projection.kind, FieldProjection::Kind::kNull);
+
} // namespace
-TEST(HasFieldIds, PrimitiveNode) {
+TEST(HasFieldIdsTest, PrimitiveNode) {
EXPECT_FALSE(HasFieldIds(MakeInt32Node("test_field")));
EXPECT_TRUE(HasFieldIds(MakeInt32Node("test_field", /*field_id=*/1)));
}
-TEST(HasFieldIds, GroupNode) {
- auto group_node_without_field_id =
- MakeGroupNode("test_group", {MakeInt32Node("c1"), MakeInt32Node("c2")});
- EXPECT_FALSE(HasFieldIds(group_node_without_field_id));
+TEST(HasFieldIdsTest, GroupNode) {
Review Comment:
Never mind. I don't know why it did succeed once but keep failing after that.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2267803477
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ const auto& time_type =
+ internal::checked_cast<const ::arrow::Time64Type&>(*arrow_type);
+ if (time_type.unit() == ::arrow::TimeUnit::MICRO) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
Review Comment:
What about LargeString, LargeBinary, StringView and BinaryView?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] feat(parquet): add schema projection for parquet [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #159:
URL: https://github.com/apache/iceberg-cpp/pull/159#discussion_r2268673543
##########
src/iceberg/parquet/parquet_schema_util.cc:
##########
@@ -17,20 +17,397 @@
* under the License.
*/
+#include <charconv>
+
+#include <arrow/type.h>
+#include <arrow/type_fwd.h>
+#include <arrow/util/key_value_metadata.h>
+#include <parquet/arrow/schema.h>
#include <parquet/schema.h>
+#include "iceberg/metadata_columns.h"
#include "iceberg/parquet/parquet_schema_util_internal.h"
+#include "iceberg/result.h"
+#include "iceberg/schema_util_internal.h"
#include "iceberg/util/checked_cast.h"
+#include "iceberg/util/formatter.h"
+#include "iceberg/util/macros.h"
namespace iceberg::parquet {
+namespace {
+
+constexpr std::string_view kParquetFieldIdKey = "PARQUET:field_id";
+
+std::optional<int32_t> FieldIdFromMetadata(
+ const std::shared_ptr<const ::arrow::KeyValueMetadata>& metadata) {
+ if (!metadata) {
+ return std::nullopt;
+ }
+ int key = metadata->FindKey(kParquetFieldIdKey);
+ if (key < 0) {
+ return std::nullopt;
+ }
+ std::string field_id_str = metadata->value(key);
+ int32_t field_id = -1;
+ auto [_, ec] = std::from_chars(field_id_str.data(),
+ field_id_str.data() + field_id_str.size(), field_id);
+ if (ec != std::errc() || field_id < 0) {
+ return std::nullopt;
+ }
+ return field_id;
+}
+
+std::optional<int32_t> GetFieldId(const ::parquet::arrow::SchemaField& parquet_field) {
+ return FieldIdFromMetadata(parquet_field.field->metadata());
+}
+
+// TODO(gangwu): support v3 unknown type
+Status ValidateParquetSchemaEvolution(
+ const Type& expected_type, const ::parquet::arrow::SchemaField& parquet_field) {
+ const auto& arrow_type = parquet_field.field->type();
+ switch (expected_type.type_id()) {
+ case TypeId::kBoolean:
+ if (arrow_type->id() == ::arrow::Type::BOOL) {
+ return {};
+ }
+ break;
+ case TypeId::kInt:
+ if (arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kLong:
+ if (arrow_type->id() == ::arrow::Type::INT64 ||
+ arrow_type->id() == ::arrow::Type::INT32) {
+ return {};
+ }
+ break;
+ case TypeId::kFloat:
+ if (arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDouble:
+ if (arrow_type->id() == ::arrow::Type::DOUBLE ||
+ arrow_type->id() == ::arrow::Type::FLOAT) {
+ return {};
+ }
+ break;
+ case TypeId::kDate:
+ if (arrow_type->id() == ::arrow::Type::DATE32) {
+ return {};
+ }
+ break;
+ case TypeId::kTime:
+ if (arrow_type->id() == ::arrow::Type::TIME64) {
+ const auto& time_type =
+ internal::checked_cast<const ::arrow::Time64Type&>(*arrow_type);
+ if (time_type.unit() == ::arrow::TimeUnit::MICRO) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestamp:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kTimestampTz:
+ if (arrow_type->id() == ::arrow::Type::TIMESTAMP) {
+ const auto& timestamp_type =
+ internal::checked_cast<const ::arrow::TimestampType&>(*arrow_type);
+ if (timestamp_type.unit() == ::arrow::TimeUnit::MICRO &&
+ !timestamp_type.timezone().empty()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kString:
+ if (arrow_type->id() == ::arrow::Type::STRING) {
+ return {};
+ }
+ break;
+ case TypeId::kBinary:
+ if (arrow_type->id() == ::arrow::Type::BINARY) {
+ return {};
+ }
+ break;
+ case TypeId::kDecimal:
+ if (arrow_type->id() == ::arrow::Type::DECIMAL128) {
+ const auto& decimal_type =
+ internal::checked_cast<const DecimalType&>(expected_type);
+ const auto& arrow_decimal =
+ internal::checked_cast<const ::arrow::Decimal128Type&>(*arrow_type);
+ if (decimal_type.scale() == arrow_decimal.scale() &&
+ decimal_type.precision() >= arrow_decimal.precision()) {
+ return {};
+ }
+ }
+ break;
+ case TypeId::kUuid:
+ if (arrow_type->id() == ::arrow::Type::FIXED_SIZE_BINARY) {
+ const auto& fixed_binary =
+ internal::checked_cast<const ::arrow::FixedSizeBinaryType&>(*arrow_type);
+ if (fixed_binary.byte_width() == 16) {
+ return {};
+ }
+ }
Review Comment:
Good catch! Fixed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org