feat:add init expression interface. #58
Conversation
zhjwpku
left a comment
zhjwpku
left a comment
There was a problem hiding this comment.
I'm wondering should we add some subdirs below iceberg, this feature seems not be in the spec?
| const auto& other = static_cast<const And&>(expr); | ||
| return (left_->Equals(*other.left()) && right_->Equals(*other.right())) || | ||
| (left_->Equals(*other.right()) && right_->Equals(*other.left())); |
There was a problem hiding this comment.
In Java we call this isEquivalentTo: https://github.com/apache/iceberg/blob/97d1107d60b42d29306c663f23f869fc1d439e6b/api/src/main/java/org/apache/iceberg/expressions/Or.java#L43-L52, which is different from being equal. Should we follow the same naming for clarity?
There was a problem hiding this comment.
Good suggestion! In the first version, it was isEquivalentTo. I followed the style of other classes in the codebase and switched to Equals for consistency.
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
@yingcai-cy Can you rebase against main to resolve the conflict? Thanks!
Xuanwo
left a comment
Xuanwo
left a comment
There was a problem hiding this comment.
Thank you for this great work!
|
Thanks @yingcai-cy for working on this, and thanks @alonesniper, @gty404, @lidavidm, @wgtmac, @Xuanwo and @zhjwpku for the review 🙌 Great to have this in! |
8 participants
No description provided.