You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "manuzhang (via GitHub)" <gi...@apache.org> on 2026/05/20 11:07:24 UTC
[PR] feat: support Iceberg v3 unknown type [iceberg-cpp]
manuzhang opened a new pull request, #662:
URL: https://github.com/apache/iceberg-cpp/pull/662
## Summary
- Add Iceberg v3 unknown primitive type and JSON serialization/deserialization support.
- Support unknown as null-only data across Arrow, Avro, Parquet, schema projection, and nested fields.
- Enforce required-field invariants for unknown/null-only projections and Arrow null imports.
## Validation
- ctest --test-dir build --output-on-failure
- git diff --check upstream/main...HEAD
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3356654337
##########
src/iceberg/test/update_schema_test.cc:
##########
@@ -1054,6 +1057,113 @@ TEST_F(UpdateSchemaTest, UpdateColumnFloatToDouble) {
EXPECT_EQ(*field_opt->get().type(), *float64());
}
+TEST_F(UpdateSchemaTest, UpdateColumnUnknownToPrimitive) {
+ ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
+ update->AddColumn("mystery", unknown(), "A null-only placeholder");
Review Comment:
Doesn't it require format version v3 to succeed?
##########
src/iceberg/avro/avro_writer.cc:
##########
@@ -81,6 +89,126 @@ Result<std::optional<int32_t>> ParseCodecLevel(const WriterProperties& propertie
return level;
}
+enum class FieldContext {
+ kTopLevel,
+ kStruct,
+ kListElement,
+ kMapKey,
+ kMapValue,
+};
+
+Result<std::optional<SchemaField>> PruneUnknownField(const SchemaField& field,
Review Comment:
I don't think it is worth this significant change but only top-level unknown fields are pruned. We should just simplify this by always writing AVRO_NULL for unknown types. Same for parquet writer.
##########
src/iceberg/update/update_schema.h:
##########
@@ -180,23 +180,23 @@ class ICEBERG_EXPORT UpdateSchema : public PendingUpdate {
/// this change conflicts with other additions, renames, or updates.
UpdateSchema& RenameColumn(std::string_view name, std::string_view new_name);
- /// \brief Update a column in the schema to a new primitive type.
+ /// \brief Update a column in the schema to a new type.
///
/// The name is used to find the column to update using Schema::FindFieldByName().
///
- /// Only updates that widen types are allowed.
+ /// Only updates that widen types are allowed. Unknown type placeholders may be
+ /// updated to any type.
///
/// Columns may be updated and renamed in the same schema update.
///
/// \param name Name of the column to update.
- /// \param new_type Replacement type for the column (must be primitive).
+ /// \param new_type Replacement type for the column.
/// \return Reference to this for method chaining.
/// \note InvalidArgument will be reported if name doesn't identify a column in the
/// schema or if
/// this change introduces a type incompatibility or if it conflicts with
/// other additions, renames, or updates.
- UpdateSchema& UpdateColumn(std::string_view name,
- std::shared_ptr<PrimitiveType> new_type);
+ UpdateSchema& UpdateColumn(std::string_view name, std::shared_ptr<Type> new_type);
Review Comment:
I don't think this is the right direction. As per the spec, unknown type can only be promoted to primitive type. The Java implementation follows this rule so it is consistent.
##########
src/iceberg/json_serde.cc:
##########
@@ -441,12 +443,22 @@ Result<std::unique_ptr<Type>> StructTypeFromJson(const nlohmann::json& json) {
return std::make_unique<StructType>(std::move(fields));
}
+Status ValidateUnknownFieldOptional(const Type& type, bool optional,
+ std::string_view field_name) {
+ if (type.type_id() == TypeId::kUnknown && !optional) {
+ return JsonParseError("Unknown type field '{}' must be optional", field_name);
+ }
+ return {};
+}
+
Result<std::unique_ptr<Type>> ListTypeFromJson(const nlohmann::json& json) {
ICEBERG_ASSIGN_OR_RAISE(auto element_type, TypeFromJson(json[kElement]));
ICEBERG_ASSIGN_OR_RAISE(auto element_id, GetJsonValue<int32_t>(json, kElementId));
ICEBERG_ASSIGN_OR_RAISE(auto element_required,
GetJsonValue<bool>(json, kElementRequired));
+ ICEBERG_RETURN_UNEXPECTED(ValidateUnknownFieldOptional(*element_type, !element_required,
Review Comment:
This check is duplicate since Schema::Validate will cover the same thing.
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -650,6 +661,35 @@ Result<FieldProjection> ProjectNested(const Type& expected_type,
const ::avro::NodePtr& avro_node,
bool prune_source);
+Result<FieldProjection> ProjectField(const SchemaField& expected_field,
Review Comment:
Nice refactoring!
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3286530627
##########
src/iceberg/type_fwd.h:
##########
@@ -31,7 +31,7 @@ namespace iceberg {
/// This is not a complete data type by itself because some types are nested
/// and/or parameterized.
///
-/// Iceberg V3 types are not currently supported.
+/// Iceberg V3's `unknown` type is supported as a null-only placeholder type.
Review Comment:
Please register `TypeId::kUnknown -> 3` in `TableMetadata::kMinFormatVersions`. This advertises a v3-only type, but the validation table is still empty, so v1/v2 metadata can contain `unknown`. Java rejects `UNKNOWN` before v3.
##########
src/iceberg/util/type_util.cc:
##########
@@ -426,6 +426,10 @@ bool IsPromotionAllowed(const std::shared_ptr<Type>& from_type,
TypeId from_id = from_type->type_id();
TypeId to_id = to_type->type_id();
+ if (from_id == TypeId::kUnknown) {
Review Comment:
Java does not allow `UNKNOWN` as a schema-update promotion source. With this, `UpdateColumn` can turn an `unknown` column into any primitive type, which diverges from Java's `TypeUtil.isPromotionAllowed` rules.
##########
src/iceberg/json_serde.cc:
##########
@@ -540,6 +558,7 @@ Result<std::unique_ptr<SchemaField>> FieldFromJson(const nlohmann::json& json) {
ICEBERG_ASSIGN_OR_RAISE(auto required, GetJsonValue<bool>(json, kRequired));
ICEBERG_ASSIGN_OR_RAISE(auto doc, GetJsonValueOrDefault<std::string>(json, kDoc));
+ ICEBERG_RETURN_UNEXPECTED(ValidateUnknownFieldOptional(*type, !required, name));
Review Comment:
This only rejects required `unknown` during JSON parsing. The public construction/update paths still allow it via `SchemaField::MakeRequired(..., unknown())` and `AllowIncompatibleChanges().AddRequiredColumn(..., unknown())`. The invariant should live in the model/update path too.
##########
src/iceberg/schema_internal.cc:
##########
@@ -141,6 +141,9 @@ ArrowErrorCode ToArrowSchema(const Type& type, bool optional, std::string_view n
ArrowMetadataBuilderAppend(&metadata_buffer, ArrowCharView(kArrowExtensionName),
ArrowCharView(kArrowUuidExtensionName)));
} break;
+ case TypeId::kUnknown:
Review Comment:
Java Parquet skips unknown fields on write. Mapping `unknown` to Arrow null here makes the Parquet writer emit a physical null column, or depend on Arrow rejecting it. The write path should skip unknown columns instead.
##########
src/iceberg/schema_util.cc:
##########
@@ -79,6 +82,55 @@ Status ValidateSchemaEvolution(const Type& expected_type, const Type& source_typ
return NotSupported("Cannot read {} from {}", expected_type, source_type);
}
+Result<FieldProjection> ProjectNested(const Type& expected_type, const Type& source_type,
+ bool prune_source);
+
+Result<FieldProjection> ProjectField(const SchemaField& expected_field,
+ const SchemaField& source_field, size_t source_index,
+ bool prune_source) {
+ FieldProjection projection;
+
+ if (expected_field.type()->type_id() == TypeId::kUnknown) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown &&
+ expected_field.type()->is_nested()) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown && !expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+
+ if (expected_field.type()->is_nested()) {
Review Comment:
`unknown` is still a primitive type, so it should not evolve to struct/list/map. This path should fail instead of projecting the whole nested value as 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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3287137948
##########
src/iceberg/test/update_schema_test.cc:
##########
@@ -1054,6 +1054,36 @@ TEST_F(UpdateSchemaTest, UpdateColumnFloatToDouble) {
EXPECT_EQ(*field_opt->get().type(), *float64());
}
+TEST_F(UpdateSchemaTest, UpdateColumnUnknownToPrimitiveFails) {
Review Comment:
Spec v3 allows `unknown` to promote to any primitive type. This should succeed; `unknown -> struct/list/map` should be rejected instead.
##########
src/iceberg/parquet/parquet_writer.cc:
##########
@@ -71,6 +78,64 @@ Result<std::optional<int32_t>> ParseCodecLevel(const WriterProperties& propertie
return level;
}
+std::optional<std::shared_ptr<Type>> PruneUnknownType(const std::shared_ptr<Type>& type);
+
+std::optional<SchemaField> PruneUnknownField(const SchemaField& field) {
+ auto pruned_type = PruneUnknownType(field.type());
+ if (!pruned_type.has_value()) {
+ return std::nullopt;
+ }
+ return SchemaField(field.field_id(), field.name(), std::move(pruned_type.value()),
+ field.optional(), field.doc());
+}
+
+std::optional<std::shared_ptr<Type>> PruneUnknownType(const std::shared_ptr<Type>& type) {
+ switch (type->type_id()) {
+ case TypeId::kUnknown:
+ return std::nullopt;
+ case TypeId::kStruct: {
+ const auto& struct_type = static_cast<const StructType&>(*type);
+ std::vector<SchemaField> fields;
+ for (const auto& field : struct_type.fields()) {
+ if (auto pruned_field = PruneUnknownField(field)) {
+ fields.emplace_back(std::move(pruned_field.value()));
+ }
+ }
+ return std::make_shared<StructType>(std::move(fields));
+ }
+ case TypeId::kList: {
+ const auto& list_type = static_cast<const ListType&>(*type);
+ auto pruned_element = PruneUnknownField(list_type.element());
+ if (!pruned_element.has_value()) {
Review Comment:
Pruning `list<unknown>` or `map<..., unknown>` drops the whole container, including list lengths or map keys. Java rejects these on write; only top-level/struct unknown fields should be skipped.
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3287254438
##########
src/iceberg/schema_util.cc:
##########
@@ -79,6 +82,55 @@ Status ValidateSchemaEvolution(const Type& expected_type, const Type& source_typ
return NotSupported("Cannot read {} from {}", expected_type, source_type);
}
+Result<FieldProjection> ProjectNested(const Type& expected_type, const Type& source_type,
+ bool prune_source);
+
+Result<FieldProjection> ProjectField(const SchemaField& expected_field,
+ const SchemaField& source_field, size_t source_index,
+ bool prune_source) {
+ FieldProjection projection;
+
+ if (expected_field.type()->type_id() == TypeId::kUnknown) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown &&
+ expected_field.type()->is_nested()) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown && !expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+
+ if (expected_field.type()->is_nested()) {
Review Comment:
https://iceberg.apache.org/spec/#schema-evolution
It says `Valid primitive type promotions are` and `Evolving primitive types to structs is not allowed`.
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3287060955
##########
src/iceberg/schema_util.cc:
##########
@@ -79,6 +82,55 @@ Status ValidateSchemaEvolution(const Type& expected_type, const Type& source_typ
return NotSupported("Cannot read {} from {}", expected_type, source_type);
}
+Result<FieldProjection> ProjectNested(const Type& expected_type, const Type& source_type,
+ bool prune_source);
+
+Result<FieldProjection> ProjectField(const SchemaField& expected_field,
+ const SchemaField& source_field, size_t source_index,
+ bool prune_source) {
+ FieldProjection projection;
+
+ if (expected_field.type()->type_id() == TypeId::kUnknown) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown &&
+ expected_field.type()->is_nested()) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown && !expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+
+ if (expected_field.type()->is_nested()) {
Review Comment:
Can you point me to the spec or impl where it's not allowed?
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#issuecomment-4560630321
@manuzhang Do you have time to revive this?
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3286530627
##########
src/iceberg/type_fwd.h:
##########
@@ -31,7 +31,7 @@ namespace iceberg {
/// This is not a complete data type by itself because some types are nested
/// and/or parameterized.
///
-/// Iceberg V3 types are not currently supported.
+/// Iceberg V3's `unknown` type is supported as a null-only placeholder type.
Review Comment:
Please register `TypeId::kUnknown -> 3` in `TableMetadata::kMinFormatVersions`. This advertises a v3-only type, but the validation table is still empty, so v1/v2 metadata can contain `unknown`. Java rejects `UNKNOWN` before v3.
##########
src/iceberg/util/type_util.cc:
##########
@@ -426,6 +426,10 @@ bool IsPromotionAllowed(const std::shared_ptr<Type>& from_type,
TypeId from_id = from_type->type_id();
TypeId to_id = to_type->type_id();
+ if (from_id == TypeId::kUnknown) {
Review Comment:
Java does not allow `UNKNOWN` as a schema-update promotion source. With this, `UpdateColumn` can turn an `unknown` column into any primitive type, which diverges from Java's `TypeUtil.isPromotionAllowed` rules.
##########
src/iceberg/json_serde.cc:
##########
@@ -540,6 +558,7 @@ Result<std::unique_ptr<SchemaField>> FieldFromJson(const nlohmann::json& json) {
ICEBERG_ASSIGN_OR_RAISE(auto required, GetJsonValue<bool>(json, kRequired));
ICEBERG_ASSIGN_OR_RAISE(auto doc, GetJsonValueOrDefault<std::string>(json, kDoc));
+ ICEBERG_RETURN_UNEXPECTED(ValidateUnknownFieldOptional(*type, !required, name));
Review Comment:
This only rejects required `unknown` during JSON parsing. The public construction/update paths still allow it via `SchemaField::MakeRequired(..., unknown())` and `AllowIncompatibleChanges().AddRequiredColumn(..., unknown())`. The invariant should live in the model/update path too.
##########
src/iceberg/schema_internal.cc:
##########
@@ -141,6 +141,9 @@ ArrowErrorCode ToArrowSchema(const Type& type, bool optional, std::string_view n
ArrowMetadataBuilderAppend(&metadata_buffer, ArrowCharView(kArrowExtensionName),
ArrowCharView(kArrowUuidExtensionName)));
} break;
+ case TypeId::kUnknown:
Review Comment:
Java Parquet skips unknown fields on write. Mapping `unknown` to Arrow null here makes the Parquet writer emit a physical null column, or depend on Arrow rejecting it. The write path should skip unknown columns instead.
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3286927100
##########
src/iceberg/schema_util.cc:
##########
@@ -79,6 +82,55 @@ Status ValidateSchemaEvolution(const Type& expected_type, const Type& source_typ
return NotSupported("Cannot read {} from {}", expected_type, source_type);
}
+Result<FieldProjection> ProjectNested(const Type& expected_type, const Type& source_type,
+ bool prune_source);
+
+Result<FieldProjection> ProjectField(const SchemaField& expected_field,
+ const SchemaField& source_field, size_t source_index,
+ bool prune_source) {
+ FieldProjection projection;
+
+ if (expected_field.type()->type_id() == TypeId::kUnknown) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown &&
+ expected_field.type()->is_nested()) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown && !expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+
+ if (expected_field.type()->is_nested()) {
Review Comment:
This looks incorrect because unknown type is not allowed to be projected to a nested 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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3355222807
##########
src/iceberg/schema.cc:
##########
@@ -35,6 +35,45 @@
namespace iceberg {
+namespace {
+
+Status ValidateUnknownFieldsOptional(const Type& type) {
+ switch (type.type_id()) {
+ case TypeId::kStruct: {
+ const auto& struct_type = static_cast<const StructType&>(type);
+ for (const auto& field : struct_type.fields()) {
+ ICEBERG_PRECHECK(field.optional() || field.type()->type_id() != TypeId::kUnknown,
+ "Unknown type field '{}' must be optional", field.name());
+ ICEBERG_RETURN_UNEXPECTED(ValidateUnknownFieldsOptional(*field.type()));
+ }
+ return {};
+ }
+ case TypeId::kList: {
+ const auto& list_type = static_cast<const ListType&>(type);
+ const auto& element = list_type.element();
+ ICEBERG_PRECHECK(
+ element.optional() || element.type()->type_id() != TypeId::kUnknown,
+ "Unknown type field '{}' must be optional", element.name());
+ return ValidateUnknownFieldsOptional(*element.type());
+ }
+ case TypeId::kMap: {
+ const auto& map_type = static_cast<const MapType&>(type);
+ const auto& key = map_type.key();
+ const auto& value = map_type.value();
+ ICEBERG_PRECHECK(key.type()->type_id() != TypeId::kUnknown,
+ "Map 'key' cannot be unknown type");
+ ICEBERG_PRECHECK(value.optional() || value.type()->type_id() != TypeId::kUnknown,
+ "Unknown type field '{}' must be optional", value.name());
+ ICEBERG_RETURN_UNEXPECTED(ValidateUnknownFieldsOptional(*key.type()));
+ return ValidateUnknownFieldsOptional(*value.type());
+ }
+ default:
+ return {};
+ }
+}
Review Comment:
```suggestion
Status ValidateFieldNullability(const Type& type) {
auto validate_field = [&](const SchemaField& field) -> Status {
ICEBERG_PRECHECK(field.optional() || field.type()->type_id() != TypeId::kUnknown,
"Unknown type field '{}' must be optional", field.name());
return ValidateFieldNullability(*field.type());
};
switch (type.type_id()) {
case TypeId::kStruct: {
const auto& struct_type = static_cast<const StructType&>(type);
for (const auto& field : struct_type.fields()) {
ICEBERG_RETURN_UNEXPECTED(validate_field(field));
}
return {};
}
case TypeId::kList: {
const auto& list_type = static_cast<const ListType&>(type);
const auto& element = list_type.element();
return validate_field(element);
}
case TypeId::kMap: {
const auto& map_type = static_cast<const MapType&>(type);
const auto& key = map_type.key();
const auto& value = map_type.value();
ICEBERG_PRECHECK(key.type()->type_id() != TypeId::kUnknown,
"Map 'key' cannot be unknown type");
ICEBERG_RETURN_UNEXPECTED(ValidateFieldNullability(*key.type()));
return validate_field(value);
}
default:
return {};
}
}
```
I'd rather rewriting it as the above. In the next step, we can reuse it for checking v3 default values.
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#issuecomment-4619020298
@wgtmac Do you have further comments?
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#issuecomment-4517142843
Please rebase on the latest main as well.
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#issuecomment-4524019729
Could you please rebase to resolve conflicts? PR for v3 nano precision timestamp types has been merged.
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3294955524
##########
src/iceberg/test/schema_json_test.cc:
##########
@@ -134,6 +135,87 @@ TEST(SchemaJsonTest, RoundTrip) {
ASSERT_EQ(dumped_json, json);
}
+TEST(SchemaJsonTest, UnknownFieldRoundTrip) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mystery","required":false,"type":"unknown"}],"schema-id":1,"type":"struct"})";
+
+ ICEBERG_UNWRAP_OR_FAIL(auto schema, SchemaFromJson(nlohmann::json::parse(json)));
+ ASSERT_EQ(schema->fields().size(), 1);
+
+ const auto& field = schema->fields()[0];
+ ASSERT_EQ(field.field_id(), 1);
+ ASSERT_EQ(field.name(), "mystery");
+ ASSERT_EQ(field.type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(field.optional());
+ ASSERT_EQ(ToJson(*schema).dump(), json);
+}
+
+TEST(SchemaJsonTest, NestedUnknownFieldsRoundTrip) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"profile","required":false,"type":{"fields":[{"id":2,"name":"mystery","required":false,"type":"unknown"}],"type":"struct"}},{"id":3,"name":"mysteries","required":false,"type":{"element":"unknown","element-id":4,"element-required":false,"type":"list"}},{"id":5,"name":"properties","required":false,"type":{"key":"string","key-id":6,"type":"map","value":"unknown","value-id":7,"value-required":false}}],"schema-id":1,"type":"struct"})";
Review Comment:
nit: It would be more readable if we made this multi-line.
##########
src/iceberg/test/schema_json_test.cc:
##########
@@ -134,6 +135,87 @@ TEST(SchemaJsonTest, RoundTrip) {
ASSERT_EQ(dumped_json, json);
}
+TEST(SchemaJsonTest, UnknownFieldRoundTrip) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mystery","required":false,"type":"unknown"}],"schema-id":1,"type":"struct"})";
+
+ ICEBERG_UNWRAP_OR_FAIL(auto schema, SchemaFromJson(nlohmann::json::parse(json)));
+ ASSERT_EQ(schema->fields().size(), 1);
+
+ const auto& field = schema->fields()[0];
+ ASSERT_EQ(field.field_id(), 1);
+ ASSERT_EQ(field.name(), "mystery");
+ ASSERT_EQ(field.type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(field.optional());
+ ASSERT_EQ(ToJson(*schema).dump(), json);
+}
+
+TEST(SchemaJsonTest, NestedUnknownFieldsRoundTrip) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"profile","required":false,"type":{"fields":[{"id":2,"name":"mystery","required":false,"type":"unknown"}],"type":"struct"}},{"id":3,"name":"mysteries","required":false,"type":{"element":"unknown","element-id":4,"element-required":false,"type":"list"}},{"id":5,"name":"properties","required":false,"type":{"key":"string","key-id":6,"type":"map","value":"unknown","value-id":7,"value-required":false}}],"schema-id":1,"type":"struct"})";
+
+ ICEBERG_UNWRAP_OR_FAIL(auto schema, SchemaFromJson(nlohmann::json::parse(json)));
+ ASSERT_EQ(schema->fields().size(), 3);
+
+ const auto* profile = dynamic_cast<const StructType*>(schema->fields()[0].type().get());
+ ASSERT_NE(profile, nullptr);
+ ASSERT_EQ(profile->fields().size(), 1);
+ ASSERT_EQ(profile->fields()[0].type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(profile->fields()[0].optional());
+
+ const auto* mysteries = dynamic_cast<const ListType*>(schema->fields()[1].type().get());
+ ASSERT_NE(mysteries, nullptr);
+ ASSERT_EQ(mysteries->fields()[0].type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(mysteries->fields()[0].optional());
+
+ const auto* properties = dynamic_cast<const MapType*>(schema->fields()[2].type().get());
+ ASSERT_NE(properties, nullptr);
+ ASSERT_EQ(properties->value().type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(properties->value().optional());
+
+ ASSERT_EQ(ToJson(*schema).dump(), json);
+}
+
+TEST(SchemaJsonTest, RejectRequiredUnknownField) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mystery","required":true,"type":"unknown"}],"schema-id":1,"type":"struct"})";
+
+ auto schema_result = SchemaFromJson(nlohmann::json::parse(json));
+ ASSERT_THAT(schema_result, IsError(ErrorKind::kJsonParseError));
+ ASSERT_THAT(schema_result,
+ HasErrorMessage("Unknown type field 'mystery' must be optional"));
+}
+
+TEST(SchemaJsonTest, RejectRequiredUnknownListElement) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mysteries","required":false,"type":{"element":"unknown","element-id":2,"element-required":true,"type":"list"}}],"schema-id":1,"type":"struct"})";
+
+ auto schema_result = SchemaFromJson(nlohmann::json::parse(json));
+ ASSERT_THAT(schema_result, IsError(ErrorKind::kJsonParseError));
+ ASSERT_THAT(schema_result,
+ HasErrorMessage("Unknown type field 'element' must be optional"));
+}
+
+TEST(SchemaJsonTest, RejectUnknownMapKey) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mysteries","required":false,"type":{"key":"unknown","key-id":2,"type":"map","value":"string","value-id":3,"value-required":false}}],"schema-id":1,"type":"struct"})";
+
+ auto schema_result = SchemaFromJson(nlohmann::json::parse(json));
Review Comment:
From the spec: Map keys are required and map values can be either optional or required. So I'm wondering should we report `'key' can not be unknow type` rather than the current error msg?
##########
src/iceberg/json_serde.cc:
##########
@@ -458,6 +470,10 @@ Result<std::unique_ptr<Type>> MapTypeFromJson(const nlohmann::json& json) {
ICEBERG_ASSIGN_OR_RAISE(auto value_id, GetJsonValue<int32_t>(json, kValueId));
ICEBERG_ASSIGN_OR_RAISE(auto value_required, GetJsonValue<bool>(json, kValueRequired));
+ ICEBERG_RETURN_UNEXPECTED(
Review Comment:
Maybe we can restrict `key_type` can't be `unknown` here, then we don't need to call ValidateUnknownFieldOptional for key_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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3294967736
##########
src/iceberg/test/schema_json_test.cc:
##########
@@ -134,6 +135,87 @@ TEST(SchemaJsonTest, RoundTrip) {
ASSERT_EQ(dumped_json, json);
}
+TEST(SchemaJsonTest, UnknownFieldRoundTrip) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mystery","required":false,"type":"unknown"}],"schema-id":1,"type":"struct"})";
+
+ ICEBERG_UNWRAP_OR_FAIL(auto schema, SchemaFromJson(nlohmann::json::parse(json)));
+ ASSERT_EQ(schema->fields().size(), 1);
+
+ const auto& field = schema->fields()[0];
+ ASSERT_EQ(field.field_id(), 1);
+ ASSERT_EQ(field.name(), "mystery");
+ ASSERT_EQ(field.type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(field.optional());
+ ASSERT_EQ(ToJson(*schema).dump(), json);
+}
+
+TEST(SchemaJsonTest, NestedUnknownFieldsRoundTrip) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"profile","required":false,"type":{"fields":[{"id":2,"name":"mystery","required":false,"type":"unknown"}],"type":"struct"}},{"id":3,"name":"mysteries","required":false,"type":{"element":"unknown","element-id":4,"element-required":false,"type":"list"}},{"id":5,"name":"properties","required":false,"type":{"key":"string","key-id":6,"type":"map","value":"unknown","value-id":7,"value-required":false}}],"schema-id":1,"type":"struct"})";
+
+ ICEBERG_UNWRAP_OR_FAIL(auto schema, SchemaFromJson(nlohmann::json::parse(json)));
+ ASSERT_EQ(schema->fields().size(), 3);
+
+ const auto* profile = dynamic_cast<const StructType*>(schema->fields()[0].type().get());
+ ASSERT_NE(profile, nullptr);
+ ASSERT_EQ(profile->fields().size(), 1);
+ ASSERT_EQ(profile->fields()[0].type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(profile->fields()[0].optional());
+
+ const auto* mysteries = dynamic_cast<const ListType*>(schema->fields()[1].type().get());
+ ASSERT_NE(mysteries, nullptr);
+ ASSERT_EQ(mysteries->fields()[0].type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(mysteries->fields()[0].optional());
+
+ const auto* properties = dynamic_cast<const MapType*>(schema->fields()[2].type().get());
+ ASSERT_NE(properties, nullptr);
+ ASSERT_EQ(properties->value().type()->type_id(), TypeId::kUnknown);
+ ASSERT_TRUE(properties->value().optional());
+
+ ASSERT_EQ(ToJson(*schema).dump(), json);
+}
+
+TEST(SchemaJsonTest, RejectRequiredUnknownField) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mystery","required":true,"type":"unknown"}],"schema-id":1,"type":"struct"})";
+
+ auto schema_result = SchemaFromJson(nlohmann::json::parse(json));
+ ASSERT_THAT(schema_result, IsError(ErrorKind::kJsonParseError));
+ ASSERT_THAT(schema_result,
+ HasErrorMessage("Unknown type field 'mystery' must be optional"));
+}
+
+TEST(SchemaJsonTest, RejectRequiredUnknownListElement) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mysteries","required":false,"type":{"element":"unknown","element-id":2,"element-required":true,"type":"list"}}],"schema-id":1,"type":"struct"})";
+
+ auto schema_result = SchemaFromJson(nlohmann::json::parse(json));
+ ASSERT_THAT(schema_result, IsError(ErrorKind::kJsonParseError));
+ ASSERT_THAT(schema_result,
+ HasErrorMessage("Unknown type field 'element' must be optional"));
+}
+
+TEST(SchemaJsonTest, RejectUnknownMapKey) {
+ constexpr std::string_view json =
+ R"({"fields":[{"id":1,"name":"mysteries","required":false,"type":{"key":"unknown","key-id":2,"type":"map","value":"string","value-id":3,"value-required":false}}],"schema-id":1,"type":"struct"})";
+
+ auto schema_result = SchemaFromJson(nlohmann::json::parse(json));
Review Comment:
From the spec: Map keys are required and map values can be either optional or required. So I'm wondering should we report something like `Map 'key' can not be unknown type` rather than the current error msg?
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3342607218
##########
src/iceberg/avro/avro_writer.cc:
##########
@@ -81,6 +89,126 @@ Result<std::optional<int32_t>> ParseCodecLevel(const WriterProperties& propertie
return level;
}
+enum class FieldContext {
+ kTopLevel,
+ kStruct,
+ kListElement,
+ kMapKey,
+ kMapValue,
+};
+
+Result<std::optional<SchemaField>> PruneUnknownField(const SchemaField& field,
+ FieldContext context) {
+ if (field.type()->type_id() == TypeId::kUnknown) {
+ ICEBERG_PRECHECK(context != FieldContext::kMapKey,
+ "Cannot write map key '{}' of unknown type because it has no "
+ "physical Avro representation",
+ field.name());
+ ICEBERG_PRECHECK(field.optional(), "Unknown type field '{}' must be optional",
+ field.name());
+ if (context == FieldContext::kListElement || context == FieldContext::kMapValue) {
+ return field;
+ }
Review Comment:
Sorry, I may have been misleading in my earlier comment. I think your original logic should be correct.
For Parquet, rejecting `list<unknown>` and `map<..., unknown>` makes sense because there is no physical value column to write. Pruning the unknown element or value would lose list cardinality or map keys.
Avro is different. This PR already maps `UnknownType` to `AVRO_NULL`, so `list<unknown>` can be represented as an Avro array with null items, and `map<string, unknown>` as an Avro map with null values. That preserves list lengths and map keys.
This also matches the Java impl: `TypeToSchema` maps `UNKNOWN` to `NULL_SCHEMA`, and the Spark Avro test suite includes coverage for both `testUnknownListType` and `testUnknownMapType`.
cc @wgtmac to double-check my understanding.
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3287296159
##########
src/iceberg/schema_util.cc:
##########
@@ -79,6 +82,55 @@ Status ValidateSchemaEvolution(const Type& expected_type, const Type& source_typ
return NotSupported("Cannot read {} from {}", expected_type, source_type);
}
+Result<FieldProjection> ProjectNested(const Type& expected_type, const Type& source_type,
+ bool prune_source);
+
+Result<FieldProjection> ProjectField(const SchemaField& expected_field,
+ const SchemaField& source_field, size_t source_index,
+ bool prune_source) {
+ FieldProjection projection;
+
+ if (expected_field.type()->type_id() == TypeId::kUnknown) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown &&
+ expected_field.type()->is_nested()) {
+ if (!expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+ projection.kind = FieldProjection::Kind::kNull;
+ return projection;
+ }
+
+ if (source_field.type()->type_id() == TypeId::kUnknown && !expected_field.optional()) {
+ return InvalidSchema("Cannot project required field with id {} as null",
+ expected_field.field_id());
+ }
+
+ if (expected_field.type()->is_nested()) {
Review Comment:
Ok, I misunderstood your comments as unknown type is not allowed inside a nested 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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "zhjwpku (via GitHub)" <gi...@apache.org>.
zhjwpku commented on code in PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#discussion_r3335450138
##########
src/iceberg/avro/avro_writer.cc:
##########
@@ -81,6 +89,126 @@ Result<std::optional<int32_t>> ParseCodecLevel(const WriterProperties& propertie
return level;
}
+enum class FieldContext {
+ kTopLevel,
+ kStruct,
+ kListElement,
+ kMapKey,
+ kMapValue,
+};
+
+Result<std::optional<SchemaField>> PruneUnknownField(const SchemaField& field,
+ FieldContext context) {
+ if (field.type()->type_id() == TypeId::kUnknown) {
+ ICEBERG_PRECHECK(context != FieldContext::kMapKey,
+ "Cannot write map key '{}' of unknown type because it has no "
+ "physical Avro representation",
+ field.name());
+ ICEBERG_PRECHECK(field.optional(), "Unknown type field '{}' must be optional",
+ field.name());
+ if (context == FieldContext::kListElement || context == FieldContext::kMapValue) {
+ return field;
+ }
Review Comment:
This is different from the PruneUnknownField in parquet_writer.cc, is it intentional?
##########
src/iceberg/avro/avro_schema_util.cc:
##########
@@ -733,20 +764,9 @@ Result<FieldProjection> ProjectList(const ListType& list_type,
}
FieldProjection element_projection;
- ::avro::NodePtr element_node;
- ICEBERG_RETURN_UNEXPECTED(UnwrapUnion(avro_node->leafAt(0), &element_node));
- if (expected_element_field.type()->is_nested()) {
- ICEBERG_ASSIGN_OR_RAISE(
- element_projection,
- ProjectNested(*expected_element_field.type(), element_node, prune_source));
- } else {
- ICEBERG_RETURN_UNEXPECTED(
- ValidateAvroSchemaEvolution(*expected_element_field.type(), element_node));
- }
-
- // Set the element projection metadata but preserve its children
- element_projection.kind = FieldProjection::Kind::kProjected;
- element_projection.from = size_t{0};
+ ICEBERG_ASSIGN_OR_RAISE(element_projection,
+ ProjectField(expected_element_field, avro_node->leafAt(0),
+ size_t{0}, prune_source));
Review Comment:
nit:
```suggestion
/*source_index*/size_t{0}, prune_source));
```
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662
--
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: support Iceberg v3 unknown type [iceberg-cpp]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #662:
URL: https://github.com/apache/iceberg-cpp/pull/662#issuecomment-4561066124
Yes, I was on a trip this week and will update a bit 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