Fix Compare to return raw diff when source equals base#9844
Closed
ssam18 wants to merge 1 commit intotreeverse:masterfrom
Closed
Fix Compare to return raw diff when source equals base#9844ssam18 wants to merge 1 commit intotreeverse:masterfrom
ssam18 wants to merge 1 commit intotreeverse:masterfrom
Conversation
When comparing a branch with an ancestor commit (where source == base), the CompareValueIterator was filtering out all changes that were 'added on destination', resulting in empty diff results. This fix short-circuits the Compare function to return the raw diff when source equals base, which correctly shows all differences from the ancestor commit. Fixes treeverse#9802
arielshaqed
requested changes
Dec 18, 2025
Contributor
arielshaqed
left a comment
There was a problem hiding this comment.
Please see this comment on the issue. I believe you may be interested in the somewhat unfortunately-named two_dot comparison type in the OpenAPI spec.
Unfortunately, given that the existing API supports doing this in another way, and given that this would be a breaking API changing, this means that we will not be able to merge this PR.
Please free to discuss with us on #dev or #help on the lakeFS Slack workspace, lakefs.io/slack.
Contributor
|
Closing for lack of author engagement. Please re-open and grab me - on lakeFS Slack if here didn't work - when you have time to work on this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When comparing a branch with an ancestor commit (where source == base), the
CompareValueIteratorwas filtering out all changes that were 'added on destination', resulting in empty diff results.This caused:
lakectl branch revertto return 'no changes' error even when there were actual differenceslakectl diffto return empty results when comparing with an ancestor tag/commitSolution
Added a short-circuit in the
Comparefunction: whensource == base(i.e., comparing with an ancestor), return the raw diff iterator directly instead of wrapping it inCompareValueIteratorwhich filters results.When source equals base, the three-way comparison degenerates to a two-way diff, so the filtering logic is inappropriate.
Changes
pkg/graveler/committed/manager.go: Added check forsource == basecase inComparefunctionpkg/graveler/committed/manager_test.go: AddedTestManager_CompareSourceEqualsBasetestTesting
Fixes #9802