Skip to content

Conversation

dongxiao1198
Copy link
Contributor

1 parse v1v2v3 manifest schema in adapter
2 convert ManifestFile&ManifestEntry into ArrowArray
3 add e2e case

@dongxiao1198 dongxiao1198 marked this pull request as ready for review September 9, 2025 04:47
ICEBERG_ASSIGN_OR_RAISE(auto source_field,
schema_->FindFieldById(partition_field.source_id()));
if (!source_field.has_value()) {
return InvalidSchema("Cannot find source field for partition field:{}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a TODO comment to use unknown type when source field is missing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not resolved yet.

@dongxiao1198 dongxiao1198 force-pushed the manifest_adapter_impl branch from bafed12 to 977466d Compare October 9, 2025 08:23
@dongxiao1198 dongxiao1198 requested a review from wgtmac October 9, 2025 08:49
const std::shared_ptr<Schema>& schema() const { return manifest_schema_; }

protected:
virtual Result<std::shared_ptr<StructType>> GetManifestEntryStructType();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think StructType is redundant.

ICEBERG_ASSIGN_OR_RAISE(auto source_field,
schema_->FindFieldById(partition_field.source_id()));
if (!source_field.has_value()) {
return InvalidSchema("Cannot find source field for partition field:{}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not resolved yet.

std::vector<PartitionField> fields_;
int32_t last_assigned_field_id_;
std::mutex mutex_;
std::shared_ptr<Schema> partition_schema_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

@dongxiao1198 dongxiao1198 force-pushed the manifest_adapter_impl branch from 96be403 to ec65a69 Compare October 11, 2025 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants