Respond to PR feedback.

This commit is contained in:
Nick Tobey
2025-10-21 13:03:57 -07:00
parent 92519382c4
commit a7006502d7
3 changed files with 15 additions and 10 deletions

View File

@@ -1,4 +1,4 @@
// Copyright 2023 Dolthub, Inc.
// Copyright 2025 Dolthub, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
@@ -51,18 +51,16 @@ func TestDataMerge(t *testing.T) {
// new prolly tree node, but none of the keys in that new node have changed from the common ancestor. The merge
// algorithm assumed that any new node on one side of the merge would necessarily contain a changed key-value pair, but
// this is not guaranteed. One example where this could cause an error is when all the following are true:
// - An insert caused a shift in node boundaries on one side of the merge.
// - The node immediately prior to this is unchanged on at least one branch.
// - As a result of the shift, both sides of the merge now contain new nodes that have the same end key,
// but different start keys.
// - One of these two new nodes contains only key-value pairs from the common ancestor, but the other node does not.
// - After this point, neither branch has any additional changes from the common ancestor.
// - An insert caused a shift in node boundaries on the right side of the merge.
// - The node immediately prior to this boundary shift is unchanged on the left side.
// - Within the region where node boundaries have shifted, the right side has no other changes, but the left side does.
// - After the point where the node boundaries realign, neither branch has any additional changes from the common ancestor.
//
// In this situation, the merge algorithm would hit an unexpected EOF error while attempting to diff the two nodes.
//
// The below table recreates these conditions: inserting the key 15 shifts the node boundaries, resulting in the right
// branch containing a node with keys ranging from _ to _, all of which match the ancestor. The left branch contains
// a node with keys ranging from _ to _, which also includes the key 40.
// branch containing a new node with keys ranging from 32 to 47, all of which match the ancestor. The left branch contains
// a new node with keys ranging from 37 to 58, including an additional inserted key 40.
var shiftingNodeBoundariesTest = func() dataMergeTest {
charString := strings.Repeat("1", 255)
var rows []sql.Row

View File

@@ -310,6 +310,12 @@ func (td *PatchGenerator[K, O]) advanceFromPreviousPatch(ctx context.Context) (p
return Patch{}, NoDiff, true, nil
}
// Next finds the next key-value pair (including intermediate pairs pointing to child nodes)
// this is present in |td.to| that is not present in |td.from|.
// |isMore| is false iff we have exhausted both cursors and there are no more patches. Otherwise, |patch| contains
// a patch representing that diff.
// Note that we choose to use |isMore| instead of returning io.EOF because it is more explicit. Callers should
// check the value of isMore instead of checking for io.EOF.
func (td *PatchGenerator[K, O]) Next(ctx context.Context) (patch Patch, diffType DiffType, isMore bool, err error) {
if td.previousDiffType != NoDiff {
patch, diffType, isMore, err = td.advanceFromPreviousPatch(ctx)
@@ -536,7 +542,7 @@ func skipCommonVisitingParents(ctx context.Context, from, to *cursor) (lastSeenK
for from.Valid() && to.Valid() {
if !equalItems(from, to) {
// isMore the next difference
// found the next difference
return lastSeenKey, from, to, nil
}

View File

@@ -210,6 +210,7 @@ func TestPatchGeneratorFromRoots(t *testing.T) {
dfr, err = PatchGeneratorFromRoots(ctx, ns, ns, fromRoot, toRoot, desc)
require.NoError(t, err)
_, _, isMore, err = dfr.Next(ctx)
require.NoError(t, err)
require.True(t, isMore)
dif, diffType, isMore, err = dfr.split(ctx)