-
Notifications
You must be signed in to change notification settings - Fork 535
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
Merge-Tree and SharedString ISegment Deprecations #23323
Merge-Tree and SharedString ISegment Deprecations #23323
Conversation
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.
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
…to deprecate-isegment-properties
5225291
to
59051de
Compare
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.
Aproving for docs with a couple of suggestions.
Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
Co-authored-by: Alex Villarreal <716334+alexvy86@users.noreply.github.com>
Co-authored-by: Abram Sanderson <Abram.sanderson@gmail.com>
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output
|
) This change aligns the insertion information of an ISegment with the existing IRemovalInfo and IMoveInfo by adding a new interface, IInsertionInfo. This work is primarily precursor work, which is related to #23323, as these changes were made with some of those deprecation's removed locally so that we can reduce the eventual churn when we finally remove those properties. In addition to adding the new interface, this change also standardizes the methods we provide for each of these interfaces; - toInsertionInfo/toRemovealInfo/toMoveInfo - isInserted/isRemoved/IsMoved - assertInserted/assertRemoved/assertMoved There are also some general utility types and methods which make the info types easier to work with: - SegmentInfo - SegmentWithInfo - overwriteSegmentInfo After deprecations are removed. These types will also aid in improving typing to reduce the need for assertions. Lastly, I moved all the info from ISegmentInternal to ISegmentLeaf. This better approximates the final state, and allow further reducing the exposure of the internal state of the segment. Due to this some changes in other packages were necessary, but all could be solved with the existing segmentIsRemovedFunction --------- Co-authored-by: jzaffiro <110866475+jzaffiro@users.noreply.github.com>
The current ISegment interface over-exposes a number of properties which do not have an external use case, and any external usage could result in damage to the underlying merge-tree including data corruption.
The only use case that will continue to be supported is determining if a segment is removed. For this purpose we've add the following
function segmentIsRemoved(segment: ISegment): boolean
For example, checking if a segment is not removed would change as follows:
The following properties are deprecated on ISegment and its implementations:
Additionally, the following types are also deprecated, and will become internal: