-
Notifications
You must be signed in to change notification settings - Fork 310
Support multiple Update
operations for the same column name
#1596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Previously, the `Update` class used a `Map` to store its operations, keyed by column name. This prevented multiple operations on the same column, such as setting different keys in a map-type column, as later operations would overwrite earlier ones. This commit changes the internal data structure to a `List`, allowing multiple operations to be recorded. To align with the 'last-wins' semantics for conflicting operations (e.g., setting the same column twice), a conflict resolution mechanism has been added. New operations now replace existing, conflicting ones. Adds comprehensive unit tests to verify both the coexistence of non-conflicting operations and the replacement of conflicting ones. Signed-off-by: Jeongjun Min <m3k0813@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot. This looks pretty decent. I'm going to polish this pull request for inclusion in the next release.
@Override | ||
public String toString() { | ||
return StringUtils.collectionToDelimitedString(updateOperations.values(), ", "); | ||
private static boolean equalsNullSafe(@Nullable Object a, @Nullable Object b) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method isn't needed, use ObjectUtils.nullSafeEquals(…)
instead.
Thanks for the review and the feedback, @mp911de! I'm glad you liked the contribution :) |
Update
operations for a column
Update
operations for a columnUpdate
operations for the same column name
Previously, the `Update` class used a `Map` to store its operations, keyed by column name. This prevented multiple operations on the same column, such as setting different keys in a map-type column, as later operations would overwrite earlier ones. This commit changes the internal data structure to a `List`, allowing multiple operations to be recorded. To align with the 'last-wins' semantics for conflicting operations (e.g., setting the same column twice), a conflict resolution mechanism has been added. New operations now replace existing, conflicting ones. Adds comprehensive unit tests to verify both the coexistence of non-conflicting operations and the replacement of conflicting ones. Signed-off-by: Jeongjun Min <m3k0813@gmail.com> See #1525 Original pull request: #1596
Previously, the `Update` class used a `Map` to store its operations, keyed by column name. This prevented multiple operations on the same column, such as setting different keys in a map-type column, as later operations would overwrite earlier ones. This commit changes the internal data structure to a `List`, allowing multiple operations to be recorded. To align with the 'last-wins' semantics for conflicting operations (e.g., setting the same column twice), a conflict resolution mechanism has been added. New operations now replace existing, conflicting ones. Adds comprehensive unit tests to verify both the coexistence of non-conflicting operations and the replacement of conflicting ones. Signed-off-by: Jeongjun Min <m3k0813@gmail.com> See #1525 Original pull request: #1596
Thank you for your contribution. That's merged, polished, and backported now. |
Fixes #1525
This PR enhances the
Update
class to support multiple operations on the same column, resolving the limitation of the previousMap
-based implementation.Changed the internal data structure of
Update
fromMap<ColumnName, AssignmentOp>
toList<AssignmentOp>
. This allows multiple non-conflicting operations (e.g., setting different keys in a map) to coexist.Introduced a conflict resolution mechanism to handle duplicate or conflicting operations. The new implementation follows a "last-wins" policy, as suggested in the issue discussion. For example, a
set("name", "B")
operation will replace a previousset("name", "A")
.Added a comprehensive suite of unit tests to
UpdateUnitTests
to verify both the coexistence of non-conflicting operations and the "last-wins" replacement policy for various edge cases.