Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Add transform #26

Merged
merged 1 commit into from
Aug 10, 2023
Merged

feat: Add transform #26

merged 1 commit into from
Aug 10, 2023

Conversation

liurenjie1024
Copy link
Collaborator

@liurenjie1024 liurenjie1024 commented Aug 9, 2023

This pr doesn't contains actual transform function implementation, since it depends on #20 But it introduces basic transform definition.

@liurenjie1024
Copy link
Collaborator Author

cc @Fokko @nastra @JanKaul @Xuanwo PTAL

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

One suggestion on adding an UnknownTransform, otherwise LGTM. Great work on the comprehensive tests

crates/iceberg/src/spec/transform.rs Outdated Show resolved Hide resolved
crates/iceberg/src/spec/transform.rs Show resolved Hide resolved
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@JanKaul JanKaul left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you

@liurenjie1024
Copy link
Collaborator Author

CC @Fokko Any other comments?

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

@liurenjie1024 No this looks good. The next step would be to implement the actual functions, hash, truncate, etc

@Fokko Fokko merged commit 5ab3334 into apache:main Aug 10, 2023
7 checks passed
@liurenjie1024 liurenjie1024 deleted the renjie/transform branch August 11, 2023 02:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants