Skip to content

[rust] add support of iceberg format key encoder - #3934

Merged
fresh-borzoni merged 2 commits into
apache:mainfrom
zuston:rusticeberg
Aug 11, 2026
Merged

[rust] add support of iceberg format key encoder#3934
fresh-borzoni merged 2 commits into
apache:mainfrom
zuston:rusticeberg

Conversation

@zuston

@zuston zuston commented Aug 10, 2026

Copy link
Copy Markdown
Member

Purpose

add support of iceberg format key encoder

Brief change log

Tests

API and Format

Documentation

@zuston

zuston commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

could you help review this? @fresh-borzoni

@fresh-borzoni fresh-borzoni left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@zuston Thank you for the PR, LGTM over, some minor comments and suggestions, PTAL

Comment thread fluss-rust/crates/fluss/src/row/encode/mod.rs
}

#[test]
fn encodes_values_with_iceberg_binary_layout() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These check the bytes against values copied from our Java code, not from Iceberg — and the IT can't help since write and read share the encoder.

Could you assert Iceberg's Appendix B hash vectors instead? Run them through IcebergBucketingFunction with num_buckets = i32::MAX so the modulo cancels and you get the raw hash back. Negative ones need hash & i32::MAX accordingly.

Let me know if you need more clarifications.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

really insightful comment! updated

@fresh-borzoni fresh-borzoni left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@zuston Thank you, LGTM 👍

@fresh-borzoni
fresh-borzoni merged commit 471160e into apache:main Aug 11, 2026
13 checks passed
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.

2 participants